From 414e8285cdad49f196e8b92dd82177b32f811ea7 Mon Sep 17 00:00:00 2001 From: Boshen <1430279+Boshen@users.noreply.github.com> Date: Fri, 20 Dec 2024 14:13:41 +0000 Subject: [PATCH] perf(semantic): allocate symbol data in Allocator (#8012) --- crates/oxc_allocator/src/lib.rs | 5 + crates/oxc_allocator/src/vec.rs | 5 + crates/oxc_mangler/src/lib.rs | 2 +- crates/oxc_semantic/src/builder.rs | 16 +-- crates/oxc_semantic/src/scope.rs | 8 +- crates/oxc_semantic/src/symbol.rs | 122 +++++++++++++++------ crates/oxc_traverse/src/context/scoping.rs | 17 +-- crates/oxc_wasm/src/lib.rs | 6 +- tasks/transform_checker/src/lib.rs | 7 +- 9 files changed, 129 insertions(+), 59 deletions(-) diff --git a/crates/oxc_allocator/src/lib.rs b/crates/oxc_allocator/src/lib.rs index a6f5646f74139..8fd4bfa13116b 100644 --- a/crates/oxc_allocator/src/lib.rs +++ b/crates/oxc_allocator/src/lib.rs @@ -72,6 +72,11 @@ pub struct Allocator { bump: Bump, } +/// SAFETY: Not actually safe, but for enabling `Send` for downstream crates. +unsafe impl Send for Allocator {} +/// SAFETY: Not actually safe, but for enabling `Sync` for downstream crates. +unsafe impl Sync for Allocator {} + impl From for Allocator { fn from(bump: Bump) -> Self { Self { bump } diff --git a/crates/oxc_allocator/src/vec.rs b/crates/oxc_allocator/src/vec.rs index 983b38e281b99..4b1e22e30252d 100644 --- a/crates/oxc_allocator/src/vec.rs +++ b/crates/oxc_allocator/src/vec.rs @@ -32,6 +32,11 @@ use crate::{Allocator, Box, String}; #[derive(PartialEq, Eq)] pub struct Vec<'alloc, T>(ManuallyDrop>); +/// SAFETY: Not actually safe, but for enabling `Send` for downstream crates. +unsafe impl Send for Vec<'_, T> {} +/// SAFETY: Not actually safe, but for enabling `Sync` for downstream crates. +unsafe impl Sync for Vec<'_, T> {} + impl<'alloc, T> Vec<'alloc, T> { /// Constructs a new, empty `Vec`. /// diff --git a/crates/oxc_mangler/src/lib.rs b/crates/oxc_mangler/src/lib.rs index 92b209f73c275..61598118ca8ee 100644 --- a/crates/oxc_mangler/src/lib.rs +++ b/crates/oxc_mangler/src/lib.rs @@ -194,7 +194,7 @@ impl Mangler { // rename the variables for (symbol_to_rename, new_name) in symbols_to_rename_with_new_names { for &symbol_id in &symbol_to_rename.symbol_ids { - symbol_table.set_name(symbol_id, new_name.clone()); + symbol_table.set_name(symbol_id, new_name.as_str()); } } } diff --git a/crates/oxc_semantic/src/builder.rs b/crates/oxc_semantic/src/builder.rs index 0ed9d2de0f853..edd6875b2c8db 100644 --- a/crates/oxc_semantic/src/builder.rs +++ b/crates/oxc_semantic/src/builder.rs @@ -14,7 +14,7 @@ use oxc_cfg::{ IterationInstructionKind, ReturnInstructionKind, }; use oxc_diagnostics::OxcDiagnostic; -use oxc_span::{Atom, CompactStr, SourceType, Span}; +use oxc_span::{Atom, SourceType, Span}; use oxc_syntax::{ node::{NodeFlags, NodeId}, reference::{Reference, ReferenceFlags, ReferenceId}, @@ -398,13 +398,9 @@ impl<'a> SemanticBuilder<'a> { return symbol_id; } - let symbol_id = self.symbols.create_symbol( - span, - CompactStr::new(name), - includes, - scope_id, - self.current_node_id, - ); + let symbol_id = + self.symbols.create_symbol(span, name, includes, scope_id, self.current_node_id); + self.scope.add_binding(scope_id, name, symbol_id); symbol_id } @@ -466,7 +462,7 @@ impl<'a> SemanticBuilder<'a> { ) -> SymbolId { let symbol_id = self.symbols.create_symbol( span, - CompactStr::new(name), + name, includes, self.current_scope_id, self.current_node_id, @@ -521,7 +517,7 @@ impl<'a> SemanticBuilder<'a> { *flags = ReferenceFlags::Type; } reference.set_symbol_id(symbol_id); - self.symbols.resolved_references[symbol_id].push(reference_id); + self.symbols.add_resolved_reference(symbol_id, reference_id); false }); diff --git a/crates/oxc_semantic/src/scope.rs b/crates/oxc_semantic/src/scope.rs index a62c8e6e6b4be..0b6144e41c3a8 100644 --- a/crates/oxc_semantic/src/scope.rs +++ b/crates/oxc_semantic/src/scope.rs @@ -1,4 +1,4 @@ -use std::mem; +use std::{fmt, mem}; use rustc_hash::{FxBuildHasher, FxHashMap}; @@ -45,6 +45,12 @@ pub struct ScopeTree { pub(crate) cell: ScopeTreeCell, } +impl fmt::Debug for ScopeTree { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> Result<(), fmt::Error> { + f.debug_struct("ScopeTree").finish() + } +} + impl Default for ScopeTree { fn default() -> Self { Self { diff --git a/crates/oxc_semantic/src/symbol.rs b/crates/oxc_semantic/src/symbol.rs index 79740717e4a50..d1338d6ea496b 100644 --- a/crates/oxc_semantic/src/symbol.rs +++ b/crates/oxc_semantic/src/symbol.rs @@ -1,8 +1,9 @@ -use std::mem; +use std::{fmt, mem}; +use oxc_allocator::{Allocator, FromIn, Vec as ArenaVec}; use oxc_ast::ast::{Expression, IdentifierReference}; -use oxc_index::IndexVec; -use oxc_span::{CompactStr, Span}; +use oxc_index::{Idx, IndexVec}; +use oxc_span::{Atom, Span}; use oxc_syntax::{ node::NodeId, reference::{Reference, ReferenceId}, @@ -17,20 +18,56 @@ use oxc_syntax::{ /// Most symbols won't have redeclarations, so instead of storing `Vec` directly in /// `redeclare_variables` (32 bytes per symbol), store `Option` (4 bytes). /// That ID indexes into `redeclarations` where the actual `Vec` is stored. -#[derive(Debug, Default)] pub struct SymbolTable { pub(crate) spans: IndexVec, - pub(crate) names: IndexVec, pub(crate) flags: IndexVec, pub(crate) scope_ids: IndexVec, /// Pointer to the AST Node where this symbol is declared pub(crate) declarations: IndexVec, - pub resolved_references: IndexVec>, redeclarations: IndexVec>, - redeclaration_spans: IndexVec>, - pub references: IndexVec, + + inner: SymbolTableCell, +} + +impl fmt::Debug for SymbolTable { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> Result<(), fmt::Error> { + f.debug_struct("SymbolTable").finish() + } +} + +impl Default for SymbolTable { + fn default() -> Self { + let allocator = Allocator::default(); + Self { + spans: IndexVec::new(), + flags: IndexVec::new(), + scope_ids: IndexVec::new(), + declarations: IndexVec::new(), + redeclarations: IndexVec::new(), + references: IndexVec::new(), + inner: SymbolTableCell::new(allocator, |allocator| SymbolTableInner { + names: ArenaVec::new_in(allocator), + resolved_references: ArenaVec::new_in(allocator), + redeclaration_spans: ArenaVec::new_in(allocator), + }), + } + } +} + +self_cell::self_cell!( + struct SymbolTableCell { + owner: Allocator, + #[covariant] + dependent: SymbolTableInner, + } +); + +struct SymbolTableInner<'cell> { + names: ArenaVec<'cell, Atom<'cell>>, + resolved_references: ArenaVec<'cell, ArenaVec<'cell, ReferenceId>>, + redeclaration_spans: ArenaVec<'cell, ArenaVec<'cell, Span>>, } impl SymbolTable { @@ -46,12 +83,12 @@ impl SymbolTable { self.spans.is_empty() } - pub fn names(&self) -> impl Iterator + '_ { - self.names.iter() + pub fn names(&self) -> impl Iterator + '_ { + self.inner.borrow_dependent().names.iter().map(Atom::as_str) } - pub fn resolved_references(&self) -> impl Iterator> + '_ { - self.resolved_references.iter() + pub fn resolved_references(&self) -> impl Iterator> + '_ { + self.inner.borrow_dependent().resolved_references.iter() } /// Iterate over all symbol IDs in this table. @@ -91,15 +128,19 @@ impl SymbolTable { /// Get the identifier name a symbol is bound to. #[inline] pub fn get_name(&self, symbol_id: SymbolId) -> &str { - &self.names[symbol_id] + &self.inner.borrow_dependent().names[symbol_id.index()] } /// Rename a symbol. /// /// Returns the old name. #[inline] - pub fn set_name(&mut self, symbol_id: SymbolId, name: CompactStr) -> CompactStr { - mem::replace(&mut self.names[symbol_id], name) + pub fn set_name(&mut self, symbol_id: SymbolId, name: &str) -> &str { + self.inner + .with_dependent_mut(|allocator, inner| { + mem::replace(&mut inner.names[symbol_id.index()], Atom::from_in(name, allocator)) + }) + .as_str() } /// Get the [`SymbolFlags`] for a symbol, which describe how the symbol is declared. @@ -119,7 +160,7 @@ impl SymbolTable { #[inline] pub fn get_redeclarations(&self, symbol_id: SymbolId) -> &[Span] { if let Some(redeclaration_id) = self.redeclarations[symbol_id] { - &self.redeclaration_spans[redeclaration_id] + &self.inner.borrow_dependent().redeclaration_spans[redeclaration_id.index()] } else { static EMPTY: &[Span] = &[]; EMPTY @@ -159,26 +200,36 @@ impl SymbolTable { pub fn create_symbol( &mut self, span: Span, - name: CompactStr, + name: &str, flags: SymbolFlags, scope_id: ScopeId, node_id: NodeId, ) -> SymbolId { self.spans.push(span); - self.names.push(name); self.flags.push(flags); self.scope_ids.push(scope_id); self.declarations.push(node_id); - self.resolved_references.push(vec![]); + self.inner.with_dependent_mut(|allocator, inner| { + inner.names.push(Atom::from_in(name, allocator)); + inner.resolved_references.push(ArenaVec::new_in(allocator)); + }); self.redeclarations.push(None) } pub fn add_redeclaration(&mut self, symbol_id: SymbolId, span: Span) { if let Some(redeclaration_id) = self.redeclarations[symbol_id] { - self.redeclaration_spans[redeclaration_id].push(span); + self.inner.with_dependent_mut(|_, inner| { + inner.redeclaration_spans[redeclaration_id.index()].push(span); + }); } else { - let redeclaration_id = self.redeclaration_spans.push(vec![span]); - self.redeclarations[symbol_id] = Some(redeclaration_id); + self.inner.with_dependent_mut(|allocator, inner| { + let mut v = ArenaVec::new_in(allocator); + v.push(span); + let redeclaration_id = inner.redeclaration_spans.len(); + inner.redeclaration_spans.push(v); + self.redeclarations[symbol_id] = + Some(RedeclarationId::from_usize(redeclaration_id)); + }); }; } @@ -213,8 +264,8 @@ impl SymbolTable { /// If you want direct access to a symbol's [`Reference`]s, use /// [`SymbolTable::get_resolved_references`]. #[inline] - pub fn get_resolved_reference_ids(&self, symbol_id: SymbolId) -> &Vec { - &self.resolved_references[symbol_id] + pub fn get_resolved_reference_ids(&self, symbol_id: SymbolId) -> &ArenaVec<'_, ReferenceId> { + &self.inner.borrow_dependent().resolved_references[symbol_id.index()] } /// Find [`Reference`]s resolved to a symbol. @@ -222,7 +273,7 @@ impl SymbolTable { &self, symbol_id: SymbolId, ) -> impl DoubleEndedIterator + '_ { - self.resolved_references[symbol_id] + self.get_resolved_reference_ids(symbol_id) .iter() .map(|&reference_id| &self.references[reference_id]) } @@ -241,8 +292,9 @@ impl SymbolTable { /// Add a reference to a symbol. pub fn add_resolved_reference(&mut self, symbol_id: SymbolId, reference_id: ReferenceId) { - let reference_ids = &mut self.resolved_references[symbol_id]; - reference_ids.push(reference_id); + self.inner.with_dependent_mut(|_allocator, inner| { + inner.resolved_references[symbol_id.index()].push(reference_id); + }); } /// Delete a reference to a symbol. @@ -250,20 +302,22 @@ impl SymbolTable { /// # Panics /// Panics if provided `reference_id` is not a resolved reference for `symbol_id`. pub fn delete_resolved_reference(&mut self, symbol_id: SymbolId, reference_id: ReferenceId) { - let reference_ids = &mut self.resolved_references[symbol_id]; - let index = reference_ids.iter().position(|&id| id == reference_id).unwrap(); - reference_ids.swap_remove(index); + self.inner.with_dependent_mut(|_allocator, inner| { + let reference_ids = &mut inner.resolved_references[symbol_id.index()]; + let index = reference_ids.iter().position(|&id| id == reference_id).unwrap(); + reference_ids.swap_remove(index); + }); } pub fn reserve(&mut self, additional_symbols: usize, additional_references: usize) { self.spans.reserve(additional_symbols); - self.names.reserve(additional_symbols); self.flags.reserve(additional_symbols); self.scope_ids.reserve(additional_symbols); self.declarations.reserve(additional_symbols); - self.resolved_references.reserve(additional_symbols); - self.redeclarations.reserve(additional_symbols); - + self.inner.with_dependent_mut(|_allocator, inner| { + inner.names.reserve(additional_symbols); + inner.resolved_references.reserve(additional_symbols); + }); self.references.reserve(additional_references); } } diff --git a/crates/oxc_traverse/src/context/scoping.rs b/crates/oxc_traverse/src/context/scoping.rs index 96825d5bd7da6..50bc2cf1e6868 100644 --- a/crates/oxc_traverse/src/context/scoping.rs +++ b/crates/oxc_traverse/src/context/scoping.rs @@ -182,8 +182,7 @@ impl TraverseScoping { scope_id: ScopeId, flags: SymbolFlags, ) -> SymbolId { - let symbol_id = - self.symbols.create_symbol(SPAN, name.into(), flags, scope_id, NodeId::DUMMY); + let symbol_id = self.symbols.create_symbol(SPAN, name, flags, scope_id, NodeId::DUMMY); self.scopes.add_binding(scope_id, name, symbol_id); symbol_id @@ -380,9 +379,9 @@ impl TraverseScoping { #[expect(clippy::needless_pass_by_value)] pub fn rename_symbol(&mut self, symbol_id: SymbolId, scope_id: ScopeId, new_name: CompactStr) { // Rename symbol - let old_name = self.symbols.set_name(symbol_id, new_name.clone()); + let old_name = self.symbols.set_name(symbol_id, new_name.as_str()); // Rename binding - self.scopes.rename_binding(scope_id, symbol_id, &old_name, new_name.as_str()); + self.scopes.rename_binding(scope_id, symbol_id, old_name, new_name.as_str()); } } @@ -435,14 +434,10 @@ impl TraverseScoping { self.scopes .root_unresolved_references() .keys() + .map(CompactStr::as_str) .chain(self.symbols.names()) - .filter_map(|name| { - if name.as_bytes().first() == Some(&b'_') { - Some(name.clone()) - } else { - None - } - }) + .filter(|name| name.as_bytes().first() == Some(&b'_')) + .map(CompactStr::from) .collect() } } diff --git a/crates/oxc_wasm/src/lib.rs b/crates/oxc_wasm/src/lib.rs index 7516acaece7a8..a43d0a7495c32 100644 --- a/crates/oxc_wasm/src/lib.rs +++ b/crates/oxc_wasm/src/lib.rs @@ -427,7 +427,11 @@ impl Oxc { name: symbols.get_name(symbol_id).into(), flags: symbols.get_flags(symbol_id), scope_id: symbols.get_scope_id(symbol_id), - resolved_references: symbols.get_resolved_reference_ids(symbol_id).clone(), + resolved_references: symbols + .get_resolved_reference_ids(symbol_id) + .iter() + .copied() + .collect::>(), references: symbols .get_resolved_reference_ids(symbol_id) .iter() diff --git a/tasks/transform_checker/src/lib.rs b/tasks/transform_checker/src/lib.rs index ae90571598aa3..f6885942c6844 100644 --- a/tasks/transform_checker/src/lib.rs +++ b/tasks/transform_checker/src/lib.rs @@ -433,7 +433,12 @@ impl PostTransformChecker<'_, '_> { // Check resolved references match let reference_ids = self.get_pair(symbol_ids, |scoping, symbol_id| { - scoping.symbols.get_resolved_reference_ids(symbol_id).clone() + scoping + .symbols + .get_resolved_reference_ids(symbol_id) + .iter() + .copied() + .collect::>() }); if self.remap_reference_ids_sets(&reference_ids).is_mismatch() { self.errors.push_mismatch(