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

Rewrite get_many_mut methods #367

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

JustForFun88
Copy link
Contributor

The previous form of the HashMap::get_many_mut_inner and HashMap::get_many_unchecked_mut_inner functions was not very optimal:

  • firstly, the old RawTable API was not very convenient and did not combine well with each other (get_mut requires that eq: impl FnMut(&T) -> bool, get_many_mut requires that eq argument should be a closure such that eq(i, k) returns true if k is equal to the ith key to be looked up.).
  • secondly, the hash of all keys was calculated, regardless of whether there are any of given keys in the table at all.

@Amanieu
Copy link
Member

Amanieu commented Oct 13, 2022

I'm not actually sure this is an improvement, have you benchmarked this? It adds a lot of branches in the generated code to handle lazy hash computation. Straight up computing the hashes and doing multiple lookups might result is faster code. In practice, this API is used when you have few keys (usually just 2) and you're already pretty sure all are in the table.

@bors
Copy link
Contributor

bors commented May 30, 2024

☔ The latest upstream changes (presumably #528) made this pull request unmergeable. Please resolve the merge conflicts.

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.

3 participants