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 UnresolvedReferences in allocator #8046

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
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
Loading