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

Remove hashbrown dependency. #514

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mstange
Copy link
Collaborator

@mstange mstange commented Nov 29, 2022

The std HashMap uses hashbrown these days, so I think this won't make anything slower. But I haven't tested it.

The std HashMap uses hashbrown these days, so I think this
won't make anything slower. But I haven't tested it.
@mstange mstange self-assigned this Nov 29, 2022
@calixteman
Copy link
Collaborator

There is a small difference between the hashing algorithm:

And if I remember correctly, I chose the second because it was slightly faster and because we don't care about HashDoS attacks.
Anyway, it's just for you information, and you're free to do whatever you want :).

@mstange mstange marked this pull request as draft November 29, 2022 20:12
@mstange mstange removed the request for review from gabrielesvelto November 29, 2022 20:14
@mstange
Copy link
Collaborator Author

mstange commented Nov 29, 2022

Ah, thanks for checking!

Then maybe we can use the regular HashMap with a different hasher, or even FxHashMap.

I'll do a speed comparison when I get a chance. For now we can just ignore this PR.
My main interest in removing the hashbrown dependency is so that we can stop getting dependabot PRs for it which would leave us with two or three copies of hashbrown.

@mstange mstange removed their assignment Nov 29, 2022
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.

None yet

2 participants