Skip to content

Commit

Permalink
refactor(semantic): allocate data in Allocator
Browse files Browse the repository at this point in the history
  • Loading branch information
Boshen committed Dec 20, 2024
1 parent 8547e02 commit 0864eb2
Show file tree
Hide file tree
Showing 9 changed files with 129 additions and 59 deletions.
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 acutally safe, but for enabling `Send` for downstream crates.

Check warning on line 75 in crates/oxc_allocator/src/lib.rs

View workflow job for this annotation

GitHub Actions / Spell Check

"acutally" should be "actually".
unsafe impl Send for Allocator {}
/// SAFETY: Not acutally safe, but for enabling `Sync` for downstream crates.

Check warning on line 77 in crates/oxc_allocator/src/lib.rs

View workflow job for this annotation

GitHub Actions / Spell Check

"acutally" should be "actually".
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 acutally safe, but for enabling `Send` for downstream crates.

Check warning on line 35 in crates/oxc_allocator/src/vec.rs

View workflow job for this annotation

GitHub Actions / Spell Check

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

Check warning on line 37 in crates/oxc_allocator/src/vec.rs

View workflow job for this annotation

GitHub Actions / Spell Check

"acutally" should be "actually".
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

0 comments on commit 0864eb2

Please sign in to comment.