-
Notifications
You must be signed in to change notification settings - Fork 273
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
optimizes Primus observations #1061
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
i.e., if someone is interested then an observation is made, otherwise we let our opinion to ourselves
so we won't create intermediate data structures and can even hope that the inliner will do its work better than before.
uses it to cancel unused signals
adds the clock to primus as well as the clock observation, turns limited to work on the clock, which significantly improves the performance.
ivg
added a commit
to BinaryAnalysisPlatform/bap-testsuite
that referenced
this pull request
Feb 25, 2020
BinaryAnalysisPlatform/bap#1061 switches the limiter to count the clocks cycles instead of the RTL instructions, and while they more or less interchangeable it looks like that counts tick a little bit slower than RTLs, so we need to increase the depth.
the new limiter is a little bit different from the old one.
gitoleg
approved these changes
Feb 25, 2020
gitoleg
added a commit
to gitoleg/bap
that referenced
this pull request
Feb 28, 2020
The new clocks, as introduced in BinaryAnalysisPlatform#1061, were ticking too fast (roughly 3 to 5 times faster) so we recalibrated them to be closer to the old clocks that were using binop/unop/load/store events
ivg
pushed a commit
that referenced
this pull request
Feb 28, 2020
* recalibrates Primus clocks The new clocks, as introduced in #1061, were ticking too fast (roughly 3 to 5 times faster) so we recalibrated them to be closer to the old clocks that were using binop/unop/load/store events * constrainted result < 1.5.0 because of zed incompatibility * constrained result =1.4.0 * result 1.4.0 => 1.4
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR brings a few small optimizations that together make Primus run 3 to 4 times faster. E.g., in the current master branch, it takes more than 150 seconds to run the memcheck analysis on /bin/true, while in this branch it terminates in 40 seconds (with the same results, coverage, and the number of states). This is 3.5 improvement in speed. YMMV of course as it depends on the analysis and appetite. Speaking of the appetite, this branch implements lazy notifications, so that if an observation is not subscribed than it is not provided. Therefore, we don't have to pay for unused notifications. As a result, the fewer observations you watch the faster your analysis will run.
Technical Details
Since Primus 2.0 is stall far from being ready, we decided to backport some of the ideas to the current version. In Primus 1.0 the observation are usually tuples (or other heap-allocated values), therefore every time we post an observation we have to allocate a new object even if nobody is interested in receiving it. In Primus 2.0 we changed the observation machiner, so that now each observation is parametrized by an arrow type and observers could receive their inputs in a curried (untupled) form, e.g.,
This will still create a closure (as there are free variables in the continuation), but in this form the compiler has much more chances to optimize the closure allocation. Additionally, in this form the continuation
f
won't be called at all if the given observation has no subscribers.The latter feature is backported to the current version of Primus. We got the
post
function, that has the same interface (except that arguments are still tupled):It is already a little bit better, especially in case when the observation itself has to be computed (e.g., the taint-attached, incident, incident-location, calls (and the upcoming primitive from #1049) are pretty heavy-weight.
However, many of the observations are reflected to Primus Lisp signals, so that they could be watched and processed directly in Primus Lisp. Since signal reflection is implemented via observations, we have all of the reflected observations subscribed, that prevents the optimization above from kicking in. Even when there is no method for the given signal, it is still treated as it is being watched. To prevent this from happening, we provide a new interface of cancelable subscriptions, i.e., now it is possible to subscribe to an observation and the revoke your subscription. Using this new interface we unsubscribe in Primus Lisp from all reflected observations that do not have a corresponding method.
Another culprit was the primus-limiter plugin which was subscribing to a lot of observations just to track the number of operations that the Primus machine performs. The plugin was ignoring the values associated with the observations and was just counting them. A much more natural solution would be to add a clock primus machine and corresponding observations, so that implementing such kind of alarms would be much easier. We also added the
time
operation to the interpreter interface, that gives read/only access to the machine's clock.Finally, we removed the primus-limited notifications to the progress bar, as they were slowing down everything (especially on slow terminals) and wasn't really useful (we have basically 1k to 10k observations per second, probably too fine granular for being useful).