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

std.hash_map: adding a rehash() method #19923

Merged
merged 2 commits into from
Aug 8, 2024
Merged

Conversation

mrjbq7
Copy link
Contributor

@mrjbq7 mrjbq7 commented May 10, 2024

Note: this resurrects the recently closed #17890.


This allows a highly fragmented HashMap to have tombstones removed as the values are all rehashed.

It would be nice to make this rehash() automatically, but that currently presents a challenge where it doesn't work with adapted contexts since the keys are not preserved in the map for re-hashing and the hash value is not stored currently, and the non-adapted contexts require a bit of additional book-keeping to check before calling rehash().

This is a partial fix for #17851, but requires the user to call rehash() periodically to get the benefit.

@mrjbq7 mrjbq7 force-pushed the rehash-again branch 2 times, most recently from 82a8478 to 24a1c54 Compare May 10, 2024 00:26
lib/std/hash_map.zig Outdated Show resolved Hide resolved
lib/std/hash_map.zig Outdated Show resolved Hide resolved
lib/std/hash_map.zig Outdated Show resolved Hide resolved
@mrjbq7
Copy link
Contributor Author

mrjbq7 commented May 10, 2024

@linusg thanks! fixed those 3 comments, and rebased on master again.

@mrjbq7 mrjbq7 force-pushed the rehash-again branch 2 times, most recently from ad2ebd0 to 51e0b86 Compare May 20, 2024 03:01
@mrjbq7
Copy link
Contributor Author

mrjbq7 commented Jun 6, 2024

I'll just keep it rebased on master until @andrewrk tells me he doesn't like this PR.

@tau-dev
Copy link
Contributor

tau-dev commented Jul 15, 2024

To mirror my comment in #17851: We can remove the user burden of calling rehash manually by requiring any adapter context to have an additional hashStored function capable of hashing non-adapted keys, which enables the HashMap to rehash automatically and maintain good performance by default.

@tau-dev
Copy link
Contributor

tau-dev commented Jul 23, 2024

After this is merged, I'd be happy to fix #7494 (if it turns out to still be a problem).

@andrewrk andrewrk merged commit a854ce3 into ziglang:master Aug 8, 2024
10 checks passed
@andrewrk andrewrk added standard library This issue involves writing Zig code for the standard library. release notes This PR should be mentioned in the release notes. labels Aug 8, 2024
igor84 pushed a commit to igor84/zig that referenced this pull request Aug 11, 2024
SammyJames pushed a commit to SammyJames/zig that referenced this pull request Aug 13, 2024
Rexicon226 pushed a commit to Rexicon226/zig that referenced this pull request Aug 13, 2024
richerfu pushed a commit to richerfu/zig that referenced this pull request Oct 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release notes This PR should be mentioned in the release notes. standard library This issue involves writing Zig code for the standard library.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants