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`
This commit is contained in:
Dunqing 2024-07-22 12:18:37 +00:00
parent bc8d4e5876
commit 1b51511bb6
3 changed files with 30 additions and 21 deletions

View file

@ -13,7 +13,7 @@ use oxc_cfg::{
IterationInstructionKind, ReturnInstructionKind, IterationInstructionKind, ReturnInstructionKind,
}; };
use oxc_diagnostics::OxcDiagnostic; 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 oxc_syntax::{module_record::ModuleRecord, operator::AssignmentOperator};
use crate::{ use crate::{
@ -72,7 +72,7 @@ pub struct SemanticBuilder<'a> {
pub scope: ScopeTree, pub scope: ScopeTree,
pub symbols: SymbolTable, pub symbols: SymbolTable,
unresolved_references: UnresolvedReferencesStack, unresolved_references: UnresolvedReferencesStack<'a>,
pub(crate) module_record: Arc<ModuleRecord>, pub(crate) module_record: Arc<ModuleRecord>,
@ -218,7 +218,12 @@ impl<'a> SemanticBuilder<'a> {
} }
debug_assert_eq!(self.unresolved_references.scope_depth(), 1); 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() }; 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. /// Declare an unresolved reference in the current scope.
/// ///
/// # Panics /// # Panics
pub fn declare_reference(&mut self, reference: Reference) -> ReferenceId { pub fn declare_reference(&mut self, name: Atom<'a>, reference: Reference) -> ReferenceId {
let reference_name = reference.name().clone();
let reference_flag = *reference.flag(); let reference_flag = *reference.flag();
let reference_id = self.symbols.create_reference(reference); let reference_id = self.symbols.create_reference(reference);
self.unresolved_references self.unresolved_references
.current_mut() .current_mut()
.entry(reference_name) .entry(name)
.or_default() .or_default()
.push((reference_id, reference_flag)); .push((reference_id, reference_flag));
reference_id reference_id
@ -404,7 +408,7 @@ impl<'a> SemanticBuilder<'a> {
// Try to resolve a reference. // Try to resolve a reference.
// If unresolved, transfer it to parent scope's unresolved references. // If unresolved, transfer it to parent scope's unresolved references.
let bindings = self.scope.get_bindings(self.current_scope_id); 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 symbol_flag = self.symbols.get_flag(symbol_id);
let resolved_references: &mut Vec<_> = 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 flag = self.resolve_reference_usages();
let name = ident.name.to_compact_str(); let name = ident.name.to_compact_str();
let reference = Reference::new(ident.span, name, self.current_node_id, flag); 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)); 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) { match self.nodes.parent_kind(self.current_node_id) {
Some(AstKind::JSXElementName(_)) => { Some(AstKind::JSXElementName(_)) => {
if !ident.name.chars().next().is_some_and(char::is_uppercase) { if !ident.name.chars().next().is_some_and(char::is_uppercase) {
@ -1941,7 +1945,7 @@ impl<'a> SemanticBuilder<'a> {
self.current_node_id, self.current_node_id,
ReferenceFlag::read(), ReferenceFlag::read(),
); );
self.declare_reference(reference); self.declare_reference(ident.name.clone(), reference);
} }
fn is_not_expression_statement_parent(&self) -> bool { fn is_not_expression_statement_parent(&self) -> bool {

View file

@ -13,7 +13,7 @@ type FxIndexMap<K, V> = IndexMap<K, V, BuildHasherDefault<FxHasher>>;
type Bindings = FxIndexMap<CompactStr, SymbolId>; type Bindings = FxIndexMap<CompactStr, SymbolId>;
pub(crate) type UnresolvedReference = (ReferenceId, ReferenceFlag); pub(crate) type UnresolvedReference = (ReferenceId, ReferenceFlag);
pub(crate) type UnresolvedReferences = FxHashMap<CompactStr, Vec<UnresolvedReference>>; pub type UnresolvedReferences = FxHashMap<CompactStr, Vec<UnresolvedReference>>;
/// Scope Tree /// Scope Tree
/// ///

View file

@ -1,21 +1,26 @@
use assert_unchecked::assert_unchecked; 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<Atom<'a>, Vec<UnresolvedReference>>;
// Stack used to accumulate unresolved refs while traversing scopes. // Stack used to accumulate unresolved refs while traversing scopes.
// Indexed by scope depth. We recycle `UnresolvedReferences` instances during traversal // Indexed by scope depth. We recycle `UnresolvedReferences` instances during traversal
// to reduce allocations, so the stack grows to maximum scope depth, but never shrinks. // to reduce allocations, so the stack grows to maximum scope depth, but never shrinks.
// See: <https://github.com/oxc-project/oxc/issues/4169> // See: <https://github.com/oxc-project/oxc/issues/4169>
// This stack abstraction uses the invariant that stack only grows to avoid bounds checks. // This stack abstraction uses the invariant that stack only grows to avoid bounds checks.
pub(crate) struct UnresolvedReferencesStack { pub(crate) struct UnresolvedReferencesStack<'a> {
stack: Vec<UnresolvedReferences>, stack: Vec<TempUnresolvedReferences<'a>>,
/// Current scope depth. /// Current scope depth.
/// 0 is global scope. 1 is `Program`. /// 0 is global scope. 1 is `Program`.
/// Incremented on entering a scope, and decremented on exit. /// Incremented on entering a scope, and decremented on exit.
current_scope_depth: usize, current_scope_depth: usize,
} }
impl UnresolvedReferencesStack { impl<'a> UnresolvedReferencesStack<'a> {
// Most programs will have at least 1 place where scope depth reaches 16, // 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. // 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 // 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 { pub(crate) fn new() -> Self {
let mut stack = vec![]; 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 } 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 // Grow stack if required to ensure `self.stack[self.current_scope_depth]` is in bounds
if self.stack.len() <= self.current_scope_depth { 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 /// 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. // SAFETY: `stack.len() > current_scope_depth` initially.
// Thereafter, `stack` never shrinks, only grows. // Thereafter, `stack` never shrinks, only grows.
// `current_scope_depth` is only increased in `increment_scope_depth`, // `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 /// Get unresolved references hash maps for current scope, and parent scope
pub(crate) fn current_and_parent_mut( pub(crate) fn current_and_parent_mut(
&mut self, &mut self,
) -> (&mut UnresolvedReferences, &mut UnresolvedReferences) { ) -> (&mut TempUnresolvedReferences<'a>, &mut TempUnresolvedReferences<'a>) {
// Assert invariants to remove bounds checks in code below. // Assert invariants to remove bounds checks in code below.
// https://godbolt.org/z/vv5Wo5csv // https://godbolt.org/z/vv5Wo5csv
// SAFETY: `current_scope_depth` starts at 1, and is only decremented // SAFETY: `current_scope_depth` starts at 1, and is only decremented
@ -87,7 +92,7 @@ impl UnresolvedReferencesStack {
(current, parent) (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. // SAFETY: Stack starts with a non-zero size and never shrinks.
// This assertion removes bounds check in `.next()`. // This assertion removes bounds check in `.next()`.
unsafe { assert_unchecked!(!self.stack.is_empty()) }; unsafe { assert_unchecked!(!self.stack.is_empty()) };