Skip to content

Commit

Permalink
perf(semantic): allocate Bindings in allocator (#8021)
Browse files Browse the repository at this point in the history
  • Loading branch information
Boshen committed Dec 19, 2024
1 parent 02f968d commit 7aebed0
Show file tree
Hide file tree
Showing 11 changed files with 142 additions and 69 deletions.
14 changes: 14 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ futures = "0.3.31"
glob = "0.3.1"
globset = "0.4.15"
handlebars = "6.2.0"
hashbrown = "0.15.2"
humansize = "2.1.3"
ignore = "0.4.23"
insta = "1.41.1"
Expand Down
3 changes: 3 additions & 0 deletions crates/oxc_semantic/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,12 @@ oxc_span = { workspace = true }
oxc_syntax = { workspace = true }

assert-unchecked = { workspace = true }
bumpalo = { workspace = true, features = ["allocator-api2"] }
hashbrown = { workspace = true, features = ["allocator-api2"] }
itertools = { workspace = true }
phf = { workspace = true, features = ["macros"] }
rustc-hash = { workspace = true }
self_cell = { workspace = true }

[dev-dependencies]
insta = { workspace = true, features = ["glob"] }
Expand Down
3 changes: 1 addition & 2 deletions crates/oxc_semantic/src/binder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,9 @@ impl<'a> Binder<'a> for VariableDeclarator<'a> {
builder.add_redeclare_variable(symbol_id, span);
declared_symbol_id = Some(symbol_id);

let name = name.to_compact_str();
// remove current scope binding and add to target scope
// avoid same symbols appear in multi-scopes
builder.scope.remove_binding(scope_id, &name);
builder.scope.remove_binding(scope_id, name);
builder.scope.add_binding(target_scope_id, name, symbol_id);
builder.symbols.scope_ids[symbol_id] = target_scope_id;
break;
Expand Down
40 changes: 25 additions & 15 deletions crates/oxc_semantic/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,18 @@ use oxc_syntax::{
};

use crate::{
binder::Binder, checker, class::ClassTableBuilder, diagnostics::redeclaration,
jsdoc::JSDocBuilder, label::UnusedLabels, node::AstNodes, scope::ScopeTree, stats::Stats,
symbol::SymbolTable, unresolved_stack::UnresolvedReferencesStack, JSDocFinder, Semantic,
binder::Binder,
checker,
class::ClassTableBuilder,
diagnostics::redeclaration,
jsdoc::JSDocBuilder,
label::UnusedLabels,
node::AstNodes,
scope::{Bindings, ScopeTree},
stats::Stats,
symbol::SymbolTable,
unresolved_stack::UnresolvedReferencesStack,
JSDocFinder, Semantic,
};

macro_rules! control_flow {
Expand Down Expand Up @@ -389,10 +398,9 @@ impl<'a> SemanticBuilder<'a> {
return symbol_id;
}

let name = CompactStr::new(name);
let symbol_id = self.symbols.create_symbol(
span,
name.clone(),
CompactStr::new(name),
includes,
scope_id,
self.current_node_id,
Expand Down Expand Up @@ -456,15 +464,14 @@ impl<'a> SemanticBuilder<'a> {
scope_id: ScopeId,
includes: SymbolFlags,
) -> SymbolId {
let name = CompactStr::new(name);
let symbol_id = self.symbols.create_symbol(
span,
name.clone(),
CompactStr::new(name),
includes,
self.current_scope_id,
self.current_node_id,
);
self.scope.get_bindings_mut(scope_id).insert(name, symbol_id);
self.scope.insert_binding(scope_id, name, symbol_id);
symbol_id
}

Expand Down Expand Up @@ -702,14 +709,17 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> {
// Move all bindings from catch clause param scope to catch clause body scope
// to make it easier to resolve references and check redeclare errors
if self.scope.get_flags(parent_scope_id).is_catch_clause() {
let parent_bindings = self.scope.get_bindings_mut(parent_scope_id);
if !parent_bindings.is_empty() {
let parent_bindings = mem::take(parent_bindings);
for &symbol_id in parent_bindings.values() {
self.symbols.set_scope_id(symbol_id, self.current_scope_id);
self.scope.cell.with_dependent_mut(|allocator, inner| {
if !inner.bindings[parent_scope_id].is_empty() {
let mut parent_bindings =
Bindings::with_hasher_in(rustc_hash::FxBuildHasher, allocator);
mem::swap(&mut inner.bindings[parent_scope_id], &mut parent_bindings);
for &symbol_id in parent_bindings.values() {
self.symbols.set_scope_id(symbol_id, self.current_scope_id);
}
inner.bindings[self.current_scope_id] = parent_bindings;
}
*self.scope.get_bindings_mut(self.current_scope_id) = parent_bindings;
}
});
}

self.visit_statements(&it.body);
Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_semantic/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,7 @@ mod tests {
let top_level_a = semantic
.scopes()
.iter_bindings()
.find(|(_scope_id, _symbol_id, name)| name.as_str() == "Fn")
.find(|(_scope_id, _symbol_id, name)| *name == "Fn")
.unwrap();
assert_eq!(semantic.symbols.get_scope_id(top_level_a.1), top_level_a.0);
}
Expand Down
117 changes: 81 additions & 36 deletions crates/oxc_semantic/src/scope.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
use std::mem;

use rustc_hash::FxHashMap;
use rustc_hash::{FxBuildHasher, FxHashMap};

use bumpalo::Bump;
use oxc_index::IndexVec;
use oxc_span::CompactStr;
use oxc_syntax::{
Expand All @@ -11,7 +12,7 @@ use oxc_syntax::{
symbol::SymbolId,
};

pub(crate) type Bindings = FxHashMap<CompactStr, SymbolId>;
pub(crate) type Bindings<'a> = hashbrown::HashMap<&'a str, SymbolId, FxBuildHasher, &'a Bump>;
pub type UnresolvedReferences = FxHashMap<CompactStr, Vec<ReferenceId>>;

/// Scope Tree
Expand All @@ -24,7 +25,6 @@ pub type UnresolvedReferences = FxHashMap<CompactStr, Vec<ReferenceId>>;
/// - Nodes that create a scope store the [`ScopeId`] of the scope they create.
///
/// `SoA` (Struct of Arrays) for memory efficiency.
#[derive(Debug, Default)]
pub struct ScopeTree {
/// Maps a scope to the parent scope it belongs in.
parent_ids: IndexVec<ScopeId, Option<ScopeId>>,
Expand All @@ -40,12 +40,40 @@ pub struct ScopeTree {

flags: IndexVec<ScopeId, ScopeFlags>,

pub(crate) root_unresolved_references: UnresolvedReferences,

pub(crate) cell: ScopeTreeCell,
}

impl Default for ScopeTree {
fn default() -> Self {
Self {
parent_ids: IndexVec::new(),
child_ids: IndexVec::new(),
build_child_ids: false,
node_ids: IndexVec::new(),
flags: IndexVec::new(),
root_unresolved_references: UnresolvedReferences::default(),
cell: ScopeTreeCell::new(Bump::new(), |_bump| ScopeTreeInner {
bindings: IndexVec::new(),
}),
}
}
}

self_cell::self_cell!(
pub(crate) struct ScopeTreeCell {
owner: Bump,
#[covariant]
dependent: ScopeTreeInner,
}
);

pub(crate) struct ScopeTreeInner<'cell> {
/// Symbol bindings in a scope.
///
/// A binding is a mapping from an identifier name to its [`SymbolId`]
bindings: IndexVec<ScopeId, Bindings>,

pub(crate) root_unresolved_references: UnresolvedReferences,
pub(crate) bindings: IndexVec<ScopeId, Bindings<'cell>>,
}

impl ScopeTree {
Expand Down Expand Up @@ -200,7 +228,7 @@ impl ScopeTree {

/// Check if a symbol is declared in a certain scope.
pub fn has_binding(&self, scope_id: ScopeId, name: &str) -> bool {
self.bindings[scope_id].contains_key(name)
self.cell.borrow_dependent().bindings[scope_id].contains_key(name)
}

/// Get the symbol bound to an identifier name in a scope.
Expand All @@ -212,7 +240,7 @@ impl ScopeTree {
///
/// [`find_binding`]: ScopeTree::find_binding
pub fn get_binding(&self, scope_id: ScopeId, name: &str) -> Option<SymbolId> {
self.bindings[scope_id].get(name).copied()
self.cell.borrow_dependent().bindings[scope_id].get(name).copied()
}

/// Find a binding by name in a scope or its ancestors.
Expand All @@ -221,7 +249,7 @@ impl ScopeTree {
/// found. If no binding is found, [`None`] is returned.
pub fn find_binding(&self, scope_id: ScopeId, name: &str) -> Option<SymbolId> {
for scope_id in self.ancestors(scope_id) {
if let Some(&symbol_id) = self.bindings[scope_id].get(name) {
if let Some(symbol_id) = self.get_binding(scope_id, name) {
return Some(symbol_id);
}
}
Expand All @@ -231,7 +259,7 @@ impl ScopeTree {
/// Get all bound identifiers in a scope.
#[inline]
pub fn get_bindings(&self, scope_id: ScopeId) -> &Bindings {
&self.bindings[scope_id]
&self.cell.borrow_dependent().bindings[scope_id]
}

/// Get the ID of the [`AstNode`] that created a scope.
Expand All @@ -247,21 +275,24 @@ impl ScopeTree {
/// If you only want bindings in a specific scope, use [`iter_bindings_in`].
///
/// [`iter_bindings_in`]: ScopeTree::iter_bindings_in
pub fn iter_bindings(&self) -> impl Iterator<Item = (ScopeId, SymbolId, &'_ CompactStr)> + '_ {
self.bindings.iter_enumerated().flat_map(|(scope_id, bindings)| {
bindings.iter().map(move |(name, &symbol_id)| (scope_id, symbol_id, name))
pub fn iter_bindings(&self) -> impl Iterator<Item = (ScopeId, SymbolId, &str)> + '_ {
self.cell.borrow_dependent().bindings.iter_enumerated().flat_map(|(scope_id, bindings)| {
bindings.iter().map(move |(&name, &symbol_id)| (scope_id, symbol_id, name))
})
}

/// Iterate over bindings declared inside a scope.
#[inline]
pub fn iter_bindings_in(&self, scope_id: ScopeId) -> impl Iterator<Item = SymbolId> + '_ {
self.bindings[scope_id].values().copied()
self.cell.borrow_dependent().bindings[scope_id].values().copied()
}

#[inline]
pub(crate) fn get_bindings_mut(&mut self, scope_id: ScopeId) -> &mut Bindings {
&mut self.bindings[scope_id]
pub(crate) fn insert_binding(&mut self, scope_id: ScopeId, name: &str, symbol_id: SymbolId) {
self.cell.with_dependent_mut(|allocator, inner| {
let name = allocator.alloc_str(name);
inner.bindings[scope_id].insert(name, symbol_id);
});
}

/// Return whether this `ScopeTree` has child IDs recorded
Expand Down Expand Up @@ -307,7 +338,9 @@ impl ScopeTree {
) -> ScopeId {
let scope_id = self.parent_ids.push(parent_id);
self.flags.push(flags);
self.bindings.push(Bindings::default());
self.cell.with_dependent_mut(|allocator, inner| {
inner.bindings.push(Bindings::with_hasher_in(FxBuildHasher, allocator));
});
self.node_ids.push(node_id);
if self.build_child_ids {
self.child_ids.push(vec![]);
Expand All @@ -321,21 +354,28 @@ impl ScopeTree {
/// Add a binding to a scope.
///
/// [`binding`]: Bindings
pub fn add_binding(&mut self, scope_id: ScopeId, name: CompactStr, symbol_id: SymbolId) {
self.bindings[scope_id].insert(name, symbol_id);
pub fn add_binding(&mut self, scope_id: ScopeId, name: &str, symbol_id: SymbolId) {
self.cell.with_dependent_mut(|allocator, inner| {
let name = allocator.alloc_str(name);
inner.bindings[scope_id].insert(name, symbol_id);
});
}

/// Remove an existing binding from a scope.
pub fn remove_binding(&mut self, scope_id: ScopeId, name: &CompactStr) {
self.bindings[scope_id].remove(name);
pub fn remove_binding(&mut self, scope_id: ScopeId, name: &str) {
self.cell.with_dependent_mut(|_allocator, inner| {
inner.bindings[scope_id].remove(name);
});
}

/// Move a binding from one scope to another.
pub fn move_binding(&mut self, from: ScopeId, to: ScopeId, name: &str) {
let from_map = &mut self.bindings[from];
if let Some((name, symbol_id)) = from_map.remove_entry(name) {
self.bindings[to].insert(name, symbol_id);
}
self.cell.with_dependent_mut(|_allocator, inner| {
let from_map = &mut inner.bindings[from];
if let Some((name, symbol_id)) = from_map.remove_entry(name) {
inner.bindings[to].insert(name, symbol_id);
}
});
}

/// Rename a binding to a new name.
Expand All @@ -353,24 +393,29 @@ impl ScopeTree {
scope_id: ScopeId,
symbol_id: SymbolId,
old_name: &str,
new_name: CompactStr,
new_name: &str,
) {
let bindings = &mut self.bindings[scope_id];
// Insert on end
let existing_symbol_id = bindings.insert(new_name, symbol_id);
debug_assert!(existing_symbol_id.is_none());
// Remove old entry. `swap_remove` swaps the last entry into the place of removed entry.
// We just inserted the new entry as last, so the new entry takes the place of the old.
// Order of entries is same as before, only the key has changed.
let old_symbol_id = bindings.remove(old_name);
debug_assert_eq!(old_symbol_id, Some(symbol_id));
self.cell.with_dependent_mut(|allocator, inner| {
let new_name = allocator.alloc_str(new_name);
let bindings = &mut inner.bindings[scope_id];
// Insert on end
let existing_symbol_id = bindings.insert(new_name, symbol_id);
debug_assert!(existing_symbol_id.is_none());
// Remove old entry. `swap_remove` swaps the last entry into the place of removed entry.
// We just inserted the new entry as last, so the new entry takes the place of the old.
// Order of entries is same as before, only the key has changed.
let old_symbol_id = bindings.remove(old_name);
debug_assert_eq!(old_symbol_id, Some(symbol_id));
});
}

/// Reserve memory for an `additional` number of scopes.
pub fn reserve(&mut self, additional: usize) {
self.parent_ids.reserve(additional);
self.flags.reserve(additional);
self.bindings.reserve(additional);
self.cell.with_dependent_mut(|_allocator, inner| {
inner.bindings.reserve(additional);
});
self.node_ids.reserve(additional);
if self.build_child_ids {
self.child_ids.reserve(additional);
Expand Down
Loading

0 comments on commit 7aebed0

Please sign in to comment.