From df54acc05dcaedf51be903604b40f888ee9b9b2a Mon Sep 17 00:00:00 2001 From: "@brody4hire - C. Jonathan Brody" Date: Mon, 27 Jan 2025 11:50:33 -0500 Subject: [PATCH] use `hashbrown` in more crates (etc.) (#6938) Co-authored-by: Connor Fitzgerald --- CHANGELOG.md | 8 ++----- Cargo.lock | 3 +++ Cargo.toml | 7 ++++-- clippy.toml | 8 +++++++ deno_webgpu/Cargo.toml | 1 + deno_webgpu/lib.rs | 2 +- deno_webgpu/pipeline.rs | 2 +- examples/features/Cargo.toml | 3 +++ lock-analyzer/Cargo.toml | 3 +++ naga/Cargo.toml | 10 ++++++-- naga/fuzz/Cargo.toml | 3 +++ naga/fuzz/fuzz_targets/glsl_parser.rs | 33 ++++++++++----------------- naga/src/back/glsl/mod.rs | 10 ++++---- naga/src/back/mod.rs | 2 +- naga/src/back/pipeline_constants.rs | 3 ++- naga/src/back/spv/recyclable.rs | 4 ++-- naga/src/back/spv/writer.rs | 8 +++++-- naga/src/lib.rs | 11 +++++++-- tests/Cargo.toml | 3 +++ wgpu-core/Cargo.toml | 2 +- wgpu-hal/src/dx12/sampler.rs | 2 +- wgpu-hal/src/gles/device.rs | 3 ++- wgpu-hal/src/metal/mod.rs | 5 ++-- wgpu-hal/src/vulkan/mod.rs | 12 ++++------ wgpu-info/Cargo.toml | 3 +++ wgpu/Cargo.toml | 1 + wgpu/src/api/common_pipeline.rs | 2 +- wgpu/src/backend/webgpu.rs | 2 +- 28 files changed, 96 insertions(+), 60 deletions(-) create mode 100644 clippy.toml diff --git a/CHANGELOG.md b/CHANGELOG.md index 944406e70c..d7f164f0d4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -50,20 +50,16 @@ Bottom level categories: #### General +- Use `hashbrown` to simplify no-std support. By Brody in [#6938](https://github.com/gfx-rs/wgpu/pull/6938) & [#6925](https://github.com/gfx-rs/wgpu/pull/6925). - If you use Binding Arrays in a bind group, you may not use Dynamic Offset Buffers or Uniform Buffers in that bind group. By @cwfitzgerald in [#6811](https://github.com/gfx-rs/wgpu/pull/6811) + ##### Refactored internal trace path parameter Refactored some functions to handle the internal trace path as a string to avoid possible issues with `no_std` support. By @brodycj in [#6924](https://github.com/gfx-rs/wgpu/pull/6924). -##### Start using `hashbrown` - -Use `hashbrown` in `wgpu-core`, `wgpu-hal` & `wgpu-info` to simplify no-std support. (This may help improve performance as well.) - -By @brodycj in [#6925](https://github.com/gfx-rs/wgpu/pull/6925). - #### Vulkan ##### HAL queue callback support diff --git a/Cargo.lock b/Cargo.lock index 86acc2d9be..507e2b1be8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1059,6 +1059,7 @@ name = "deno_webgpu" version = "0.146.0" dependencies = [ "deno_core", + "hashbrown", "raw-window-handle 0.6.2", "serde", "thiserror 2.0.11", @@ -2248,6 +2249,7 @@ dependencies = [ "codespan-reporting", "diff", "env_logger", + "hashbrown", "hexf-parse", "hlsl-snapshots", "indexmap", @@ -4392,6 +4394,7 @@ dependencies = [ "bitflags 2.8.0", "cfg_aliases 0.2.1", "document-features", + "hashbrown", "js-sys", "log", "naga", diff --git a/Cargo.toml b/Cargo.toml index 152828c2a8..01c5f4a77c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -45,6 +45,7 @@ default-members = [ [workspace.lints.clippy] manual_c_str_literals = "allow" ref_as_ptr = "warn" +# NOTE: disallowed-types is configured in other file: clippy.toml [workspace.package] edition = "2021" @@ -126,9 +127,11 @@ raw-window-handle = "0.6" rayon = "1" renderdoc-sys = "1.1.0" ron = "0.8" -# rustc-hash 2.0 is a completely different hasher with different performance characteristics +# NOTE: rustc-hash v2 is a completely different hasher with different performance characteristics +# see discussion here (including with some other alternatives): https://github.com/gfx-rs/wgpu/issues/6999 +# (using default-features = false to support no-std build, avoiding any extra features that may require std::collections) +rustc-hash = { version = "1", default-features = false } serde_json = "1.0.137" -rustc-hash = "1" serde = { version = "1", default-features = false } smallvec = "1" static_assertions = "1.1.0" diff --git a/clippy.toml b/clippy.toml new file mode 100644 index 0000000000..35f212da62 --- /dev/null +++ b/clippy.toml @@ -0,0 +1,8 @@ +# NOTE: Other global Clippy config is in top-level Cargo.toml. + +disallowed-types = [ + { path = "std::collections::HashMap", reason = "use hashbrown::HashMap instead" }, + { path = "std::collections::HashSet", reason = "use hashbrown::HashSet instead" }, + { path = "rustc_hash::FxHashMap", reason = "use hashbrown::HashMap instead" }, + { path = "rustc_hash::FxHashSet", reason = "use hashbrown::HashSet instead" }, +] diff --git a/deno_webgpu/Cargo.toml b/deno_webgpu/Cargo.toml index 49f8a8c85d..9a55242d1b 100644 --- a/deno_webgpu/Cargo.toml +++ b/deno_webgpu/Cargo.toml @@ -28,6 +28,7 @@ wgpu-core = { workspace = true, features = [ wgpu-types = { workspace = true, features = ["serde"] } deno_core.workspace = true +hashbrown = { workspace = true, features = ["serde"] } raw-window-handle = { workspace = true } serde = { workspace = true, features = ["derive"] } thiserror.workspace = true diff --git a/deno_webgpu/lib.rs b/deno_webgpu/lib.rs index b723fe3c71..d4c201bb09 100644 --- a/deno_webgpu/lib.rs +++ b/deno_webgpu/lib.rs @@ -6,11 +6,11 @@ use deno_core::op2; use deno_core::OpState; use deno_core::Resource; use deno_core::ResourceId; +use hashbrown::HashSet; use serde::Deserialize; use serde::Serialize; use std::borrow::Cow; use std::cell::RefCell; -use std::collections::HashSet; use std::rc::Rc; use error::WebGpuResult; diff --git a/deno_webgpu/pipeline.rs b/deno_webgpu/pipeline.rs index 0ab3c40262..c8ba5f16ff 100644 --- a/deno_webgpu/pipeline.rs +++ b/deno_webgpu/pipeline.rs @@ -5,10 +5,10 @@ use deno_core::op2; use deno_core::OpState; use deno_core::Resource; use deno_core::ResourceId; +use hashbrown::HashMap; use serde::Deserialize; use serde::Serialize; use std::borrow::Cow; -use std::collections::HashMap; use std::rc::Rc; use super::error::WebGpuError; diff --git a/examples/features/Cargo.toml b/examples/features/Cargo.toml index 1bef728f3d..53029bd343 100644 --- a/examples/features/Cargo.toml +++ b/examples/features/Cargo.toml @@ -74,3 +74,6 @@ web-sys = { workspace = true, features = [ [target.'cfg(target_arch = "wasm32")'.dev-dependencies] wasm-bindgen-test.workspace = true + +[lints.clippy] +disallowed_types = "allow" diff --git a/lock-analyzer/Cargo.toml b/lock-analyzer/Cargo.toml index 513e729162..d34aa3530a 100644 --- a/lock-analyzer/Cargo.toml +++ b/lock-analyzer/Cargo.toml @@ -17,3 +17,6 @@ anyhow.workspace = true [dependencies.serde] workspace = true features = ["default", "serde_derive"] + +[lints.clippy] +disallowed_types = "allow" diff --git a/naga/Cargo.toml b/naga/Cargo.toml index 2c8d846289..a207e5b92b 100644 --- a/naga/Cargo.toml +++ b/naga/Cargo.toml @@ -41,8 +41,13 @@ msl-out = [] ## If you want to enable MSL output it regardless of the target platform, use `naga/msl-out`. msl-out-if-target-apple = [] -serialize = ["dep:serde", "bitflags/serde", "indexmap/serde"] -deserialize = ["dep:serde", "bitflags/serde", "indexmap/serde"] +serialize = ["dep:serde", "bitflags/serde", "hashbrown/serde", "indexmap/serde"] +deserialize = [ + "dep:serde", + "bitflags/serde", + "hashbrown/serde", + "indexmap/serde", +] arbitrary = ["dep:arbitrary", "bitflags/arbitrary", "indexmap/arbitrary"] spv-in = ["dep:petgraph", "dep:spirv"] spv-out = ["dep:spirv"] @@ -72,6 +77,7 @@ termcolor = { version = "1.4.1" } # termcolor minimum version was wrong and was fixed in # https://github.com/brendanzab/codespan/commit/e99c867339a877731437e7ee6a903a3d03b5439e codespan-reporting = { version = "0.11.0" } +hashbrown.workspace = true rustc-hash.workspace = true indexmap.workspace = true log = "0.4" diff --git a/naga/fuzz/Cargo.toml b/naga/fuzz/Cargo.toml index cd6c62488e..5d8647f19c 100644 --- a/naga/fuzz/Cargo.toml +++ b/naga/fuzz/Cargo.toml @@ -50,3 +50,6 @@ path = "fuzz_targets/ir.rs" bench = false test = false doc = false + +[lints.clippy] +disallowed_types = "allow" diff --git a/naga/fuzz/fuzz_targets/glsl_parser.rs b/naga/fuzz/fuzz_targets/glsl_parser.rs index 97a6ae3fbf..103a844193 100644 --- a/naga/fuzz/fuzz_targets/glsl_parser.rs +++ b/naga/fuzz/fuzz_targets/glsl_parser.rs @@ -2,6 +2,8 @@ #[cfg(enable_fuzzing)] mod fuzz { + use std::iter::FromIterator; + use arbitrary::Arbitrary; use libfuzzer_sys::fuzz_target; use naga::{ @@ -9,34 +11,23 @@ mod fuzz { FastHashMap, ShaderStage, }; - #[derive(Debug, Arbitrary)] - enum ShaderStageProxy { - Vertex, - Fragment, - Compute, - } - - impl From for ShaderStage { - fn from(proxy: ShaderStageProxy) -> Self { - match proxy { - ShaderStageProxy::Vertex => ShaderStage::Vertex, - ShaderStageProxy::Fragment => ShaderStage::Fragment, - ShaderStageProxy::Compute => ShaderStage::Compute, - } - } - } - #[derive(Debug, Arbitrary)] struct OptionsProxy { - pub stage: ShaderStageProxy, - pub defines: FastHashMap, + pub stage: ShaderStage, + pub defines: std::collections::HashMap, } impl From for Options { fn from(proxy: OptionsProxy) -> Self { Options { - stage: proxy.stage.into(), - defines: proxy.defines, + stage: proxy.stage, + // NOTE: This is a workaround needed due to lack of rust-fuzz/arbitrary support for hashbrown. + defines: FastHashMap::from_iter( + proxy + .defines + .keys() + .map(|k| (k.clone(), proxy.defines.get(&k.clone()).unwrap().clone())), + ), } } } diff --git a/naga/src/back/glsl/mod.rs b/naga/src/back/glsl/mod.rs index b058ae5ee8..0798fac82d 100644 --- a/naga/src/back/glsl/mod.rs +++ b/naga/src/back/glsl/mod.rs @@ -51,6 +51,7 @@ use crate::{ valid, Handle, ShaderStage, TypeInner, }; use features::FeaturesManager; +use hashbrown::hash_map; use std::{ cmp::Ordering, fmt::{self, Error as FmtError, Write}, @@ -4608,7 +4609,6 @@ impl<'a, W: Write> Writer<'a, W> { /// Helper method used to produce the reflection info that's returned to the user fn collect_reflection_info(&mut self) -> Result { - use std::collections::hash_map::Entry; let info = self.info.get_entry_point(self.entry_point_idx as usize); let mut texture_mapping = crate::FastHashMap::default(); let mut uniforms = crate::FastHashMap::default(); @@ -4617,13 +4617,13 @@ impl<'a, W: Write> Writer<'a, W> { let tex_name = self.reflection_names_globals[&sampling.image].clone(); match texture_mapping.entry(tex_name) { - Entry::Vacant(v) => { + hash_map::Entry::Vacant(v) => { v.insert(TextureMapping { texture: sampling.image, sampler: Some(sampling.sampler), }); } - Entry::Occupied(e) => { + hash_map::Entry::Occupied(e) => { if e.get().sampler != Some(sampling.sampler) { log::error!("Conflicting samplers for {}", e.key()); return Err(Error::ImageMultipleSamplers); @@ -4641,13 +4641,13 @@ impl<'a, W: Write> Writer<'a, W> { TypeInner::Image { .. } => { let tex_name = self.reflection_names_globals[&handle].clone(); match texture_mapping.entry(tex_name) { - Entry::Vacant(v) => { + hash_map::Entry::Vacant(v) => { v.insert(TextureMapping { texture: handle, sampler: None, }); } - Entry::Occupied(_) => { + hash_map::Entry::Occupied(_) => { // already used with a sampler, do nothing } } diff --git a/naga/src/back/mod.rs b/naga/src/back/mod.rs index 58c7fa02cb..e839853008 100644 --- a/naga/src/back/mod.rs +++ b/naga/src/back/mod.rs @@ -55,7 +55,7 @@ impl std::fmt::Display for Baked { /// the key must be the constant's identifier name. /// /// The value may represent any of WGSL's concrete scalar types. -pub type PipelineConstants = std::collections::HashMap; +pub type PipelineConstants = hashbrown::HashMap; /// Indentation level. #[derive(Clone, Copy)] diff --git a/naga/src/back/pipeline_constants.rs b/naga/src/back/pipeline_constants.rs index 420b5acf4f..bb9fb7f448 100644 --- a/naga/src/back/pipeline_constants.rs +++ b/naga/src/back/pipeline_constants.rs @@ -6,7 +6,8 @@ use crate::{ Arena, Block, Constant, Expression, Function, Handle, Literal, Module, Override, Range, Scalar, Span, Statement, TypeInner, WithSpan, }; -use std::{borrow::Cow, collections::HashSet, mem}; +use hashbrown::HashSet; +use std::{borrow::Cow, mem}; use thiserror::Error; #[derive(Error, Debug, Clone)] diff --git a/naga/src/back/spv/recyclable.rs b/naga/src/back/spv/recyclable.rs index 7e7ad5d817..8ccc7406e2 100644 --- a/naga/src/back/spv/recyclable.rs +++ b/naga/src/back/spv/recyclable.rs @@ -38,14 +38,14 @@ impl Recyclable for Vec { } } -impl Recyclable for std::collections::HashMap { +impl Recyclable for hashbrown::HashMap { fn recycle(mut self) -> Self { self.clear(); self } } -impl Recyclable for std::collections::HashSet { +impl Recyclable for hashbrown::HashSet { fn recycle(mut self) -> Self { self.clear(); self diff --git a/naga/src/back/spv/writer.rs b/naga/src/back/spv/writer.rs index 0361b5a213..a8bac54052 100644 --- a/naga/src/back/spv/writer.rs +++ b/naga/src/back/spv/writer.rs @@ -12,8 +12,8 @@ use crate::{ proc::{Alignment, TypeResolution}, valid::{FunctionInfo, ModuleInfo}, }; +use hashbrown::hash_map::Entry; use spirv::Word; -use std::collections::hash_map::Entry; struct FunctionInterface<'a> { varying_ids: &'a mut Vec, @@ -167,7 +167,11 @@ impl Writer { let selected = match self.capabilities_available { None => first, Some(ref available) => { - match capabilities.iter().find(|cap| available.contains(cap)) { + match capabilities + .iter() + // need explicit type for hashbrown::HashSet::contains fn call to keep rustc happy + .find(|cap| available.contains::(cap)) + { Some(&cap) => cap, None => { return Err(Error::MissingCapabilities(what, capabilities.to_vec())) diff --git a/naga/src/lib.rs b/naga/src/lib.rs index 2a145ed8f6..04f3f52bba 100644 --- a/naga/src/lib.rs +++ b/naga/src/lib.rs @@ -283,9 +283,15 @@ pub const BOOL_WIDTH: Bytes = 1; pub const ABSTRACT_WIDTH: Bytes = 8; /// Hash map that is faster but not resilient to DoS attacks. -pub type FastHashMap = rustc_hash::FxHashMap; +/// (Similar to rustc_hash::FxHashMap but using hashbrown::HashMap instead of std::collections::HashMap.) +/// To construct a new instance: `FastHashMap::default()` +pub type FastHashMap = + hashbrown::HashMap>; + /// Hash set that is faster but not resilient to DoS attacks. -pub type FastHashSet = rustc_hash::FxHashSet; +/// (Similar to rustc_hash::FxHashSet but using hashbrown::HashSet instead of std::collections::HashMap.) +pub type FastHashSet = + hashbrown::HashSet>; /// Insertion-order-preserving hash set (`IndexSet`), but with the same /// hasher as `FastHashSet` (faster but not resilient to DoS attacks). @@ -325,6 +331,7 @@ pub(crate) type NamedExpressions = FastIndexMap, String>; pub struct EarlyDepthTest { pub conservative: Option, } + /// Enables adjusting depth without disabling early Z. /// /// To use in a shader: diff --git a/tests/Cargo.toml b/tests/Cargo.toml index 863eec68b2..af37a472a1 100644 --- a/tests/Cargo.toml +++ b/tests/Cargo.toml @@ -70,3 +70,6 @@ wasm-bindgen-futures.workspace = true wasm-bindgen-test.workspace = true wasm-bindgen.workspace = true web-sys = { workspace = true, features = ["CanvasRenderingContext2d", "Blob"] } + +[lints.clippy] +disallowed_types = "allow" diff --git a/wgpu-core/Cargo.toml b/wgpu-core/Cargo.toml index 9269bb1140..010350554c 100644 --- a/wgpu-core/Cargo.toml +++ b/wgpu-core/Cargo.toml @@ -60,7 +60,7 @@ strict_asserts = ["wgpu-types/strict_asserts"] indirect-validation = ["naga/wgsl-in"] ## Enables serialization via `serde` on common wgpu types. -serde = ["dep:serde", "wgpu-types/serde", "arrayvec/serde"] +serde = ["dep:serde", "wgpu-types/serde", "arrayvec/serde", "hashbrown/serde"] ## Enable API tracing. trace = ["dep:ron", "serde", "naga/serialize"] diff --git a/wgpu-hal/src/dx12/sampler.rs b/wgpu-hal/src/dx12/sampler.rs index cc64c791ba..f49555391f 100644 --- a/wgpu-hal/src/dx12/sampler.rs +++ b/wgpu-hal/src/dx12/sampler.rs @@ -2,7 +2,7 @@ //! //! Nearly identical to the Vulkan sampler cache, with added descriptor heap management. -use std::collections::{hash_map::Entry, HashMap}; +use hashbrown::{hash_map::Entry, HashMap}; use ordered_float::OrderedFloat; use parking_lot::Mutex; diff --git a/wgpu-hal/src/gles/device.rs b/wgpu-hal/src/gles/device.rs index 1990acb849..a9d6209afb 100644 --- a/wgpu-hal/src/gles/device.rs +++ b/wgpu-hal/src/gles/device.rs @@ -1,6 +1,7 @@ use super::{conv, PrivateCapabilities}; use crate::auxil::map_naga_stage; use glow::HasContext; +use naga::FastHashMap; use std::{ cmp::max, convert::TryInto, @@ -16,7 +17,7 @@ type ShaderStage<'a> = ( naga::ShaderStage, &'a crate::ProgrammableStage<'a, super::ShaderModule>, ); -type NameBindingMap = rustc_hash::FxHashMap; +type NameBindingMap = FastHashMap; struct CompilationContext<'a> { layout: &'a super::PipelineLayout, diff --git a/wgpu-hal/src/metal/mod.rs b/wgpu-hal/src/metal/mod.rs index cd1136a3b4..8a3ff41bbe 100644 --- a/wgpu-hal/src/metal/mod.rs +++ b/wgpu-hal/src/metal/mod.rs @@ -36,6 +36,7 @@ use arrayvec::ArrayVec; use bitflags::bitflags; use hashbrown::HashMap; use metal::foreign_types::ForeignTypeRef as _; +use naga::FastHashMap; use parking_lot::{Mutex, RwLock}; #[derive(Clone, Debug)] @@ -937,9 +938,9 @@ struct CommandState { /// See `device::CompiledShader::sized_bindings` for more details. /// /// [`ResourceBinding`]: naga::ResourceBinding - storage_buffer_length_map: rustc_hash::FxHashMap, + storage_buffer_length_map: FastHashMap, - vertex_buffer_size_map: rustc_hash::FxHashMap, + vertex_buffer_size_map: FastHashMap, work_group_memory_sizes: Vec, push_constants: Vec, diff --git a/wgpu-hal/src/vulkan/mod.rs b/wgpu-hal/src/vulkan/mod.rs index 23f6422d8c..4421d0a5ef 100644 --- a/wgpu-hal/src/vulkan/mod.rs +++ b/wgpu-hal/src/vulkan/mod.rs @@ -42,17 +42,15 @@ use std::{ use arrayvec::ArrayVec; use ash::{ext, khr, vk}; -use hashbrown::{HashMap, HashSet}; +use hashbrown::HashSet; use parking_lot::{Mutex, RwLock}; -use rustc_hash::FxHasher; + +use naga::FastHashMap; use wgt::InternalCounter; const MILLIS_TO_NANOS: u64 = 1_000_000; const MAX_TOTAL_ATTACHMENTS: usize = crate::MAX_COLOR_ATTACHMENTS * 2 + 1; -// NOTE: This type alias is similar to rustc_hash::FxHashMap but works with hashbrown. -type FxHashMap = HashMap>; - #[derive(Clone, Debug)] pub struct Api; @@ -646,8 +644,8 @@ struct DeviceShared { private_caps: PrivateCapabilities, workarounds: Workarounds, features: wgt::Features, - render_passes: Mutex>, - framebuffers: Mutex>, + render_passes: Mutex>, + framebuffers: Mutex>, sampler_cache: Mutex, memory_allocations_counter: InternalCounter, } diff --git a/wgpu-info/Cargo.toml b/wgpu-info/Cargo.toml index 9deb4603e2..a287887a95 100644 --- a/wgpu-info/Cargo.toml +++ b/wgpu-info/Cargo.toml @@ -18,3 +18,6 @@ pico-args.workspace = true serde = { workspace = true, features = ["default"] } serde_json.workspace = true wgpu.workspace = true + +[lints.clippy] +disallowed_types = "allow" diff --git a/wgpu/Cargo.toml b/wgpu/Cargo.toml index 5f70401579..cbe0435d47 100644 --- a/wgpu/Cargo.toml +++ b/wgpu/Cargo.toml @@ -140,6 +140,7 @@ wgpu-types = { workspace = true, features = ["serde"] } arrayvec.workspace = true bitflags.workspace = true document-features.workspace = true +hashbrown.workspace = true log.workspace = true parking_lot.workspace = true profiling.workspace = true diff --git a/wgpu/src/api/common_pipeline.rs b/wgpu/src/api/common_pipeline.rs index af4d8d4b0c..7f07231f9d 100644 --- a/wgpu/src/api/common_pipeline.rs +++ b/wgpu/src/api/common_pipeline.rs @@ -1,4 +1,4 @@ -use std::collections::HashMap; +use hashbrown::HashMap; use crate::*; diff --git a/wgpu/src/backend/webgpu.rs b/wgpu/src/backend/webgpu.rs index 82b5f06f19..154f01f02a 100644 --- a/wgpu/src/backend/webgpu.rs +++ b/wgpu/src/backend/webgpu.rs @@ -5,10 +5,10 @@ mod ext_bindings; #[allow(clippy::allow_attributes)] mod webgpu_sys; +use hashbrown::HashMap; use js_sys::Promise; use std::{ cell::RefCell, - collections::HashMap, fmt, future::Future, ops::Range,