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): avoid repeated hashing resolving bindings #4238

Closed
1 change: 1 addition & 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 @@ -115,6 +115,7 @@ dashmap = "6.0.0"
flate2 = "1.0.30"
futures = "0.3.30"
glob = "0.3.1"
hashbrown = "0.14.5"
ignore = "0.4.22"
itertools = "0.13.0"
jemallocator = "0.5.4"
Expand Down
5 changes: 3 additions & 2 deletions crates/oxc_linter/src/snapshots/no_confusing_set_timeout.snap
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
---
source: crates/oxc_linter/src/tester.rs
assertion_line: 216
---
⚠ eslint-plugin-jest(no-confusing-set-timeout)
╭─[no_confusing_set_timeout.tsx:10:17]
Expand Down Expand Up @@ -35,7 +36,7 @@ source: crates/oxc_linter/src/tester.rs
· ───────────────
11 │
╰────
help: Do not call `jest.setTimeout` multiple times, as only the last call will have an effect
help: `jest.setTimeout` should be placed before any other jest methods

⚠ eslint-plugin-jest(no-confusing-set-timeout)
╭─[no_confusing_set_timeout.tsx:10:17]
Expand All @@ -44,7 +45,7 @@ source: crates/oxc_linter/src/tester.rs
· ───────────────
11 │
╰────
help: `jest.setTimeout` should be placed before any other jest methods
help: Do not call `jest.setTimeout` multiple times, as only the last call will have an effect

⚠ eslint-plugin-jest(no-confusing-set-timeout)
╭─[no_confusing_set_timeout.tsx:3:21]
Expand Down
1 change: 1 addition & 0 deletions crates/oxc_semantic/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ oxc_diagnostics = { workspace = true }
oxc_index = { workspace = true }
oxc_allocator = { workspace = true }

hashbrown = { workspace = true }
indexmap = { workspace = true }
phf = { workspace = true, features = ["macros"] }
rustc-hash = { workspace = true }
Expand Down
38 changes: 18 additions & 20 deletions crates/oxc_semantic/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,13 @@ use crate::{
checker,
class::ClassTableBuilder,
diagnostics::redeclaration,
hash::TempUnresolvedReferences,
jsdoc::JSDocBuilder,
label::LabelBuilder,
module_record::ModuleRecordBuilder,
node::{AstNodeId, AstNodes, NodeFlags},
reference::{Reference, ReferenceFlag, ReferenceId},
scope::{ScopeFlags, ScopeId, ScopeTree, UnresolvedReferences},
scope::{ScopeFlags, ScopeId, ScopeTree},
symbol::{SymbolFlags, SymbolId, SymbolTable},
JSDocFinder, Semantic,
};
Expand Down Expand Up @@ -74,10 +75,10 @@ pub struct SemanticBuilder<'a> {
pub symbols: SymbolTable,

// Stack used to accumulate unresolved refs while traversing scopes.
// Indexed by scope depth. We recycle `UnresolvedReferences` instances during traversal
// Indexed by scope depth. We recycle `TempUnresolvedReferences` instances during traversal
// to reduce allocations, so the stack grows to maximum scope depth, but never shrinks.
// See: <https://github.com/oxc-project/oxc/issues/4169>
unresolved_references: Vec<UnresolvedReferences>,
unresolved_references: Vec<TempUnresolvedReferences>,

pub(crate) module_record: Arc<ModuleRecord>,

Expand Down Expand Up @@ -110,7 +111,7 @@ impl<'a> SemanticBuilder<'a> {
// This is just an estimate of a good initial size, but certainly better than
// `Vec`'s default initial capacity of 4.
let mut unresolved_references = vec![];
unresolved_references.resize_with(16, Default::default);
unresolved_references.resize_with(16, TempUnresolvedReferences::default);

let trivias = Trivias::default();
Self {
Expand Down Expand Up @@ -198,11 +199,13 @@ impl<'a> SemanticBuilder<'a> {
if self.check_syntax_error {
checker::check_module_record(&self);
}
}

debug_assert_eq!(self.current_scope_depth, 0);
self.scope.root_unresolved_references =
self.unresolved_references.into_iter().next().unwrap();
// Convert remaining unresolved references to a standard hash map
debug_assert_eq!(self.current_scope_depth, 0);
let mut unresolved_references = self.unresolved_references.into_iter().next().unwrap();
self.scope.root_unresolved_references =
unresolved_references.drain().map(|line| (line.name, line.reference_ids)).collect();
}

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

Expand Down Expand Up @@ -339,10 +342,7 @@ impl<'a> SemanticBuilder<'a> {
let reference_name = reference.name().clone();
let reference_id = self.symbols.create_reference(reference);

self.unresolved_references[self.current_scope_depth]
.entry(reference_name)
.or_default()
.push(reference_id);
self.unresolved_references[self.current_scope_depth].insert(reference_name, reference_id);
reference_id
}

Expand Down Expand Up @@ -370,18 +370,16 @@ impl<'a> SemanticBuilder<'a> {
let current_refs = iter.next().unwrap();

let bindings = self.scope.get_bindings(self.current_scope_id);
for (name, reference_ids) in current_refs.drain() {
for line in current_refs.drain() {
// Try to resolve a reference.
// If unresolved, transfer it to parent scope's unresolved references.
if let Some(symbol_id) = bindings.get(&name).copied() {
for reference_id in &reference_ids {
if let Some(symbol_id) = bindings.get(&line.name).copied() {
for reference_id in &line.reference_ids {
self.symbols.references[*reference_id].set_symbol_id(symbol_id);
}
self.symbols.resolved_references[symbol_id].extend(reference_ids);
} else if let Some(parent_reference_ids) = parent_refs.get_mut(&name) {
parent_reference_ids.extend(reference_ids);
self.symbols.resolved_references[symbol_id].extend(line.reference_ids);
} else {
parent_refs.insert(name, reference_ids);
parent_refs.extend(line.name, line.hash, line.reference_ids);
}
}
}
Expand Down Expand Up @@ -442,7 +440,7 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> {
// `self.unresolved_references[self.current_scope_depth]` is initialized
self.current_scope_depth += 1;
if self.unresolved_references.len() <= self.current_scope_depth {
self.unresolved_references.push(UnresolvedReferences::default());
self.unresolved_references.push(TempUnresolvedReferences::default());
}
}

Expand Down
77 changes: 77 additions & 0 deletions crates/oxc_semantic/src/hash.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
use std::hash::Hasher;

use hashbrown::hash_table::{Drain, HashTable};
use rustc_hash::FxHasher;

use oxc_span::CompactStr;

use crate::reference::ReferenceId;

#[derive(Clone, Copy, PartialEq)]
pub struct IdentifierHash(u64);

impl IdentifierHash {
fn new(name: &str) -> Self {
let mut hasher = FxHasher::default();
hasher.write(name.as_bytes());
Self(hasher.finish())
}
}

pub struct Line {
pub name: CompactStr,
pub hash: IdentifierHash,
pub reference_ids: Vec<ReferenceId>,
}

#[derive(Default)]
pub struct TempUnresolvedReferences {
inner: HashTable<Line>,
}

impl TempUnresolvedReferences {
#[allow(dead_code)]
pub fn get(&self, name: &str, hash: IdentifierHash) -> Option<&Vec<ReferenceId>> {
self.inner
.find(hash.0, |line| line.hash == hash && line.name.as_ref() == name)
.map(|entry| &entry.reference_ids)
}

#[allow(dead_code)]
pub fn get_mut(&mut self, name: &str, hash: IdentifierHash) -> Option<&mut Vec<ReferenceId>> {
self.inner
.find_mut(hash.0, |line| line.hash == hash && line.name.as_ref() == name)
.map(|entry| &mut entry.reference_ids)
}

pub fn insert(&mut self, name: CompactStr, reference_id: ReferenceId) {
let hash = IdentifierHash::new(&name);
if let Some(line) = self.inner.find_mut(hash.0, |line| line.name == name) {
line.reference_ids.push(reference_id);
} else {
self.inner.insert_unique(
hash.0,
Line { name, hash, reference_ids: vec![reference_id] },
|line| line.hash.0,
);
}
}

pub fn extend(
&mut self,
name: CompactStr,
hash: IdentifierHash,
reference_ids: Vec<ReferenceId>,
) {
if let Some(line) = self.inner.find_mut(hash.0, |line| line.name == name) {
line.reference_ids.extend(reference_ids);
} else {
self.inner
.insert_unique(hash.0, Line { name, hash, reference_ids }, |line| line.hash.0);
}
}

pub fn drain(&mut self) -> Drain<Line> {
self.inner.drain()
}
}
1 change: 1 addition & 0 deletions crates/oxc_semantic/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ mod builder;
mod checker;
mod class;
mod diagnostics;
mod hash;
mod jsdoc;
mod label;
mod module_record;
Expand Down
Loading