Skip to content
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

Expose specialized versions of AHasher through specialized-hashers feature #156

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

fu5ha
Copy link

@fu5ha fu5ha commented May 8, 2023

This adds a specialized-hashers feature which exposes the existing specialized hasher implementations that were under the nightly-only specialization feature for manual use. To support this, also adds BuildAHasherU64, BuildAHasherFixed, and BuildAHasherStr types that can be used in places that expect BuildHashers and which leverage existing infrastructure to create these specialized versions of hashers.

Copy link
Owner

@tkaitchuck tkaitchuck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be good to have a sanity test to make sure these instantiate and can be used as expected. Perhaps a executable doc example.

Based on the compiler error, it looks like there is also a missing use statement somewhere.

@@ -264,11 +266,13 @@ impl Hasher for AHasherU64 {
}
}

#[cfg(feature = "specialize")]
pub(crate) struct AHasherFixed(pub AHasher);

/// A specialized hasher for fixed size primitives larger than 64 bits.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't have to be strictly primitives, just fixed in size. So for example a (u128, u128) would be ok with this. It just hasn't been used that way now as I don't have a way to detect such arbitrary structures.

@@ -213,14 +213,16 @@ impl Hasher for AHasher {
}
}

#[cfg(feature = "specialize")]
pub(crate) struct AHasherU64 {
/// A specialized hasher for only primitives <= 64 bits.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If these are public, they should have a bit more docs. It is probably worth enumerating the types that this works with. (Notably usize is not one, as it can technically be 128 bits on some exotic systems)

@@ -311,12 +315,15 @@ impl Hasher for AHasherFixed {
}
}

#[cfg(feature = "specialize")]
pub(crate) struct AHasherStr(pub AHasher);
/// A specialized hasher for strings.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works for anything that can deref / cast to a [u8] so, String, str, slices of primitives etc.

@fu5ha
Copy link
Author

fu5ha commented May 24, 2023

Those all make sense to me! It will probably be a few weeks until I get around to it -- going on vacation soon :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants