[DON'T MERGE] Alternative hashing interface #14442
Closed
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
|
@swift-ci please benchmark |
|
@swift-ci please smoke test |
|
(Some hash values are different from master, so some tests will fail.) |
Build comment file:Optimized (O)Regression (63)
Improvement (11)
No Changes (286)
Unoptimized (Onone)Regression (90)
Improvement (8)
No Changes (262)
Hardware Overview |
- Add the method _hash(into:) as a Hashable requirement. It takes a closure into which the function can feed integer values representing bits to be hashed. - Implement _hash(into:) in terms of hashValue in an extension, for compatibility. - Add struct _QuickHasher, for emulating Swift 4 hashes in the new interface. - Add _defaultHashValue<T>(for:), implementing hashValue in terms of _hash(into:) and _QuickHasher.
We don't need to stream bytes, just words.
lorentey
force-pushed
the
smashing-hashing
branch
from
7ffe402 to
c3be208
Compare
|
@swift-ci please benchmark |
Build comment file:Optimized (O)Regression (58)
Improvement (6)
No Changes (298)
Unoptimized (Onone)Regression (87)
Improvement (7)
No Changes (268)
Hardware Overview |
This is a version for hasher that uses a nonmutating append, so that we can mark it @effects(readonly), reducing retain/releases around its calls.
|
@swift-ci please benchmark |
|
c433efd applies the lesson from #14466 directly onto the This may speed things up a bit, but in exchange, we lose the ability to select a hasher at runtime, and the size of the hasher state likely becomes a limiting factor. (SipHash has a state that is 5 words wide.) |
This restores speed of String hashing, among other types.
|
@swift-ci please benchmark |
Build comment file:Optimized (O)Regression (50)
Improvement (4)
No Changes (308)
Unoptimized (Onone)Regression (17)
Improvement (4)
No Changes (341)
Hardware Overview |
|
Very good! We're down to +40% on synthetic microbenchmarks. |
|
The latest commit should have equivalent or better performance, while hopefully also giving a boost to #14511's benchmarks. (This also enables us to remove the size of the hash state from the ABI.) |
|
@swift-ci please benchmark |
lorentey
commented
Feb 12, 2018
stdlib/public/core/Hashable.swift
Outdated
| var hasher = _Hasher(_inlineable: ()) | ||
| return withUnsafeMutablePointer(to: &hasher) { p in | ||
| return _UnsafeHasher(p).appending(value).finalized() | ||
| } |
Build comment file:Build failed before running benchmark. |
|
@swift-ci please benchmark |
Build comment file:Optimized (O)Regression (51)
Improvement (7)
No Changes (304)
Unoptimized (Onone)Regression (15)
Improvement (18)
No Changes (329)
Hardware Overview |
Always synthesizing _hash(into:) almost worked, but it still had trouble with some imported classes and it had diagnostics regressions that seemed impossible to resolve without a major rework of how DerivedConformance works. This approach works more reliably although it's a lot uglier.
This eliminates the chance of random test failures due to changing hash values, freak hash collision situations and unstable Dictionary ordering. On the other hand, it allows tests to rely on specific hash values, which is unwise. It also requires that a TestSuite be initialized before the first element is inserted into any Set or Dictionary.
|
@swift-ci please test |
Also, support appending 32-bit values on 64-bit platforms, too.
|
@swift-ci please test |
|
Build failed |
lorentey
force-pushed
the
smashing-hashing
branch
2 times, most recently
from
1bd9b8d to
91e2eae
Compare
…length; restore some tests. Appending is still limited to 32- or 64-bit values. However, a new version of finalize accepts a partial word to append at the end of the sequence. This is only used for testing.
lorentey
force-pushed
the
smashing-hashing
branch
from
d8d8ff6 to
35c9aa4
Compare
|
A cleaned-up version of this lives on in #14913. See you there! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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 implements an alternative hashing interface that moves the choice of a hash function out of
Hashable. Doing this allows us to evolve the hash function in the stdlib without recompiling hashable types.To achieve this, it defines a streaming interface where hashable types can feed some stateful hash function with a variable number of
Ints representing the bits to be hashed:The new
_hash(into:)requirement comes with a default implementation for compatibility. However, it replaceshashValueas the primary hashing API forSetandDictionary.I expect we won't need to make the new method public. Outside the stdlib, it will be implemented by synthesized conformances of
Hashable. Manually conformingHashabletypes will continue to implementhashValue, as they have done since Swift 1.0. (We can't deprecatehashValuewithout significant source compatibility issues, and it doesn't seem worth the upgrade churn.)