-
Notifications
You must be signed in to change notification settings - Fork 116
Support specializing get_hash without nightly #242
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
base: master
Are you sure you want to change the base?
Conversation
Retargeting this to the 0.9 pre-release branch. |
@tkaitchuck If only 0.9 is fixed, then the abandoned unfixed 0.8 version will cause the same kind of pain that the abandoned unfixed 0.7 version has caused when only 0.8 had a fix. |
I agree with #242 (comment) — I was hoping to get this in an 0.8 patch. |
I want to pull the generic type from the type of the map and type the builder to it rather than use that parameter. The advantage of this is we don't depend on having an implementation for the dereferenced type. |
src/specialize.rs
Outdated
#[inline] | ||
fn get_hash<H: Hash + ?Sized, B: BuildHasher>(value: &H, build_hasher: &B) -> u64 { | ||
let mut hasher = build_hasher.build_hasher(); | ||
if let Some(state) = as_ahash_state(state) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think CallHasher actually needs to be generic with respect to BuildHasher, if we can change it to just take a RandomState we can eliminate these ifs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good call. I opened #261 to change CallHasher
.
|
|
Submitting as an alternative to #239. Benchmarks:
Before:
After:
It's clear that we are now getting exactly the specialized behavior of both aeshash and fallbackhash, even without the use of unstable min_specialization.