From 32b8b5048313525c421c5dc3873da5865ceccc96 Mon Sep 17 00:00:00 2001 From: Jim Blandy Date: Wed, 20 Sep 2023 15:25:47 -0700 Subject: [PATCH 1/6] Use `FastIndexSet` for `UniqueArena`. --- src/arena.rs | 11 +++++------ src/compact/mod.rs | 6 +++--- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/src/arena.rs b/src/arena.rs index 1499cf1772..49b9328012 100644 --- a/src/arena.rs +++ b/src/arena.rs @@ -5,8 +5,7 @@ use std::{cmp::Ordering, fmt, hash, marker::PhantomData, num::NonZeroU32, ops}; /// the same size and representation as `Handle`. type Index = NonZeroU32; -use crate::Span; -use indexmap::set::IndexSet; +use crate::{FastIndexSet, Span}; #[derive(Clone, Copy, Debug, thiserror::Error, PartialEq)] #[error("Handle {index} of {kind} is either not present, or inaccessible yet")] @@ -516,11 +515,11 @@ mod tests { /// `UniqueArena` is `HashSet`-like. #[cfg_attr(feature = "clone", derive(Clone))] pub struct UniqueArena { - set: IndexSet, + set: FastIndexSet, /// Spans for the elements, indexed by handle. /// - /// The length of this vector is always equal to `set.len()`. `IndexSet` + /// The length of this vector is always equal to `set.len()`. `FastIndexSet` /// promises that its elements "are indexed in a compact range, without /// holes in the range 0..set.len()", so we can always use the indices /// returned by insertion as indices into this vector. @@ -532,7 +531,7 @@ impl UniqueArena { /// Create a new arena with no initial capacity allocated. pub fn new() -> Self { UniqueArena { - set: IndexSet::new(), + set: FastIndexSet::default(), #[cfg(feature = "span")] span_info: Vec::new(), } @@ -741,7 +740,7 @@ where where D: serde::Deserializer<'de>, { - let set = IndexSet::deserialize(deserializer)?; + let set = FastIndexSet::deserialize(deserializer)?; #[cfg(feature = "span")] let span_info = std::iter::repeat(Span::default()).take(set.len()).collect(); diff --git a/src/compact/mod.rs b/src/compact/mod.rs index e0de1fe971..137f3bbe30 100644 --- a/src/compact/mod.rs +++ b/src/compact/mod.rs @@ -95,9 +95,9 @@ pub fn compact(module: &mut crate::Module) { // Drop unused types from the type arena. // - // `IndexSet`s don't have an underlying Vec that we can steal, compact in - // place, and then rebuild the `IndexSet` from. So we have to rebuild the - // type arena from scratch. + // `FastIndexSet`s don't have an underlying Vec that we can + // steal, compact in place, and then rebuild the `FastIndexSet` + // from. So we have to rebuild the type arena from scratch. log::trace!("compacting types"); let mut new_types = arena::UniqueArena::new(); for (old_handle, mut ty, span) in module.types.drain_all() { From 952c5e0514a4a218e0cc8490f238367bcdc01618 Mon Sep 17 00:00:00 2001 From: Jim Blandy Date: Wed, 20 Sep 2023 15:34:26 -0700 Subject: [PATCH 2/6] Introduce `FastIndexMap` type alias, and use it for NamedExpressions. --- src/lib.rs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 92a0ba2ce2..b220872dd0 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -310,12 +310,13 @@ pub type FastHashSet = rustc_hash::FxHashSet; pub type FastIndexSet = indexmap::IndexSet>; +/// Insertion-order-preserving hash map (`IndexMap`), but with the same +/// hasher as `FastHashMap` (faster but not resilient to DoS attacks). +pub type FastIndexMap = + indexmap::IndexMap>; + /// Map of expressions that have associated variable names -pub(crate) type NamedExpressions = indexmap::IndexMap< - Handle, - String, - std::hash::BuildHasherDefault, ->; +pub(crate) type NamedExpressions = FastIndexMap, String>; /// Early fragment tests. /// From 629c3cd516b7fa4f823d9b5af2edbd2c60680966 Mon Sep 17 00:00:00 2001 From: Jim Blandy Date: Wed, 20 Sep 2023 15:36:03 -0700 Subject: [PATCH 3/6] [wgsl-in] Use FastIndexMap for temporary named expressions table. --- src/front/wgsl/lower/mod.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/front/wgsl/lower/mod.rs b/src/front/wgsl/lower/mod.rs index 36366d0138..56d8709708 100644 --- a/src/front/wgsl/lower/mod.rs +++ b/src/front/wgsl/lower/mod.rs @@ -6,8 +6,7 @@ use crate::front::wgsl::parse::number::Number; use crate::front::wgsl::parse::{ast, conv}; use crate::front::{Emitter, Typifier}; use crate::proc::{ensure_block_returns, Alignment, Layouter, ResolveContext, TypeResolution}; -use crate::{Arena, FastHashMap, Handle, Span}; -use indexmap::IndexMap; +use crate::{Arena, FastHashMap, FastIndexMap, Handle, Span}; mod construction; @@ -101,7 +100,7 @@ pub struct StatementContext<'source, 'temp, 'out> { naga_expressions: &'out mut Arena, /// Stores the names of expressions that are assigned in `let` statement /// Also stores the spans of the names, for use in errors. - named_expressions: &'out mut IndexMap, (String, Span)>, + named_expressions: &'out mut FastIndexMap, (String, Span)>, arguments: &'out [crate::FunctionArgument], module: &'out mut crate::Module, } @@ -915,7 +914,7 @@ impl<'source, 'temp> Lowerer<'source, 'temp> { let mut local_table = FastHashMap::default(); let mut local_variables = Arena::new(); let mut expressions = Arena::new(); - let mut named_expressions = IndexMap::default(); + let mut named_expressions = FastIndexMap::default(); let arguments = f .arguments From afd361e5d074fb779230b1157f41d2f06d2add65 Mon Sep 17 00:00:00 2001 From: Jim Blandy Date: Wed, 20 Sep 2023 15:38:46 -0700 Subject: [PATCH 4/6] Use `FastIndexMap` for `SpecialTypes::predeclared_types`. --- src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index b220872dd0..9351572892 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1996,7 +1996,7 @@ pub struct SpecialTypes { /// /// Call [`Module::generate_predeclared_type`] to populate this if /// needed and return the handle. - pub predeclared_types: indexmap::IndexMap>, + pub predeclared_types: FastIndexMap>, } /// Shader module. From e1049abfb00635ebb0698bf12ff87c04139d0764 Mon Sep 17 00:00:00 2001 From: Jim Blandy Date: Wed, 20 Sep 2023 15:40:28 -0700 Subject: [PATCH 5/6] [spv-out] Use `FastIndexMap` for `Frontend::switch_cases`. --- src/front/spv/mod.rs | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/front/spv/mod.rs b/src/front/spv/mod.rs index 36108fdd9b..5fd54b023f 100644 --- a/src/front/spv/mod.rs +++ b/src/front/spv/mod.rs @@ -40,7 +40,7 @@ use function::*; use crate::{ arena::{Arena, Handle, UniqueArena}, proc::{Alignment, Layouter}, - FastHashMap, FastHashSet, + FastHashMap, FastHashSet, FastIndexMap, }; use num_traits::cast::FromPrimitive; @@ -596,11 +596,7 @@ pub struct Frontend { /// use that target block id. /// /// Used to preserve allocations between instruction parsing. - switch_cases: indexmap::IndexMap< - spirv::Word, - (BodyIndex, Vec), - std::hash::BuildHasherDefault, - >, + switch_cases: FastIndexMap)>, /// Tracks access to gl_PerVertex's builtins, it is used to cull unused builtins since initializing those can /// affect performance and the mere presence of some of these builtins might cause backends to error since they @@ -641,7 +637,7 @@ impl> Frontend { dummy_functions: Arena::new(), function_call_graph: GraphMap::new(), options: options.clone(), - switch_cases: indexmap::IndexMap::default(), + switch_cases: FastIndexMap::default(), gl_per_vertex_builtin_access: FastHashSet::default(), } } From 2bd004d9bf3e73e8821d5347ac246020f6a27bb4 Mon Sep 17 00:00:00 2001 From: Jim Blandy Date: Wed, 20 Sep 2023 15:52:09 -0700 Subject: [PATCH 6/6] [wgsl-in] Use deterministic ordering for dependency ordering. Use `FastIndexSet`, rather than `FastHashSet`, for tracking global declarations' dependencies, so that the order in which functions are inserted into the `Module` is not dependent on the hash function. --- src/front/wgsl/parse/ast.rs | 4 ++-- src/front/wgsl/parse/mod.rs | 8 ++++---- tests/out/wgsl/module-scope.wgsl | 8 ++++---- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/front/wgsl/parse/ast.rs b/src/front/wgsl/parse/ast.rs index 34aeb21a1a..fca7457f05 100644 --- a/src/front/wgsl/parse/ast.rs +++ b/src/front/wgsl/parse/ast.rs @@ -1,5 +1,5 @@ use crate::front::wgsl::parse::number::Number; -use crate::{Arena, FastHashSet, Handle, Span}; +use crate::{Arena, FastIndexSet, Handle, Span}; use std::hash::Hash; #[derive(Debug, Default)] @@ -73,7 +73,7 @@ pub struct GlobalDecl<'a> { /// Names of all module-scope or predeclared objects this /// declaration uses. - pub dependencies: FastHashSet>, + pub dependencies: FastIndexSet>, } #[derive(Debug)] diff --git a/src/front/wgsl/parse/mod.rs b/src/front/wgsl/parse/mod.rs index 86dd12799d..325a8e690a 100644 --- a/src/front/wgsl/parse/mod.rs +++ b/src/front/wgsl/parse/mod.rs @@ -2,7 +2,7 @@ use crate::front::wgsl::error::{Error, ExpectedToken}; use crate::front::wgsl::parse::lexer::{Lexer, Token}; use crate::front::wgsl::parse::number::Number; use crate::front::SymbolTable; -use crate::{Arena, FastHashSet, Handle, ShaderStage, Span}; +use crate::{Arena, FastIndexSet, Handle, ShaderStage, Span}; pub mod ast; pub mod conv; @@ -51,7 +51,7 @@ struct ExpressionContext<'input, 'temp, 'out> { /// /// [`GlobalDecl`]: ast::GlobalDecl /// [`dependencies`]: ast::GlobalDecl::dependencies - unresolved: &'out mut FastHashSet>, + unresolved: &'out mut FastIndexSet>, } impl<'a> ExpressionContext<'a, '_, '_> { @@ -2076,7 +2076,7 @@ impl Parser { &mut self, lexer: &mut Lexer<'a>, out: &mut ast::TranslationUnit<'a>, - dependencies: &mut FastHashSet>, + dependencies: &mut FastIndexSet>, ) -> Result, Error<'a>> { self.push_rule_span(Rule::FunctionDecl, lexer); // read function name @@ -2238,7 +2238,7 @@ impl Parser { (None, None) => {} } - let mut dependencies = FastHashSet::default(); + let mut dependencies = FastIndexSet::default(); let mut ctx = ExpressionContext { expressions: &mut out.expressions, local_table: &mut SymbolTable::default(), diff --git a/tests/out/wgsl/module-scope.wgsl b/tests/out/wgsl/module-scope.wgsl index 141c28b500..3276a0a10a 100644 --- a/tests/out/wgsl/module-scope.wgsl +++ b/tests/out/wgsl/module-scope.wgsl @@ -9,14 +9,14 @@ var Texture: texture_2d; @group(0) @binding(1) var Sampler: sampler; -fn returns() -> S { - return S(Value); -} - fn statement() { return; } +fn returns() -> S { + return S(Value); +} + fn call() { statement(); let _e0 = returns();