Optimize calls to increase_counter #3554
Merged
Conversation
This file contains hidden or 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
romainbrenguier
requested review from
kroening,
peterschrammel and
tautschnig
as code owners
tautschnig
approved these changes
Dec 10, 2018
romainbrenguier
force-pushed
the
optimize/symex-assignment
branch
from
ae368be to
1eb5181
Compare
allredj
approved these changes
Dec 10, 2018
| } | ||
| else | ||
| state.level1.current_names.emplace( | ||
| l0_name, std::make_pair(ssa, frame_nr)); |
There was a problem hiding this comment.
⛏️ Braces would be helpful here
| @@ -38,10 +38,9 @@ struct symex_renaming_levelt | |||
| } | |||
|
|
|||
| /// Increase the counter corresponding to an identifier | |||
| void increase_counter(const irep_idt &identifier) | |||
| static void increase_counter(const current_namest::iterator &it) | |||
There was a problem hiding this comment.
⛏️ Would have been better to create two different commits for these two changes
There was a problem hiding this comment.
I'm not sure which two changes you are talking about
allredj
reviewed
Dec 10, 2018
There was a problem hiding this comment.
✔️
Passed Diffblue compatibility checks (cbmc commit: 1eb5181).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/94207006
Taking an iterator instead of an identifier removes the need for a precondition (thus avoiding a lookup) and can avoid an additional lookup.
romainbrenguier
force-pushed
the
optimize/symex-assignment
branch
from
1eb5181 to
e71ca91
Compare
|
@tautschnig on my benchmarks the symex time went down by 9% |
allredj
reviewed
Dec 10, 2018
There was a problem hiding this comment.
✔️
Passed Diffblue compatibility checks (cbmc commit: e71ca91).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/94218134
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.
increase_counteris always called after a lookup in the corresponding map, and performs two additional lookup, we can get rid of this redundancy by passing an iterator as argument.