Skip to content

Commit

Permalink
perf(semantic): use Atom instead of CompactStr for `UnresolvedRef…
Browse files Browse the repository at this point in the history
…erencesStack` (#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`
  • Loading branch information
Dunqing committed Jul 22, 2024
1 parent bc8d4e5 commit 1b51511
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 21 deletions.
26 changes: 15 additions & 11 deletions crates/oxc_semantic/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down Expand Up @@ -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<ModuleRecord>,

Expand Down Expand Up @@ -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() };

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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<_> =
Expand Down Expand Up @@ -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));
}

Expand All @@ -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) {
Expand All @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_semantic/src/scope.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ type FxIndexMap<K, V> = IndexMap<K, V, BuildHasherDefault<FxHasher>>;

type Bindings = FxIndexMap<CompactStr, SymbolId>;
pub(crate) type UnresolvedReference = (ReferenceId, ReferenceFlag);
pub(crate) type UnresolvedReferences = FxHashMap<CompactStr, Vec<UnresolvedReference>>;
pub type UnresolvedReferences = FxHashMap<CompactStr, Vec<UnresolvedReference>>;

/// Scope Tree
///
Expand Down
23 changes: 14 additions & 9 deletions crates/oxc_semantic/src/unresolved_stack.rs
Original file line number Diff line number Diff line change
@@ -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<Atom<'a>, Vec<UnresolvedReference>>;

// 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: <https://github.com/oxc-project/oxc/issues/4169>
// This stack abstraction uses the invariant that stack only grows to avoid bounds checks.
pub(crate) struct UnresolvedReferencesStack {
stack: Vec<UnresolvedReferences>,
pub(crate) struct UnresolvedReferencesStack<'a> {
stack: Vec<TempUnresolvedReferences<'a>>,
/// 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
Expand All @@ -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 }
}

Expand All @@ -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());
}
}

Expand All @@ -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`,
Expand All @@ -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
Expand All @@ -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()) };
Expand Down

0 comments on commit 1b51511

Please sign in to comment.