From 1b51511bb605d61cb209eb3421fd6375bb4cc099 Mon Sep 17 00:00:00 2001 From: Dunqing <29533304+Dunqing@users.noreply.github.com> Date: Mon, 22 Jul 2024 12:18:37 +0000 Subject: [PATCH] perf(semantic): use `Atom` instead of `CompactStr` for `UnresolvedReferencesStack` (#4401) related: #4394 The `UnresolvedReferencesStack` is only used for resolving references, and the `Atom` clone is cheap, So we can safely change `CompactStr`to `Atom` --- crates/oxc_semantic/src/builder.rs | 26 ++++++++++++--------- crates/oxc_semantic/src/scope.rs | 2 +- crates/oxc_semantic/src/unresolved_stack.rs | 23 +++++++++++------- 3 files changed, 30 insertions(+), 21 deletions(-) diff --git a/crates/oxc_semantic/src/builder.rs b/crates/oxc_semantic/src/builder.rs index 825dfdc7ede35..39325c6a8f3c8 100644 --- a/crates/oxc_semantic/src/builder.rs +++ b/crates/oxc_semantic/src/builder.rs @@ -13,7 +13,7 @@ use oxc_cfg::{ IterationInstructionKind, ReturnInstructionKind, }; use oxc_diagnostics::OxcDiagnostic; -use oxc_span::{CompactStr, SourceType, Span}; +use oxc_span::{Atom, CompactStr, SourceType, Span}; use oxc_syntax::{module_record::ModuleRecord, operator::AssignmentOperator}; use crate::{ @@ -72,7 +72,7 @@ pub struct SemanticBuilder<'a> { pub scope: ScopeTree, pub symbols: SymbolTable, - unresolved_references: UnresolvedReferencesStack, + unresolved_references: UnresolvedReferencesStack<'a>, pub(crate) module_record: Arc, @@ -218,7 +218,12 @@ impl<'a> SemanticBuilder<'a> { } debug_assert_eq!(self.unresolved_references.scope_depth(), 1); - self.scope.root_unresolved_references = self.unresolved_references.into_root(); + self.scope.root_unresolved_references = self + .unresolved_references + .into_root() + .into_iter() + .map(|(k, v)| (k.into(), v)) + .collect(); let jsdoc = if self.build_jsdoc { self.jsdoc.build() } else { JSDocFinder::default() }; @@ -367,14 +372,13 @@ impl<'a> SemanticBuilder<'a> { /// Declare an unresolved reference in the current scope. /// /// # Panics - pub fn declare_reference(&mut self, reference: Reference) -> ReferenceId { - let reference_name = reference.name().clone(); + pub fn declare_reference(&mut self, name: Atom<'a>, reference: Reference) -> ReferenceId { let reference_flag = *reference.flag(); let reference_id = self.symbols.create_reference(reference); self.unresolved_references .current_mut() - .entry(reference_name) + .entry(name) .or_default() .push((reference_id, reference_flag)); reference_id @@ -404,7 +408,7 @@ impl<'a> SemanticBuilder<'a> { // Try to resolve a reference. // If unresolved, transfer it to parent scope's unresolved references. let bindings = self.scope.get_bindings(self.current_scope_id); - if let Some(symbol_id) = bindings.get(&name).copied() { + if let Some(symbol_id) = bindings.get(name.as_str()).copied() { let symbol_flag = self.symbols.get_flag(symbol_id); let resolved_references: &mut Vec<_> = @@ -1908,11 +1912,11 @@ impl<'a> SemanticBuilder<'a> { } } - fn reference_identifier(&mut self, ident: &IdentifierReference) { + fn reference_identifier(&mut self, ident: &IdentifierReference<'a>) { let flag = self.resolve_reference_usages(); let name = ident.name.to_compact_str(); let reference = Reference::new(ident.span, name, self.current_node_id, flag); - let reference_id = self.declare_reference(reference); + let reference_id = self.declare_reference(ident.name.clone(), reference); ident.reference_id.set(Some(reference_id)); } @@ -1925,7 +1929,7 @@ impl<'a> SemanticBuilder<'a> { } } - fn reference_jsx_identifier(&mut self, ident: &JSXIdentifier) { + fn reference_jsx_identifier(&mut self, ident: &JSXIdentifier<'a>) { match self.nodes.parent_kind(self.current_node_id) { Some(AstKind::JSXElementName(_)) => { if !ident.name.chars().next().is_some_and(char::is_uppercase) { @@ -1941,7 +1945,7 @@ impl<'a> SemanticBuilder<'a> { self.current_node_id, ReferenceFlag::read(), ); - self.declare_reference(reference); + self.declare_reference(ident.name.clone(), reference); } fn is_not_expression_statement_parent(&self) -> bool { diff --git a/crates/oxc_semantic/src/scope.rs b/crates/oxc_semantic/src/scope.rs index 73d334ae41dd6..802b52df7707e 100644 --- a/crates/oxc_semantic/src/scope.rs +++ b/crates/oxc_semantic/src/scope.rs @@ -13,7 +13,7 @@ type FxIndexMap = IndexMap>; type Bindings = FxIndexMap; pub(crate) type UnresolvedReference = (ReferenceId, ReferenceFlag); -pub(crate) type UnresolvedReferences = FxHashMap>; +pub type UnresolvedReferences = FxHashMap>; /// Scope Tree /// diff --git a/crates/oxc_semantic/src/unresolved_stack.rs b/crates/oxc_semantic/src/unresolved_stack.rs index 110f030d396b7..54f30a9b856b2 100644 --- a/crates/oxc_semantic/src/unresolved_stack.rs +++ b/crates/oxc_semantic/src/unresolved_stack.rs @@ -1,21 +1,26 @@ use assert_unchecked::assert_unchecked; +use oxc_span::Atom; +use rustc_hash::FxHashMap; -use crate::scope::UnresolvedReferences; +use crate::scope::UnresolvedReference; + +/// The difference with Scope's `UnresolvedReferences` is that this type uses Atom as the key. its clone is very cheap! +type TempUnresolvedReferences<'a> = FxHashMap, Vec>; // Stack used to accumulate unresolved refs while traversing scopes. // Indexed by scope depth. We recycle `UnresolvedReferences` instances during traversal // to reduce allocations, so the stack grows to maximum scope depth, but never shrinks. // See: // This stack abstraction uses the invariant that stack only grows to avoid bounds checks. -pub(crate) struct UnresolvedReferencesStack { - stack: Vec, +pub(crate) struct UnresolvedReferencesStack<'a> { + stack: Vec>, /// Current scope depth. /// 0 is global scope. 1 is `Program`. /// Incremented on entering a scope, and decremented on exit. current_scope_depth: usize, } -impl UnresolvedReferencesStack { +impl<'a> UnresolvedReferencesStack<'a> { // Most programs will have at least 1 place where scope depth reaches 16, // so initialize `stack` with this length, to reduce reallocations as it grows. // This is just an estimate of a good initial size, but certainly better than @@ -31,7 +36,7 @@ impl UnresolvedReferencesStack { pub(crate) fn new() -> Self { let mut stack = vec![]; - stack.resize_with(Self::INITIAL_SIZE, UnresolvedReferences::default); + stack.resize_with(Self::INITIAL_SIZE, TempUnresolvedReferences::default); Self { stack, current_scope_depth: Self::INITIAL_DEPTH } } @@ -40,7 +45,7 @@ impl UnresolvedReferencesStack { // Grow stack if required to ensure `self.stack[self.current_scope_depth]` is in bounds if self.stack.len() <= self.current_scope_depth { - self.stack.push(UnresolvedReferences::default()); + self.stack.push(TempUnresolvedReferences::default()); } } @@ -57,7 +62,7 @@ impl UnresolvedReferencesStack { } /// Get unresolved references hash map for current scope - pub(crate) fn current_mut(&mut self) -> &mut UnresolvedReferences { + pub(crate) fn current_mut(&mut self) -> &mut TempUnresolvedReferences<'a> { // SAFETY: `stack.len() > current_scope_depth` initially. // Thereafter, `stack` never shrinks, only grows. // `current_scope_depth` is only increased in `increment_scope_depth`, @@ -69,7 +74,7 @@ impl UnresolvedReferencesStack { /// Get unresolved references hash maps for current scope, and parent scope pub(crate) fn current_and_parent_mut( &mut self, - ) -> (&mut UnresolvedReferences, &mut UnresolvedReferences) { + ) -> (&mut TempUnresolvedReferences<'a>, &mut TempUnresolvedReferences<'a>) { // Assert invariants to remove bounds checks in code below. // https://godbolt.org/z/vv5Wo5csv // SAFETY: `current_scope_depth` starts at 1, and is only decremented @@ -87,7 +92,7 @@ impl UnresolvedReferencesStack { (current, parent) } - pub(crate) fn into_root(self) -> UnresolvedReferences { + pub(crate) fn into_root(self) -> TempUnresolvedReferences<'a> { // SAFETY: Stack starts with a non-zero size and never shrinks. // This assertion removes bounds check in `.next()`. unsafe { assert_unchecked!(!self.stack.is_empty()) };