jonhoo / griddle

A HashMap variant that spreads resize load across inserts
Apache License 2.0
190 stars 7 forks source link

Work with hashbrown 0.12 #20

Closed PsiACE closed 7 months ago

PsiACE commented 2 years ago

hi @jonhoo , these are some of the changes needed to work with hashbrown 0.12.

Maybe consider releasing a griddle 0.6.0.

codecov-commenter commented 2 years ago

Codecov Report

Merging #20 (32f3fbe) into master (46f1940) will decrease coverage by 0.27%. The diff coverage is 100.00%.

Impacted Files Coverage Δ
src/map.rs 64.11% <100.00%> (ø)
src/raw/mod.rs 71.94% <100.00%> (+0.63%) :arrow_up:
src/external_trait_impls/rayon/helpers.rs 92.85% <0.00%> (-7.15%) :arrow_down:
src/set.rs 55.65% <0.00%> (-0.87%) :arrow_down:
src/external_trait_impls/serde.rs 0.00% <0.00%> (ø)
jonhoo commented 2 years ago

How come those two methods are now marked as unsafe?

PsiACE commented 2 years ago

How come those two methods are now marked as unsafe?

I think carry can be marked as safe. So I reverted the previous changes.

As for insert_no_grow, I think it should be the same as hashbrown. see https://github.com/rust-lang/hashbrown/pull/254

For performance reasons, this method assumes that there is sufficient capacity for the new element, and it misbehaves otherwise, breaking invariants or even segfaulting. The necessary conditions could be checked, but if we're to keep it lean, it should be unsafe, so the burden is on the caller to ensure capacity.

jonhoo commented 2 years ago

I think you're right. We should update the docs to reflect the new safety requirement though, and also document why we believe we uphold those requirements in the place where we call insert_no_grow.