From 2736657dcd6bce963bccaec7b4e8e35158b74e78 Mon Sep 17 00:00:00 2001 From: Boshen <1430279+Boshen@users.noreply.github.com> Date: Fri, 20 Dec 2024 15:18:16 +0000 Subject: [PATCH] perf(semantic): allocate `UnresolvedReferences` in allocator (#8046) --- .../src/rules/eslint/no_global_assign.rs | 2 +- .../src/rules/jest/no_jasmine_globals.rs | 2 +- crates/oxc_linter/src/utils/jest.rs | 2 +- crates/oxc_semantic/src/builder.rs | 9 +-- crates/oxc_semantic/src/scope.rs | 63 +++++++++++++------ crates/oxc_transformer/src/jsx/jsx_impl.rs | 3 +- crates/oxc_transformer/src/jsx/refresh.rs | 6 +- .../src/plugins/inject_global_variables.rs | 2 +- .../oxc_transformer/src/typescript/module.rs | 6 +- crates/oxc_traverse/src/context/mod.rs | 12 ++-- crates/oxc_traverse/src/context/scoping.rs | 14 ++--- tasks/transform_checker/src/lib.rs | 12 +++- 12 files changed, 77 insertions(+), 56 deletions(-) diff --git a/crates/oxc_linter/src/rules/eslint/no_global_assign.rs b/crates/oxc_linter/src/rules/eslint/no_global_assign.rs index 9ade4716f3bf7..d65b80fc332b1 100644 --- a/crates/oxc_linter/src/rules/eslint/no_global_assign.rs +++ b/crates/oxc_linter/src/rules/eslint/no_global_assign.rs @@ -63,7 +63,7 @@ impl Rule for NoGlobalAssign { for &reference_id in reference_id_list { let reference = symbol_table.get_reference(reference_id); if reference.is_write() - && !self.excludes.contains(name) + && !self.excludes.iter().any(|n| n == name) && ctx.env_contains_var(name) { ctx.diagnostic(no_global_assign_diagnostic( diff --git a/crates/oxc_linter/src/rules/jest/no_jasmine_globals.rs b/crates/oxc_linter/src/rules/jest/no_jasmine_globals.rs index 7085d04c7ac81..8eda042bd714c 100644 --- a/crates/oxc_linter/src/rules/jest/no_jasmine_globals.rs +++ b/crates/oxc_linter/src/rules/jest/no_jasmine_globals.rs @@ -49,7 +49,7 @@ impl Rule for NoJasmineGlobals { .scopes() .root_unresolved_references() .iter() - .filter(|(key, _)| NON_JASMINE_PROPERTY_NAMES.contains(&key.as_str())); + .filter(|(key, _)| NON_JASMINE_PROPERTY_NAMES.contains(key)); for (name, reference_ids) in jasmine_references { for &reference_id in reference_ids { diff --git a/crates/oxc_linter/src/utils/jest.rs b/crates/oxc_linter/src/utils/jest.rs index 22b1dc363b48b..9ae9b6a6b1e37 100644 --- a/crates/oxc_linter/src/utils/jest.rs +++ b/crates/oxc_linter/src/utils/jest.rs @@ -250,7 +250,7 @@ fn collect_ids_referenced_to_global<'c>( .scopes() .root_unresolved_references() .iter() - .filter(|(name, _)| JEST_METHOD_NAMES.contains(name.as_str())) + .filter(|(name, _)| JEST_METHOD_NAMES.contains(name)) .flat_map(|(_, reference_ids)| reference_ids.iter().copied()) } diff --git a/crates/oxc_semantic/src/builder.rs b/crates/oxc_semantic/src/builder.rs index edd6875b2c8db..b1959b9f839f9 100644 --- a/crates/oxc_semantic/src/builder.rs +++ b/crates/oxc_semantic/src/builder.rs @@ -279,12 +279,9 @@ impl<'a> SemanticBuilder<'a> { if self.check_syntax_error && !self.source_type.is_typescript() { checker::check_unresolved_exports(&self); } - self.scope.root_unresolved_references = self - .unresolved_references - .into_root() - .into_iter() - .map(|(k, v)| (k.into(), v)) - .collect(); + self.scope.set_root_unresolved_references( + self.unresolved_references.into_root().into_iter().map(|(k, v)| (k.as_str(), v)), + ); let jsdoc = if self.build_jsdoc { self.jsdoc.build() } else { JSDocFinder::default() }; diff --git a/crates/oxc_semantic/src/scope.rs b/crates/oxc_semantic/src/scope.rs index 86e47d9c36e04..808898db5ad30 100644 --- a/crates/oxc_semantic/src/scope.rs +++ b/crates/oxc_semantic/src/scope.rs @@ -1,10 +1,9 @@ use std::{fmt, mem}; -use rustc_hash::{FxBuildHasher, FxHashMap}; +use rustc_hash::FxBuildHasher; use oxc_allocator::{Allocator, Vec as ArenaVec}; use oxc_index::{Idx, IndexVec}; -use oxc_span::CompactStr; use oxc_syntax::{ node::NodeId, reference::ReferenceId, @@ -13,7 +12,8 @@ use oxc_syntax::{ }; pub(crate) type Bindings<'a> = hashbrown::HashMap<&'a str, SymbolId, FxBuildHasher, &'a Allocator>; -pub type UnresolvedReferences = FxHashMap>; +pub type UnresolvedReferences<'a> = + hashbrown::HashMap<&'a str, ArenaVec<'a, ReferenceId>, FxBuildHasher, &'a Allocator>; /// Scope Tree /// @@ -37,8 +37,6 @@ pub struct ScopeTree { flags: IndexVec, - pub(crate) root_unresolved_references: UnresolvedReferences, - pub(crate) cell: ScopeTreeCell, } @@ -55,10 +53,13 @@ impl Default for ScopeTree { build_child_ids: false, node_ids: IndexVec::new(), flags: IndexVec::new(), - root_unresolved_references: UnresolvedReferences::default(), cell: ScopeTreeCell::new(Allocator::default(), |allocator| ScopeTreeInner { bindings: IndexVec::new(), child_ids: ArenaVec::new_in(allocator), + root_unresolved_references: UnresolvedReferences::with_hasher_in( + FxBuildHasher, + allocator, + ), }), } } @@ -80,6 +81,8 @@ pub(crate) struct ScopeTreeInner<'cell> { /// Maps a scope to direct children scopes. child_ids: ArenaVec<'cell, ArenaVec<'cell, ScopeId>>, + + pub(crate) root_unresolved_references: UnresolvedReferences<'cell>, } impl ScopeTree { @@ -131,13 +134,28 @@ impl ScopeTree { #[inline] pub fn root_unresolved_references(&self) -> &UnresolvedReferences { - &self.root_unresolved_references + &self.cell.borrow_dependent().root_unresolved_references } pub fn root_unresolved_references_ids( &self, ) -> impl Iterator + '_> + '_ { - self.root_unresolved_references.values().map(|v| v.iter().copied()) + self.cell.borrow_dependent().root_unresolved_references.values().map(|v| v.iter().copied()) + } + + pub(crate) fn set_root_unresolved_references<'a>( + &mut self, + entries: impl Iterator)>, + ) { + self.cell.with_dependent_mut(|allocator, inner| { + for (k, v) in entries { + let k = allocator.alloc_str(k); + let v = ArenaVec::from_iter_in(v, allocator); + inner.root_unresolved_references.insert(k, v); + } + // = + // .extend_from(entries.map(|(k, v)| (allocator.alloc(k),))) + }); } /// Delete an unresolved reference. @@ -153,14 +171,16 @@ impl ScopeTree { // but `map.entry` requires an owned key to be provided. Currently we use `CompactStr`s as keys // which are not cheap to construct, so this is best we can do at present. // TODO: Switch to `Entry` API once we use `&str`s or `Atom`s as keys. - let reference_ids = self.root_unresolved_references.get_mut(name).unwrap(); - if reference_ids.len() == 1 { - assert!(reference_ids[0] == reference_id); - self.root_unresolved_references.remove(name); - } else { - let index = reference_ids.iter().position(|&id| id == reference_id).unwrap(); - reference_ids.swap_remove(index); - } + self.cell.with_dependent_mut(|_allocator, inner| { + let reference_ids = inner.root_unresolved_references.get_mut(name).unwrap(); + if reference_ids.len() == 1 { + assert!(reference_ids[0] == reference_id); + inner.root_unresolved_references.remove(name); + } else { + let index = reference_ids.iter().position(|&id| id == reference_id).unwrap(); + reference_ids.swap_remove(index); + } + }); } #[inline] @@ -234,8 +254,15 @@ impl ScopeTree { self.get_binding(self.root_scope_id(), name) } - pub fn add_root_unresolved_reference(&mut self, name: CompactStr, reference_id: ReferenceId) { - self.root_unresolved_references.entry(name).or_default().push(reference_id); + pub fn add_root_unresolved_reference(&mut self, name: &str, reference_id: ReferenceId) { + self.cell.with_dependent_mut(|allocator, inner| { + let name = allocator.alloc_str(name); + inner + .root_unresolved_references + .entry(name) + .or_insert_with(|| ArenaVec::new_in(allocator)) + .push(reference_id); + }); } /// Check if a symbol is declared in a certain scope. diff --git a/crates/oxc_transformer/src/jsx/jsx_impl.rs b/crates/oxc_transformer/src/jsx/jsx_impl.rs index 5338a9e79b744..b90aff146e515 100644 --- a/crates/oxc_transformer/src/jsx/jsx_impl.rs +++ b/crates/oxc_transformer/src/jsx/jsx_impl.rs @@ -1081,8 +1081,7 @@ fn get_read_identifier_reference<'a>( name: Atom<'a>, ctx: &mut TraverseCtx<'a>, ) -> Expression<'a> { - let reference_id = - ctx.create_reference_in_current_scope(name.to_compact_str(), ReferenceFlags::Read); + let reference_id = ctx.create_reference_in_current_scope(name.as_str(), ReferenceFlags::Read); let ident = ctx.ast.alloc_identifier_reference_with_reference_id(span, name, reference_id); Expression::Identifier(ident) } diff --git a/crates/oxc_transformer/src/jsx/refresh.rs b/crates/oxc_transformer/src/jsx/refresh.rs index bdee87debfe35..676213a9634ff 100644 --- a/crates/oxc_transformer/src/jsx/refresh.rs +++ b/crates/oxc_transformer/src/jsx/refresh.rs @@ -68,8 +68,7 @@ impl<'a> RefreshIdentifierResolver<'a> { pub fn to_expression(&self, ctx: &mut TraverseCtx<'a>) -> Expression<'a> { match self { Self::Identifier(ident) => { - let reference_id = - ctx.create_unbound_reference(ident.name.to_compact_str(), ReferenceFlags::Read); + let reference_id = ctx.create_unbound_reference(&ident.name, ReferenceFlags::Read); Expression::Identifier(ctx.ast.alloc_identifier_reference_with_reference_id( ident.span, ident.name.clone(), @@ -77,8 +76,7 @@ impl<'a> RefreshIdentifierResolver<'a> { )) } Self::Member((ident, property)) => { - let reference_id = - ctx.create_unbound_reference(ident.name.to_compact_str(), ReferenceFlags::Read); + let reference_id = ctx.create_unbound_reference(&ident.name, ReferenceFlags::Read); let ident = Expression::Identifier(ctx.ast.alloc_identifier_reference_with_reference_id( ident.span, diff --git a/crates/oxc_transformer/src/plugins/inject_global_variables.rs b/crates/oxc_transformer/src/plugins/inject_global_variables.rs index 7243d819d82c6..3a77614e479ed 100644 --- a/crates/oxc_transformer/src/plugins/inject_global_variables.rs +++ b/crates/oxc_transformer/src/plugins/inject_global_variables.rs @@ -181,7 +181,7 @@ impl<'a> InjectGlobalVariables<'a> { } else if self.replaced_dot_defines.iter().any(|d| d.0 == i.specifier.local()) { false } else { - scopes.root_unresolved_references().contains_key(i.specifier.local()) + scopes.root_unresolved_references().contains_key(i.specifier.local().as_str()) } }) .cloned() diff --git a/crates/oxc_transformer/src/typescript/module.rs b/crates/oxc_transformer/src/typescript/module.rs index e9c4151d93927..f0e05b58b7d0f 100644 --- a/crates/oxc_transformer/src/typescript/module.rs +++ b/crates/oxc_transformer/src/typescript/module.rs @@ -1,5 +1,5 @@ use oxc_ast::{ast::*, NONE}; -use oxc_span::{CompactStr, SPAN}; +use oxc_span::SPAN; use oxc_syntax::reference::ReferenceFlags; use oxc_traverse::{Traverse, TraverseCtx}; @@ -58,8 +58,8 @@ impl<'a, 'ctx> TypeScriptModule<'a, 'ctx> { // module.exports let module_exports = { - let reference_id = ctx - .create_reference_in_current_scope(CompactStr::new("module"), ReferenceFlags::Read); + let reference_id = + ctx.create_reference_in_current_scope("module", ReferenceFlags::Read); let reference = ctx.ast.alloc_identifier_reference_with_reference_id(SPAN, "module", reference_id); let object = Expression::Identifier(reference); diff --git a/crates/oxc_traverse/src/context/mod.rs b/crates/oxc_traverse/src/context/mod.rs index 98b62e8d5c0b2..4909deeeca163 100644 --- a/crates/oxc_traverse/src/context/mod.rs +++ b/crates/oxc_traverse/src/context/mod.rs @@ -488,11 +488,7 @@ impl<'a> TraverseCtx<'a> { /// /// This is a shortcut for `ctx.scoping.create_unbound_reference`. #[inline] - pub fn create_unbound_reference( - &mut self, - name: CompactStr, - flags: ReferenceFlags, - ) -> ReferenceId { + pub fn create_unbound_reference(&mut self, name: &str, flags: ReferenceFlags) -> ReferenceId { self.scoping.create_unbound_reference(name, flags) } @@ -503,7 +499,7 @@ impl<'a> TraverseCtx<'a> { name: Atom<'a>, flags: ReferenceFlags, ) -> IdentifierReference<'a> { - let reference_id = self.create_unbound_reference(name.to_compact_str(), flags); + let reference_id = self.create_unbound_reference(name.as_str(), flags); self.ast.identifier_reference_with_reference_id(span, name, reference_id) } @@ -527,7 +523,7 @@ impl<'a> TraverseCtx<'a> { #[inline] pub fn create_reference( &mut self, - name: CompactStr, + name: &str, symbol_id: Option, flags: ReferenceFlags, ) -> ReferenceId { @@ -576,7 +572,7 @@ impl<'a> TraverseCtx<'a> { #[inline] pub fn create_reference_in_current_scope( &mut self, - name: CompactStr, + name: &str, flags: ReferenceFlags, ) -> ReferenceId { self.scoping.create_reference_in_current_scope(name, flags) diff --git a/crates/oxc_traverse/src/context/scoping.rs b/crates/oxc_traverse/src/context/scoping.rs index 407ed853545e2..b932e5e8bc783 100644 --- a/crates/oxc_traverse/src/context/scoping.rs +++ b/crates/oxc_traverse/src/context/scoping.rs @@ -313,11 +313,7 @@ impl TraverseScoping { } /// Create an unbound reference - pub fn create_unbound_reference( - &mut self, - name: CompactStr, - flags: ReferenceFlags, - ) -> ReferenceId { + pub fn create_unbound_reference(&mut self, name: &str, flags: ReferenceFlags) -> ReferenceId { let reference = Reference::new(NodeId::DUMMY, flags); let reference_id = self.symbols.create_reference(reference); self.scopes.add_root_unresolved_reference(name, reference_id); @@ -330,7 +326,7 @@ impl TraverseScoping { /// or `TraverseCtx::create_unbound_reference`. pub fn create_reference( &mut self, - name: CompactStr, + name: &str, symbol_id: Option, flags: ReferenceFlags, ) -> ReferenceId { @@ -344,10 +340,10 @@ impl TraverseScoping { /// Create reference in current scope, looking up binding for `name` pub fn create_reference_in_current_scope( &mut self, - name: CompactStr, + name: &str, flags: ReferenceFlags, ) -> ReferenceId { - let symbol_id = self.scopes.find_binding(self.current_scope_id, name.as_str()); + let symbol_id = self.scopes.find_binding(self.current_scope_id, name); self.create_reference(name, symbol_id, flags) } @@ -433,7 +429,7 @@ impl TraverseScoping { self.scopes .root_unresolved_references() .keys() - .map(CompactStr::as_str) + .copied() .chain(self.symbols.names()) .filter(|name| name.as_bytes().first() == Some(&b'_')) .map(CompactStr::from) diff --git a/tasks/transform_checker/src/lib.rs b/tasks/transform_checker/src/lib.rs index f6885942c6844..1c7248f5ddd97 100644 --- a/tasks/transform_checker/src/lib.rs +++ b/tasks/transform_checker/src/lib.rs @@ -506,8 +506,12 @@ impl PostTransformChecker<'_, '_> { fn check_unresolved_references(&mut self) { let unresolved_names = self.get_static_pair(|scoping| { - let mut names = - scoping.scopes.root_unresolved_references().keys().cloned().collect::>(); + let mut names = scoping + .scopes + .root_unresolved_references() + .keys() + .map(ToString::to_string) + .collect::>(); names.sort_unstable(); names }); @@ -521,6 +525,10 @@ impl PostTransformChecker<'_, '_> { if let Some(reference_ids_rebuilt) = self.scoping_rebuilt.scopes.root_unresolved_references().get(name) { + let reference_ids_after_transform = + reference_ids_after_transform.iter().copied().collect::>(); + let reference_ids_rebuilt = + reference_ids_rebuilt.iter().copied().collect::>(); let reference_ids = Pair::new(reference_ids_after_transform, reference_ids_rebuilt); if self.remap_reference_ids_sets(&reference_ids).is_mismatch() { self.errors.push_mismatch_single(