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

use hashbrown in naga & main wgpu crates, etc. #6938

Open
wants to merge 22 commits into
base: trunk
Choose a base branch
from

Conversation

brody4hire
Copy link
Contributor

@brody4hire brody4hire commented Jan 17, 2025

Connections

Description

  • use hashbrown in naga & top-level wgpu crates
  • completely replace indirect usage of std::collections::HashMap via rustc_hash::FxHashMap in wgpu-hal
  • use hashbrown in main wgpu crate
  • use hashbrown in deno_webgpu
  • use hashbrown with default-features = false, as needed to support no-std build
  • other minor updates

POSSIBLE API IMPACT

Exported naga API with these updates would be exporting types & members using HashMap & HashSet from hashbrown rather than std::collections. I think this is a crucial part of supporting no-std. I don't expect the blast radius to be very high, just wanted to point this out to avoid potential surprises for anyone.

/cc @bushrat011899

Testing

  • cargo test --all-features -p naga
  • cargo test --all-features -p wgpu-core
  • cargo test --all-features -p wgpu-hal
  • cargo test --all-features -p wgpu
  • cargo xtask test

Checklist

  • Run cargo fmt.
  • Run taplo format.
  • Run cargo clippy. If applicable, add:
    • --target wasm32-unknown-unknown
    • --target wasm32-unknown-emscripten
  • Run cargo xtask test to run tests.
  • Add change to CHANGELOG.md. See simple instructions inside file.
  • Resolve or defer XXX todo items in these updates

@brody4hire brody4hire mentioned this pull request Jan 17, 2025
9 tasks
Comment on lines -170 to +173
match capabilities.iter().find(|cap| available.contains(cap)) {
match capabilities
.iter()
.find(|cap| available.contains::<spirv::Capability>(cap))
{
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If I use hashbrown::HashSet without this update, cargo clippy (and cargo test with all features) fails with this:

error[E0277]: the trait bound `&spirv::Capability: hashbrown::Equivalent<spirv::Capability>` is not satisfied
   --> naga/src/back/spv/writer.rs:172:60
    |
172 | ...                   .find(|cap| available.contains(cap))
    |                                             -------- ^^^ the trait `std::borrow::Borrow<&spirv::Capability>` is not implemented for `spirv::Capability`, which is required by `&spirv::Capability: hashbrown::Equivalent<spirv::Capability>`
    |                                             |
    |                                             required by a bound introduced by this call
    |
    = note: required for `&spirv::Capability` to implement `hashbrown::Equivalent<spirv::Capability>`
note: required by a bound in `hashbrown::HashSet::<T, S, A>::contains`
   --> /Users/cjb/.cargo/registry/src/index.crates.io-6f17d22bba15001f/hashbrown-0.15.2/src/set.rs:864:19
    |
862 |     pub fn contains<Q>(&self, value: &Q) -> bool
    |            -------- required by a bound in this associated function
863 |     where
864 |         Q: Hash + Equivalent<T> + ?Sized,
    |                   ^^^^^^^^^^^^^ required by this bound in `HashSet::<T, S, A>::contains`

For more information about this error, try `rustc --explain E0277`.

It took me a while to figure out that using <spirv::Capability> in the hashbrown::HashSet::contains function call would resolve this build issue.

Something about this does not feel 100% right to me, wondering why rustc couldn't handle this call without using the type parameter. Any ideas about this?

@brody4hire brody4hire changed the title use hashbrown in naga etc. use hashbrown in naga & main wgpu crates` etc. Jan 19, 2025
@brody4hire brody4hire changed the title use hashbrown in naga & main wgpu crates` etc. use hashbrown in naga & main wgpu crates etc. Jan 19, 2025
naga/Cargo.toml Outdated Show resolved Hide resolved
@brody4hire brody4hire marked this pull request as ready for review January 19, 2025 09:51
@brody4hire brody4hire requested review from a team and crowlKats as code owners January 19, 2025 09:51
@brody4hire brody4hire changed the title use hashbrown in naga & main wgpu crates etc. use hashbrown in naga & main wgpu crates, etc. Jan 19, 2025
@brody4hire brody4hire requested a review from Wumpf January 20, 2025 04:53
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.

2 participants