Skip to content

HashMap/HashSet migration guide #2065

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

Closed

Conversation

alice-i-cecile
Copy link
Member

@alice-i-cecile alice-i-cecile commented Apr 2, 2025

The PR that changed this upstream was rust-lang/hashbrown#563, which seems like a sensible choice for them.

We need to:

a) file an issue with hashbrown requesting changes to make it more feasible to use an alternate default hasher
b) file an issue on bevyengine/bevy to decide what to do for 0.16 and going forward

Closes #2045.

`HashMap::new` can be replaced with `HashMap::default`, which is flexible in the correct way.
Other methods, such as `with_capacity` may be replaced with the `with_capacity_and_hasher` method.

We are understand your frustrations with this regression, and are actively discussing solutions to improve this going forward.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Grammer is a bit funny.

@alice-i-cecile
Copy link
Member Author

FYI, I don't think we should merge this, and should instead solve the regression per the linked issue. I decided to write this up to lay out how gnarly the problem is and to have a fallback in case we decide to 🚀 anyways.

github-merge-queue bot pushed a commit to bevyengine/bevy that referenced this pull request Apr 6, 2025
# Objective

- Fixes #18690
- Closes [#2065](bevyengine/bevy-website#2065)
- Alternative to #18691

The changes to the Hash made in #15801 to the
[BuildHasher](https://doc.rust-lang.org/std/hash/trait.BuildHasher.html)
resulted in serious migration problems and downgraded UX for users of
Bevy's re-exported hashmaps. Once merged, we need to go in and remove
the migration guide added as part of #15801.

## Solution

- Newtype `HashMap` and `HashSet` instead of type aliases
- Added `Deref/Mut` to allow accessing future `hashbrown` methods
without maintenance from Bevy
- Added bidirectional `From` implementations to provide escape hatch for
API incompatibility
- Added inlinable re-exports of all methods directly to Bevy's types.
This ensures `HashMap::new()` works (since the `Deref` implementation
wont cover these kinds of invocations).

## Testing

- CI

---

## Migration Guide

- If you relied on Bevy's `HashMap` and/or `HashSet` types to be
identical to `hashbrown`, consider using `From` and `Into` to convert
between the `hashbrown` and Bevy types as required.
- If you relied on `hashbrown/serde` or `hashbrown/rayon` features, you
may need to enable `bevy_platform_support/serialize` and/or
`bevy_platform_support/rayon` respectively.

---

## Notes

- Did not replicate the Rayon traits, users will need to rely on the
`Deref/Mut` or `From` implementations for those methods.
- Did not re-expose the `unsafe` methods from `hashbrown`. In most cases
users will still have access via `Deref/Mut` anyway.
- I have added `inline` to all methods as they are trivial wrappings of
existing methods.
- I chose to make `HashMap::new` and `HashSet::new` const, which is
different to `hashbrown`. We can do this because we default to a
fixed-state build-hasher. Mild ergonomic win over using
`HashMap::with_hasher(FixedHasher)`.
mockersf pushed a commit to bevyengine/bevy that referenced this pull request Apr 8, 2025
# Objective

- Fixes #18690
- Closes [#2065](bevyengine/bevy-website#2065)
- Alternative to #18691

The changes to the Hash made in #15801 to the
[BuildHasher](https://doc.rust-lang.org/std/hash/trait.BuildHasher.html)
resulted in serious migration problems and downgraded UX for users of
Bevy's re-exported hashmaps. Once merged, we need to go in and remove
the migration guide added as part of #15801.

## Solution

- Newtype `HashMap` and `HashSet` instead of type aliases
- Added `Deref/Mut` to allow accessing future `hashbrown` methods
without maintenance from Bevy
- Added bidirectional `From` implementations to provide escape hatch for
API incompatibility
- Added inlinable re-exports of all methods directly to Bevy's types.
This ensures `HashMap::new()` works (since the `Deref` implementation
wont cover these kinds of invocations).

## Testing

- CI

---

## Migration Guide

- If you relied on Bevy's `HashMap` and/or `HashSet` types to be
identical to `hashbrown`, consider using `From` and `Into` to convert
between the `hashbrown` and Bevy types as required.
- If you relied on `hashbrown/serde` or `hashbrown/rayon` features, you
may need to enable `bevy_platform_support/serialize` and/or
`bevy_platform_support/rayon` respectively.

---

## Notes

- Did not replicate the Rayon traits, users will need to rely on the
`Deref/Mut` or `From` implementations for those methods.
- Did not re-expose the `unsafe` methods from `hashbrown`. In most cases
users will still have access via `Deref/Mut` anyway.
- I have added `inline` to all methods as they are trivial wrappings of
existing methods.
- I chose to make `HashMap::new` and `HashSet::new` const, which is
different to `hashbrown`. We can do this because we default to a
fixed-state build-hasher. Mild ergonomic win over using
`HashMap::with_hasher(FixedHasher)`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Mention that users need to change HashMap and HashSet initializations in 0.16
2 participants