Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf(semantic): allocate symbol data in Allocator #8012

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions crates/oxc_allocator/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Bump> for Allocator {
fn from(bump: Bump) -> Self {
Self { bump }
Expand Down
5 changes: 5 additions & 0 deletions crates/oxc_allocator/src/vec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,11 @@ use crate::{Allocator, Box, String};
#[derive(PartialEq, Eq)]
pub struct Vec<'alloc, T>(ManuallyDrop<vec::Vec<T, &'alloc Bump>>);

/// SAFETY: Not actually safe, but for enabling `Send` for downstream crates.
unsafe impl<T> Send for Vec<'_, T> {}
/// SAFETY: Not actually safe, but for enabling `Sync` for downstream crates.
unsafe impl<T> Sync for Vec<'_, T> {}

impl<'alloc, T> Vec<'alloc, T> {
/// Constructs a new, empty `Vec<T>`.
///
Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_mangler/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
}
}
Expand Down
16 changes: 6 additions & 10 deletions crates/oxc_semantic/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
});
Expand Down
8 changes: 7 additions & 1 deletion crates/oxc_semantic/src/scope.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::mem;
use std::{fmt, mem};

use rustc_hash::{FxBuildHasher, FxHashMap};

Expand Down Expand Up @@ -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 {
Expand Down
122 changes: 88 additions & 34 deletions crates/oxc_semantic/src/symbol.rs
Original file line number Diff line number Diff line change
@@ -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},
Expand All @@ -17,20 +18,56 @@ use oxc_syntax::{
/// Most symbols won't have redeclarations, so instead of storing `Vec<Span>` directly in
/// `redeclare_variables` (32 bytes per symbol), store `Option<RedeclarationId>` (4 bytes).
/// That ID indexes into `redeclarations` where the actual `Vec<Span>` is stored.
#[derive(Debug, Default)]
pub struct SymbolTable {
pub(crate) spans: IndexVec<SymbolId, Span>,
pub(crate) names: IndexVec<SymbolId, CompactStr>,
pub(crate) flags: IndexVec<SymbolId, SymbolFlags>,
pub(crate) scope_ids: IndexVec<SymbolId, ScopeId>,
/// Pointer to the AST Node where this symbol is declared
pub(crate) declarations: IndexVec<SymbolId, NodeId>,
pub resolved_references: IndexVec<SymbolId, Vec<ReferenceId>>,
redeclarations: IndexVec<SymbolId, Option<RedeclarationId>>,

redeclaration_spans: IndexVec<RedeclarationId, Vec<Span>>,

pub references: IndexVec<ReferenceId, Reference>,

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 {
Expand All @@ -46,12 +83,12 @@ impl SymbolTable {
self.spans.is_empty()
}

pub fn names(&self) -> impl Iterator<Item = &CompactStr> + '_ {
self.names.iter()
pub fn names(&self) -> impl Iterator<Item = &str> + '_ {
self.inner.borrow_dependent().names.iter().map(Atom::as_str)
}

pub fn resolved_references(&self) -> impl Iterator<Item = &Vec<ReferenceId>> + '_ {
self.resolved_references.iter()
pub fn resolved_references(&self) -> impl Iterator<Item = &ArenaVec<'_, ReferenceId>> + '_ {
self.inner.borrow_dependent().resolved_references.iter()
}

/// Iterate over all symbol IDs in this table.
Expand Down Expand Up @@ -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.
Expand All @@ -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
Expand Down Expand Up @@ -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));
});
};
}

Expand Down Expand Up @@ -213,16 +264,16 @@ 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<ReferenceId> {
&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.
pub fn get_resolved_references(
&self,
symbol_id: SymbolId,
) -> impl DoubleEndedIterator<Item = &Reference> + '_ {
self.resolved_references[symbol_id]
self.get_resolved_reference_ids(symbol_id)
.iter()
.map(|&reference_id| &self.references[reference_id])
}
Expand All @@ -241,29 +292,32 @@ 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.
///
/// # 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);
}
}
Expand Down
17 changes: 6 additions & 11 deletions crates/oxc_traverse/src/context/scoping.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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());
}
}

Expand Down Expand Up @@ -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()
}
}
Expand Down
6 changes: 5 additions & 1 deletion crates/oxc_wasm/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::<Vec<_>>(),
references: symbols
.get_resolved_reference_ids(symbol_id)
.iter()
Expand Down
7 changes: 6 additions & 1 deletion tasks/transform_checker/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::<Vec<_>>()
});
if self.remap_reference_ids_sets(&reference_ids).is_mismatch() {
self.errors.push_mismatch(
Expand Down
Loading