Fix incorrect hashing leading to cache pollution #1170
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
Before this commit, Hashable#addDelta did not work correctly when the input hash was the special value NotCached, instead of returning NotCached, it returned NotCached + delta. This means that many different values ended up being cached with the same hash when they should not be cached at all, this is especially bad since our HashSet implementation uses open addressing. I noticed this bug while working on a phase to collect API information for sbt (this phase needs to collect every member of a class, including inherited members), after enabling it, the compileStdLib test took ~500 seconds to complete, this commit reduces this to ~100 seconds.
odersky
added a commit
that referenced
this pull request
Mar 12, 2016
Fix incorrect hashing leading to cache pollution
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.
Before this commit, Hashable#addDelta did not work correctly when the
input hash was the special value NotCached, instead of returning
NotCached, it returned NotCached + delta. This means that many different
values ended up being cached with the same hash when they should not be
cached at all, this is especially bad since our HashSet implementation
uses open addressing.
I noticed this bug while working on a phase to collect API information
for sbt (this phase needs to collect every member of a class, including
inherited members), after enabling it, the compileStdLib test took
~500 seconds to complete, this commit reduces this to ~100 seconds.
Review by @odersky