Skip to content

Commit

Permalink
perf(semantic): allocate UnresolvedReferences in allocator (#8046)
Browse files Browse the repository at this point in the history
  • Loading branch information
Boshen committed Dec 20, 2024
1 parent 2e8872c commit 2736657
Show file tree
Hide file tree
Showing 12 changed files with 77 additions and 56 deletions.
2 changes: 1 addition & 1 deletion crates/oxc_linter/src/rules/eslint/no_global_assign.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ impl Rule for NoGlobalAssign {
for &reference_id in reference_id_list {
let reference = symbol_table.get_reference(reference_id);
if reference.is_write()
&& !self.excludes.contains(name)
&& !self.excludes.iter().any(|n| n == name)
&& ctx.env_contains_var(name)
{
ctx.diagnostic(no_global_assign_diagnostic(
Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_linter/src/rules/jest/no_jasmine_globals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ impl Rule for NoJasmineGlobals {
.scopes()
.root_unresolved_references()
.iter()
.filter(|(key, _)| NON_JASMINE_PROPERTY_NAMES.contains(&key.as_str()));
.filter(|(key, _)| NON_JASMINE_PROPERTY_NAMES.contains(key));

for (name, reference_ids) in jasmine_references {
for &reference_id in reference_ids {
Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_linter/src/utils/jest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ fn collect_ids_referenced_to_global<'c>(
.scopes()
.root_unresolved_references()
.iter()
.filter(|(name, _)| JEST_METHOD_NAMES.contains(name.as_str()))
.filter(|(name, _)| JEST_METHOD_NAMES.contains(name))
.flat_map(|(_, reference_ids)| reference_ids.iter().copied())
}

Expand Down
9 changes: 3 additions & 6 deletions crates/oxc_semantic/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -279,12 +279,9 @@ impl<'a> SemanticBuilder<'a> {
if self.check_syntax_error && !self.source_type.is_typescript() {
checker::check_unresolved_exports(&self);
}
self.scope.root_unresolved_references = self
.unresolved_references
.into_root()
.into_iter()
.map(|(k, v)| (k.into(), v))
.collect();
self.scope.set_root_unresolved_references(
self.unresolved_references.into_root().into_iter().map(|(k, v)| (k.as_str(), v)),
);

let jsdoc = if self.build_jsdoc { self.jsdoc.build() } else { JSDocFinder::default() };

Expand Down
63 changes: 45 additions & 18 deletions crates/oxc_semantic/src/scope.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
use std::{fmt, mem};

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

use oxc_allocator::{Allocator, Vec as ArenaVec};
use oxc_index::{Idx, IndexVec};
use oxc_span::CompactStr;
use oxc_syntax::{
node::NodeId,
reference::ReferenceId,
Expand All @@ -13,7 +12,8 @@ use oxc_syntax::{
};

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

/// Scope Tree
///
Expand All @@ -37,8 +37,6 @@ pub struct ScopeTree {

flags: IndexVec<ScopeId, ScopeFlags>,

pub(crate) root_unresolved_references: UnresolvedReferences,

pub(crate) cell: ScopeTreeCell,
}

Expand All @@ -55,10 +53,13 @@ impl Default for ScopeTree {
build_child_ids: false,
node_ids: IndexVec::new(),
flags: IndexVec::new(),
root_unresolved_references: UnresolvedReferences::default(),
cell: ScopeTreeCell::new(Allocator::default(), |allocator| ScopeTreeInner {
bindings: IndexVec::new(),
child_ids: ArenaVec::new_in(allocator),
root_unresolved_references: UnresolvedReferences::with_hasher_in(
FxBuildHasher,
allocator,
),
}),
}
}
Expand All @@ -80,6 +81,8 @@ pub(crate) struct ScopeTreeInner<'cell> {

/// Maps a scope to direct children scopes.
child_ids: ArenaVec<'cell, ArenaVec<'cell, ScopeId>>,

pub(crate) root_unresolved_references: UnresolvedReferences<'cell>,
}

impl ScopeTree {
Expand Down Expand Up @@ -131,13 +134,28 @@ impl ScopeTree {

#[inline]
pub fn root_unresolved_references(&self) -> &UnresolvedReferences {
&self.root_unresolved_references
&self.cell.borrow_dependent().root_unresolved_references
}

pub fn root_unresolved_references_ids(
&self,
) -> impl Iterator<Item = impl Iterator<Item = ReferenceId> + '_> + '_ {
self.root_unresolved_references.values().map(|v| v.iter().copied())
self.cell.borrow_dependent().root_unresolved_references.values().map(|v| v.iter().copied())
}

pub(crate) fn set_root_unresolved_references<'a>(
&mut self,
entries: impl Iterator<Item = (&'a str, Vec<ReferenceId>)>,
) {
self.cell.with_dependent_mut(|allocator, inner| {
for (k, v) in entries {
let k = allocator.alloc_str(k);
let v = ArenaVec::from_iter_in(v, allocator);
inner.root_unresolved_references.insert(k, v);
}
// =
// .extend_from(entries.map(|(k, v)| (allocator.alloc(k),)))
});
}

/// Delete an unresolved reference.
Expand All @@ -153,14 +171,16 @@ impl ScopeTree {
// but `map.entry` requires an owned key to be provided. Currently we use `CompactStr`s as keys
// which are not cheap to construct, so this is best we can do at present.
// TODO: Switch to `Entry` API once we use `&str`s or `Atom`s as keys.
let reference_ids = self.root_unresolved_references.get_mut(name).unwrap();
if reference_ids.len() == 1 {
assert!(reference_ids[0] == reference_id);
self.root_unresolved_references.remove(name);
} else {
let index = reference_ids.iter().position(|&id| id == reference_id).unwrap();
reference_ids.swap_remove(index);
}
self.cell.with_dependent_mut(|_allocator, inner| {
let reference_ids = inner.root_unresolved_references.get_mut(name).unwrap();
if reference_ids.len() == 1 {
assert!(reference_ids[0] == reference_id);
inner.root_unresolved_references.remove(name);
} else {
let index = reference_ids.iter().position(|&id| id == reference_id).unwrap();
reference_ids.swap_remove(index);
}
});
}

#[inline]
Expand Down Expand Up @@ -234,8 +254,15 @@ impl ScopeTree {
self.get_binding(self.root_scope_id(), name)
}

pub fn add_root_unresolved_reference(&mut self, name: CompactStr, reference_id: ReferenceId) {
self.root_unresolved_references.entry(name).or_default().push(reference_id);
pub fn add_root_unresolved_reference(&mut self, name: &str, reference_id: ReferenceId) {
self.cell.with_dependent_mut(|allocator, inner| {
let name = allocator.alloc_str(name);
inner
.root_unresolved_references
.entry(name)
.or_insert_with(|| ArenaVec::new_in(allocator))
.push(reference_id);
});
}

/// Check if a symbol is declared in a certain scope.
Expand Down
3 changes: 1 addition & 2 deletions crates/oxc_transformer/src/jsx/jsx_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1081,8 +1081,7 @@ fn get_read_identifier_reference<'a>(
name: Atom<'a>,
ctx: &mut TraverseCtx<'a>,
) -> Expression<'a> {
let reference_id =
ctx.create_reference_in_current_scope(name.to_compact_str(), ReferenceFlags::Read);
let reference_id = ctx.create_reference_in_current_scope(name.as_str(), ReferenceFlags::Read);
let ident = ctx.ast.alloc_identifier_reference_with_reference_id(span, name, reference_id);
Expression::Identifier(ident)
}
Expand Down
6 changes: 2 additions & 4 deletions crates/oxc_transformer/src/jsx/refresh.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,17 +68,15 @@ impl<'a> RefreshIdentifierResolver<'a> {
pub fn to_expression(&self, ctx: &mut TraverseCtx<'a>) -> Expression<'a> {
match self {
Self::Identifier(ident) => {
let reference_id =
ctx.create_unbound_reference(ident.name.to_compact_str(), ReferenceFlags::Read);
let reference_id = ctx.create_unbound_reference(&ident.name, ReferenceFlags::Read);
Expression::Identifier(ctx.ast.alloc_identifier_reference_with_reference_id(
ident.span,
ident.name.clone(),
reference_id,
))
}
Self::Member((ident, property)) => {
let reference_id =
ctx.create_unbound_reference(ident.name.to_compact_str(), ReferenceFlags::Read);
let reference_id = ctx.create_unbound_reference(&ident.name, ReferenceFlags::Read);
let ident =
Expression::Identifier(ctx.ast.alloc_identifier_reference_with_reference_id(
ident.span,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ impl<'a> InjectGlobalVariables<'a> {
} else if self.replaced_dot_defines.iter().any(|d| d.0 == i.specifier.local()) {
false
} else {
scopes.root_unresolved_references().contains_key(i.specifier.local())
scopes.root_unresolved_references().contains_key(i.specifier.local().as_str())
}
})
.cloned()
Expand Down
6 changes: 3 additions & 3 deletions crates/oxc_transformer/src/typescript/module.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use oxc_ast::{ast::*, NONE};
use oxc_span::{CompactStr, SPAN};
use oxc_span::SPAN;
use oxc_syntax::reference::ReferenceFlags;
use oxc_traverse::{Traverse, TraverseCtx};

Expand Down Expand Up @@ -58,8 +58,8 @@ impl<'a, 'ctx> TypeScriptModule<'a, 'ctx> {

// module.exports
let module_exports = {
let reference_id = ctx
.create_reference_in_current_scope(CompactStr::new("module"), ReferenceFlags::Read);
let reference_id =
ctx.create_reference_in_current_scope("module", ReferenceFlags::Read);
let reference =
ctx.ast.alloc_identifier_reference_with_reference_id(SPAN, "module", reference_id);
let object = Expression::Identifier(reference);
Expand Down
12 changes: 4 additions & 8 deletions crates/oxc_traverse/src/context/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -488,11 +488,7 @@ impl<'a> TraverseCtx<'a> {
///
/// This is a shortcut for `ctx.scoping.create_unbound_reference`.
#[inline]
pub fn create_unbound_reference(
&mut self,
name: CompactStr,
flags: ReferenceFlags,
) -> ReferenceId {
pub fn create_unbound_reference(&mut self, name: &str, flags: ReferenceFlags) -> ReferenceId {
self.scoping.create_unbound_reference(name, flags)
}

Expand All @@ -503,7 +499,7 @@ impl<'a> TraverseCtx<'a> {
name: Atom<'a>,
flags: ReferenceFlags,
) -> IdentifierReference<'a> {
let reference_id = self.create_unbound_reference(name.to_compact_str(), flags);
let reference_id = self.create_unbound_reference(name.as_str(), flags);
self.ast.identifier_reference_with_reference_id(span, name, reference_id)
}

Expand All @@ -527,7 +523,7 @@ impl<'a> TraverseCtx<'a> {
#[inline]
pub fn create_reference(
&mut self,
name: CompactStr,
name: &str,
symbol_id: Option<SymbolId>,
flags: ReferenceFlags,
) -> ReferenceId {
Expand Down Expand Up @@ -576,7 +572,7 @@ impl<'a> TraverseCtx<'a> {
#[inline]
pub fn create_reference_in_current_scope(
&mut self,
name: CompactStr,
name: &str,
flags: ReferenceFlags,
) -> ReferenceId {
self.scoping.create_reference_in_current_scope(name, flags)
Expand Down
14 changes: 5 additions & 9 deletions crates/oxc_traverse/src/context/scoping.rs
Original file line number Diff line number Diff line change
Expand Up @@ -313,11 +313,7 @@ impl TraverseScoping {
}

/// Create an unbound reference
pub fn create_unbound_reference(
&mut self,
name: CompactStr,
flags: ReferenceFlags,
) -> ReferenceId {
pub fn create_unbound_reference(&mut self, name: &str, flags: ReferenceFlags) -> ReferenceId {
let reference = Reference::new(NodeId::DUMMY, flags);
let reference_id = self.symbols.create_reference(reference);
self.scopes.add_root_unresolved_reference(name, reference_id);
Expand All @@ -330,7 +326,7 @@ impl TraverseScoping {
/// or `TraverseCtx::create_unbound_reference`.
pub fn create_reference(
&mut self,
name: CompactStr,
name: &str,
symbol_id: Option<SymbolId>,
flags: ReferenceFlags,
) -> ReferenceId {
Expand All @@ -344,10 +340,10 @@ impl TraverseScoping {
/// Create reference in current scope, looking up binding for `name`
pub fn create_reference_in_current_scope(
&mut self,
name: CompactStr,
name: &str,
flags: ReferenceFlags,
) -> ReferenceId {
let symbol_id = self.scopes.find_binding(self.current_scope_id, name.as_str());
let symbol_id = self.scopes.find_binding(self.current_scope_id, name);
self.create_reference(name, symbol_id, flags)
}

Expand Down Expand Up @@ -433,7 +429,7 @@ impl TraverseScoping {
self.scopes
.root_unresolved_references()
.keys()
.map(CompactStr::as_str)
.copied()
.chain(self.symbols.names())
.filter(|name| name.as_bytes().first() == Some(&b'_'))
.map(CompactStr::from)
Expand Down
12 changes: 10 additions & 2 deletions tasks/transform_checker/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -506,8 +506,12 @@ impl PostTransformChecker<'_, '_> {

fn check_unresolved_references(&mut self) {
let unresolved_names = self.get_static_pair(|scoping| {
let mut names =
scoping.scopes.root_unresolved_references().keys().cloned().collect::<Vec<_>>();
let mut names = scoping
.scopes
.root_unresolved_references()
.keys()
.map(ToString::to_string)
.collect::<Vec<_>>();
names.sort_unstable();
names
});
Expand All @@ -521,6 +525,10 @@ impl PostTransformChecker<'_, '_> {
if let Some(reference_ids_rebuilt) =
self.scoping_rebuilt.scopes.root_unresolved_references().get(name)
{
let reference_ids_after_transform =
reference_ids_after_transform.iter().copied().collect::<Vec<_>>();
let reference_ids_rebuilt =
reference_ids_rebuilt.iter().copied().collect::<Vec<_>>();
let reference_ids = Pair::new(reference_ids_after_transform, reference_ids_rebuilt);
if self.remap_reference_ids_sets(&reference_ids).is_mismatch() {
self.errors.push_mismatch_single(
Expand Down

0 comments on commit 2736657

Please sign in to comment.