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

Add function to support pre-hashed keys #491

Closed
wants to merge 1 commit into from
Closed

Add function to support pre-hashed keys #491

wants to merge 1 commit into from

Conversation

joaovbsevero
Copy link

@joaovbsevero joaovbsevero commented Dec 11, 2023

Motivation

I'm working on a monad type with expensive method resolution (hash being one of them). To speed up the hashmap insertion I pre-hash my inner value when it is known, build my monad and insert into the HashMap using this new method with the pre-generated hash. It gave me a speed up of about 15%.

Implementation

The implementation is straightforward, using the same code path as insert but with a given hash.
The use of mem::swap is necessary to ensure that the old value (if any) is dropped and proved to be faster on my benchmarks than ptr::write.

@joaovbsevero joaovbsevero changed the title Add function to support prehashed keys Add function to support pre-hashed keys Dec 11, 2023
@Amanieu
Copy link
Member

Amanieu commented Dec 11, 2023

Have you considered using the HashTable type instead? It is specifically designed for this kind of use case.

There is also the raw entry API which provides similar functionality, but that is deprecated in favor of HashTable.

@joaovbsevero
Copy link
Author

joaovbsevero commented Dec 11, 2023

Have you considered using the HashTable type instead? It is specifically designed for this kind of use case.

There is also the raw entry API which provides similar functionality, but that is deprecated in favor of HashTable.

I found the HashTable too low-level for what I'm doing, this Pull Request is just a suggestion, if considered not actually usefull I will just use my forked version keeping up-to-date with the library.

Edit 1.: the only method that I needed to have the pre-generated hash was the insert since I know its type beforehand.

@Amanieu
Copy link
Member

Amanieu commented Dec 11, 2023

This method is unsuitable because we don't want to expose custom hash values in HashMap/HashSet. This could cause unsoundness because it would allow constructing a HashMap<usize, usize> which has inconsistent behavior. This is relevant because no types with custom Hash impl are used: you should be able to trust that maps of standard types behave predictably.

This is why all functionality related to custom hashing was moved to HashTable. Even though it has a low-level API, you can easily build a wrapper on top of it for your use case.

@Amanieu Amanieu closed this Dec 11, 2023
@joaovbsevero
Copy link
Author

This method is unsuitable because we don't want to expose custom hash values in HashMap/HashSet. This could cause unsoundness because it would allow constructing a HashMap<usize, usize> which has inconsistent behavior. This is relevant because no types with custom Hash impl are used: you should be able to trust that maps of standard types behave predictably.

This is why all functionality related to custom hashing was moved to HashTable. Even though it has a low-level API, you can easily build a wrapper on top of it for your use case.

No problem, thanks for spending time looking into the PR 😄

@joaovbsevero joaovbsevero deleted the monad-prehash branch December 12, 2023 22:34
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