From f8829cd544aebb667f46ce0a6f4962530fd4bba2 Mon Sep 17 00:00:00 2001 From: overlookmotel Date: Fri, 12 Jul 2024 19:21:24 +0100 Subject: [PATCH 01/15] perf(semantic): avoid repeated hashing resolving bindings --- Cargo.lock | 1 + Cargo.toml | 1 + crates/oxc_semantic/Cargo.toml | 1 + crates/oxc_semantic/src/builder.rs | 37 ++++++------ crates/oxc_semantic/src/hash.rs | 90 ++++++++++++++++++++++++++++++ crates/oxc_semantic/src/lib.rs | 1 + 6 files changed, 112 insertions(+), 19 deletions(-) create mode 100644 crates/oxc_semantic/src/hash.rs diff --git a/Cargo.lock b/Cargo.lock index 9cd9083730ff5..71caf06976e93 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1704,6 +1704,7 @@ dependencies = [ name = "oxc_semantic" version = "0.20.0" dependencies = [ + "hashbrown", "indexmap", "insta", "itertools 0.13.0", diff --git a/Cargo.toml b/Cargo.toml index 49810d486c709..c7fd7850e7a1e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -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" diff --git a/crates/oxc_semantic/Cargo.toml b/crates/oxc_semantic/Cargo.toml index 8118711a26b2b..995aa8af87d18 100644 --- a/crates/oxc_semantic/Cargo.toml +++ b/crates/oxc_semantic/Cargo.toml @@ -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 } diff --git a/crates/oxc_semantic/src/builder.rs b/crates/oxc_semantic/src/builder.rs index ca19d1258812b..7d0f823524e1c 100644 --- a/crates/oxc_semantic/src/builder.rs +++ b/crates/oxc_semantic/src/builder.rs @@ -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, }; @@ -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: - unresolved_references: Vec, + unresolved_references: Vec, pub(crate) module_record: Arc, @@ -198,11 +199,14 @@ 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(); + for line in unresolved_references.drain() { + self.scope.root_unresolved_references.insert(line.name, line.reference_ids); + } + } let jsdoc = if self.build_jsdoc { self.jsdoc.build() } else { JSDocFinder::default() }; @@ -339,10 +343,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 } @@ -370,18 +371,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); } } } @@ -442,7 +441,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()); } } diff --git a/crates/oxc_semantic/src/hash.rs b/crates/oxc_semantic/src/hash.rs new file mode 100644 index 0000000000000..602fbd6e5ef9e --- /dev/null +++ b/crates/oxc_semantic/src/hash.rs @@ -0,0 +1,90 @@ +use std::hash::Hash; +use std::hash::Hasher; + +use hashbrown::hash_table::{Drain, Entry, 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(); + name.hash(&mut hasher); + let hash = hasher.finish(); + Self(hash) + } +} + +pub struct Line { + pub name: CompactStr, + pub hash: IdentifierHash, + pub reference_ids: Vec, +} + +#[derive(Default)] +pub struct TempUnresolvedReferences { + inner: HashTable, +} + +impl TempUnresolvedReferences { + #[allow(dead_code)] + pub fn get(&self, name: &str, hash: IdentifierHash) -> Option<&Vec> { + self.inner + .find(hash.0, |entry| entry.hash == hash && entry.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> { + self.inner + .find_mut(hash.0, |entry| entry.hash == hash && entry.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); + let entry = self.inner.entry( + hash.0, + |entry| entry.hash == hash && entry.name.as_ref() == name, + |entry| entry.hash.0, + ); + match entry { + Entry::Occupied(mut entry) => { + entry.get_mut().reference_ids.push(reference_id); + } + Entry::Vacant(entry) => { + entry.insert(Line { name, hash, reference_ids: vec![reference_id] }); + } + } + } + + pub fn extend( + &mut self, + name: CompactStr, + hash: IdentifierHash, + reference_ids: Vec, + ) { + let entry = self.inner.entry( + hash.0, + |entry| entry.hash == hash && entry.name.as_ref() == name, + |entry| entry.hash.0, + ); + match entry { + Entry::Occupied(mut entry) => { + entry.get_mut().reference_ids.extend(reference_ids); + } + Entry::Vacant(entry) => { + entry.insert(Line { name, hash, reference_ids }); + } + } + } + + pub fn drain(&mut self) -> Drain { + self.inner.drain() + } +} diff --git a/crates/oxc_semantic/src/lib.rs b/crates/oxc_semantic/src/lib.rs index 52376d4ff4974..d5461670bde8d 100644 --- a/crates/oxc_semantic/src/lib.rs +++ b/crates/oxc_semantic/src/lib.rs @@ -3,6 +3,7 @@ mod builder; mod checker; mod class; mod diagnostics; +mod hash; mod jsdoc; mod label; mod module_record; From 04b2acb9eac1d147c190aac866d46a06b295e87c Mon Sep 17 00:00:00 2001 From: overlookmotel Date: Fri, 12 Jul 2024 20:01:05 +0100 Subject: [PATCH 02/15] Simpler hash function --- crates/oxc_semantic/src/hash.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/crates/oxc_semantic/src/hash.rs b/crates/oxc_semantic/src/hash.rs index 602fbd6e5ef9e..c076506c752b9 100644 --- a/crates/oxc_semantic/src/hash.rs +++ b/crates/oxc_semantic/src/hash.rs @@ -1,4 +1,3 @@ -use std::hash::Hash; use std::hash::Hasher; use hashbrown::hash_table::{Drain, Entry, HashTable}; @@ -14,9 +13,8 @@ pub struct IdentifierHash(u64); impl IdentifierHash { fn new(name: &str) -> Self { let mut hasher = FxHasher::default(); - name.hash(&mut hasher); - let hash = hasher.finish(); - Self(hash) + hasher.write(name.as_bytes()); + Self(hasher.finish()) } } From b48168dbb073f5d10a95796a0a827da2bf6fe0c7 Mon Sep 17 00:00:00 2001 From: overlookmotel Date: Fri, 12 Jul 2024 20:04:44 +0100 Subject: [PATCH 03/15] Compare `CompactStr`s directly --- crates/oxc_semantic/src/hash.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/oxc_semantic/src/hash.rs b/crates/oxc_semantic/src/hash.rs index c076506c752b9..1e41efd19a549 100644 --- a/crates/oxc_semantic/src/hash.rs +++ b/crates/oxc_semantic/src/hash.rs @@ -48,7 +48,7 @@ impl TempUnresolvedReferences { let hash = IdentifierHash::new(&name); let entry = self.inner.entry( hash.0, - |entry| entry.hash == hash && entry.name.as_ref() == name, + |entry| entry.hash == hash && entry.name == name, |entry| entry.hash.0, ); match entry { @@ -69,7 +69,7 @@ impl TempUnresolvedReferences { ) { let entry = self.inner.entry( hash.0, - |entry| entry.hash == hash && entry.name.as_ref() == name, + |entry| entry.hash == hash && entry.name == name, |entry| entry.hash.0, ); match entry { From 2725a0c9aa90be7d33eec8ef3543aa6bd0ab87d5 Mon Sep 17 00:00:00 2001 From: overlookmotel Date: Fri, 12 Jul 2024 20:47:16 +0100 Subject: [PATCH 04/15] Snapshot change --- .../oxc_linter/src/snapshots/no_confusing_set_timeout.snap | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/crates/oxc_linter/src/snapshots/no_confusing_set_timeout.snap b/crates/oxc_linter/src/snapshots/no_confusing_set_timeout.snap index eb0da15879dd6..1b2c0fe59406b 100644 --- a/crates/oxc_linter/src/snapshots/no_confusing_set_timeout.snap +++ b/crates/oxc_linter/src/snapshots/no_confusing_set_timeout.snap @@ -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] @@ -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] @@ -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] From c30dd53bee0f220d3e583b850eed234f230848f8 Mon Sep 17 00:00:00 2001 From: overlookmotel Date: Fri, 12 Jul 2024 22:57:29 +0100 Subject: [PATCH 05/15] Collect into `root_unresolved_references` --- crates/oxc_semantic/src/builder.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/crates/oxc_semantic/src/builder.rs b/crates/oxc_semantic/src/builder.rs index 7d0f823524e1c..91f801857e60e 100644 --- a/crates/oxc_semantic/src/builder.rs +++ b/crates/oxc_semantic/src/builder.rs @@ -203,9 +203,8 @@ impl<'a> SemanticBuilder<'a> { // 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(); - for line in unresolved_references.drain() { - self.scope.root_unresolved_references.insert(line.name, line.reference_ids); - } + 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() }; From 463b7d1d3abd4ca290e17c6c0761cb7d4e52a960 Mon Sep 17 00:00:00 2001 From: overlookmotel Date: Fri, 12 Jul 2024 23:13:10 +0100 Subject: [PATCH 06/15] `resolve_references_for_root_scope` --- crates/oxc_semantic/src/builder.rs | 36 +++++++++++++++++++++--------- 1 file changed, 25 insertions(+), 11 deletions(-) diff --git a/crates/oxc_semantic/src/builder.rs b/crates/oxc_semantic/src/builder.rs index 91f801857e60e..c0deeed6d6fe8 100644 --- a/crates/oxc_semantic/src/builder.rs +++ b/crates/oxc_semantic/src/builder.rs @@ -27,7 +27,7 @@ use crate::{ module_record::ModuleRecordBuilder, node::{AstNodeId, AstNodes, NodeFlags}, reference::{Reference, ReferenceFlag, ReferenceId}, - scope::{ScopeFlags, ScopeId, ScopeTree}, + scope::{ScopeFlags, ScopeId, ScopeTree, UnresolvedReferences}, symbol::{SymbolFlags, SymbolId, SymbolTable}, JSDocFinder, Semantic, }; @@ -199,12 +199,6 @@ impl<'a> SemanticBuilder<'a> { if self.check_syntax_error { checker::check_module_record(&self); } - - // 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() }; @@ -371,8 +365,7 @@ impl<'a> SemanticBuilder<'a> { let bindings = self.scope.get_bindings(self.current_scope_id); for line in current_refs.drain() { - // Try to resolve a reference. - // If unresolved, transfer it to parent scope's unresolved references. + // Try to resolve reference. If unresolved, transfer it to parent scope's unresolved references. 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); @@ -384,6 +377,25 @@ impl<'a> SemanticBuilder<'a> { } } + fn resolve_references_for_root_scope(&mut self) { + let mut root_unresolved_references = UnresolvedReferences::default(); + + let bindings = self.scope.get_bindings(self.current_scope_id); + for line in self.unresolved_references[self.current_scope_depth].drain() { + // Try to resolve reference. If unresolved, transfer it to root unresolved references. + 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(line.reference_ids); + } else { + root_unresolved_references.insert(line.name, line.reference_ids); + } + } + + self.scope.root_unresolved_references = root_unresolved_references; + } + pub fn add_redeclare_variable(&mut self, symbol_id: SymbolId, span: Span) { self.symbols.add_redeclare_variable(symbol_id, span); } @@ -445,11 +457,13 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> { } fn leave_scope(&mut self) { - self.resolve_references_for_current_scope(); if let Some(parent_id) = self.scope.get_parent_id(self.current_scope_id) { + self.resolve_references_for_current_scope(); self.current_scope_id = parent_id; + self.current_scope_depth -= 1; + } else { + self.resolve_references_for_root_scope(); } - self.current_scope_depth -= 1; } // Setup all the context for the binder. From eb1fa4021692d9490c2e3999248f944660174b08 Mon Sep 17 00:00:00 2001 From: overlookmotel Date: Sat, 13 Jul 2024 22:05:30 +0100 Subject: [PATCH 07/15] Rename vars --- crates/oxc_semantic/src/hash.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/crates/oxc_semantic/src/hash.rs b/crates/oxc_semantic/src/hash.rs index 1e41efd19a549..998c2c72b0ac7 100644 --- a/crates/oxc_semantic/src/hash.rs +++ b/crates/oxc_semantic/src/hash.rs @@ -33,14 +33,14 @@ impl TempUnresolvedReferences { #[allow(dead_code)] pub fn get(&self, name: &str, hash: IdentifierHash) -> Option<&Vec> { self.inner - .find(hash.0, |entry| entry.hash == hash && entry.name.as_ref() == name) + .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> { self.inner - .find_mut(hash.0, |entry| entry.hash == hash && entry.name.as_ref() == name) + .find_mut(hash.0, |line| line.hash == hash && line.name.as_ref() == name) .map(|entry| &mut entry.reference_ids) } @@ -48,7 +48,7 @@ impl TempUnresolvedReferences { let hash = IdentifierHash::new(&name); let entry = self.inner.entry( hash.0, - |entry| entry.hash == hash && entry.name == name, + |line| line.hash == hash && line.name == name, |entry| entry.hash.0, ); match entry { @@ -69,8 +69,8 @@ impl TempUnresolvedReferences { ) { let entry = self.inner.entry( hash.0, - |entry| entry.hash == hash && entry.name == name, - |entry| entry.hash.0, + |line| line.hash == hash && line.name == name, + |line| line.hash.0, ); match entry { Entry::Occupied(mut entry) => { From 59f2bd3bc167e23c8653935970c400537e3cff74 Mon Sep 17 00:00:00 2001 From: overlookmotel Date: Sat, 13 Jul 2024 22:05:38 +0100 Subject: [PATCH 08/15] Revert "`resolve_references_for_root_scope`" This reverts commit 463b7d1d3abd4ca290e17c6c0761cb7d4e52a960. --- crates/oxc_semantic/src/builder.rs | 36 +++++++++--------------------- 1 file changed, 11 insertions(+), 25 deletions(-) diff --git a/crates/oxc_semantic/src/builder.rs b/crates/oxc_semantic/src/builder.rs index c0deeed6d6fe8..91f801857e60e 100644 --- a/crates/oxc_semantic/src/builder.rs +++ b/crates/oxc_semantic/src/builder.rs @@ -27,7 +27,7 @@ use crate::{ 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, }; @@ -199,6 +199,12 @@ impl<'a> SemanticBuilder<'a> { if self.check_syntax_error { checker::check_module_record(&self); } + + // 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() }; @@ -365,7 +371,8 @@ impl<'a> SemanticBuilder<'a> { let bindings = self.scope.get_bindings(self.current_scope_id); for line in current_refs.drain() { - // Try to resolve reference. If unresolved, transfer it to parent scope's unresolved references. + // Try to resolve a reference. + // If unresolved, transfer it to parent scope's unresolved references. 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); @@ -377,25 +384,6 @@ impl<'a> SemanticBuilder<'a> { } } - fn resolve_references_for_root_scope(&mut self) { - let mut root_unresolved_references = UnresolvedReferences::default(); - - let bindings = self.scope.get_bindings(self.current_scope_id); - for line in self.unresolved_references[self.current_scope_depth].drain() { - // Try to resolve reference. If unresolved, transfer it to root unresolved references. - 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(line.reference_ids); - } else { - root_unresolved_references.insert(line.name, line.reference_ids); - } - } - - self.scope.root_unresolved_references = root_unresolved_references; - } - pub fn add_redeclare_variable(&mut self, symbol_id: SymbolId, span: Span) { self.symbols.add_redeclare_variable(symbol_id, span); } @@ -457,13 +445,11 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> { } fn leave_scope(&mut self) { + self.resolve_references_for_current_scope(); if let Some(parent_id) = self.scope.get_parent_id(self.current_scope_id) { - self.resolve_references_for_current_scope(); self.current_scope_id = parent_id; - self.current_scope_depth -= 1; - } else { - self.resolve_references_for_root_scope(); } + self.current_scope_depth -= 1; } // Setup all the context for the binder. From 62056b5e28890a5f7ecc3de8c33caf411c9f44ab Mon Sep 17 00:00:00 2001 From: overlookmotel Date: Sat, 13 Jul 2024 22:08:02 +0100 Subject: [PATCH 09/15] Don't compare hash --- crates/oxc_semantic/src/hash.rs | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/crates/oxc_semantic/src/hash.rs b/crates/oxc_semantic/src/hash.rs index 998c2c72b0ac7..57692417b4685 100644 --- a/crates/oxc_semantic/src/hash.rs +++ b/crates/oxc_semantic/src/hash.rs @@ -32,25 +32,19 @@ pub struct TempUnresolvedReferences { impl TempUnresolvedReferences { #[allow(dead_code)] pub fn get(&self, name: &str, hash: IdentifierHash) -> Option<&Vec> { - self.inner - .find(hash.0, |line| line.hash == hash && line.name.as_ref() == name) - .map(|entry| &entry.reference_ids) + self.inner.find(hash.0, |line| 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> { self.inner - .find_mut(hash.0, |line| line.hash == hash && line.name.as_ref() == name) + .find_mut(hash.0, |line| 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); - let entry = self.inner.entry( - hash.0, - |line| line.hash == hash && line.name == name, - |entry| entry.hash.0, - ); + let entry = self.inner.entry(hash.0, |line| line.name == name, |entry| entry.hash.0); match entry { Entry::Occupied(mut entry) => { entry.get_mut().reference_ids.push(reference_id); @@ -67,11 +61,7 @@ impl TempUnresolvedReferences { hash: IdentifierHash, reference_ids: Vec, ) { - let entry = self.inner.entry( - hash.0, - |line| line.hash == hash && line.name == name, - |line| line.hash.0, - ); + let entry = self.inner.entry(hash.0, |line| line.name == name, |line| line.hash.0); match entry { Entry::Occupied(mut entry) => { entry.get_mut().reference_ids.extend(reference_ids); From 406d97e2552e6b20b2dcc0272e5f361ebc25e6fd Mon Sep 17 00:00:00 2001 From: overlookmotel Date: Sat, 13 Jul 2024 22:22:14 +0100 Subject: [PATCH 10/15] Revert "Don't compare hash" This reverts commit 62056b5e28890a5f7ecc3de8c33caf411c9f44ab. --- crates/oxc_semantic/src/hash.rs | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/crates/oxc_semantic/src/hash.rs b/crates/oxc_semantic/src/hash.rs index 57692417b4685..998c2c72b0ac7 100644 --- a/crates/oxc_semantic/src/hash.rs +++ b/crates/oxc_semantic/src/hash.rs @@ -32,19 +32,25 @@ pub struct TempUnresolvedReferences { impl TempUnresolvedReferences { #[allow(dead_code)] pub fn get(&self, name: &str, hash: IdentifierHash) -> Option<&Vec> { - self.inner.find(hash.0, |line| line.name.as_ref() == name).map(|entry| &entry.reference_ids) + 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> { self.inner - .find_mut(hash.0, |line| line.name.as_ref() == name) + .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); - let entry = self.inner.entry(hash.0, |line| line.name == name, |entry| entry.hash.0); + let entry = self.inner.entry( + hash.0, + |line| line.hash == hash && line.name == name, + |entry| entry.hash.0, + ); match entry { Entry::Occupied(mut entry) => { entry.get_mut().reference_ids.push(reference_id); @@ -61,7 +67,11 @@ impl TempUnresolvedReferences { hash: IdentifierHash, reference_ids: Vec, ) { - let entry = self.inner.entry(hash.0, |line| line.name == name, |line| line.hash.0); + let entry = self.inner.entry( + hash.0, + |line| line.hash == hash && line.name == name, + |line| line.hash.0, + ); match entry { Entry::Occupied(mut entry) => { entry.get_mut().reference_ids.extend(reference_ids); From 0c6a3dc91a0e4e079981027d14a341f9b9bd6880 Mon Sep 17 00:00:00 2001 From: overlookmotel Date: Sat, 13 Jul 2024 22:23:45 +0100 Subject: [PATCH 11/15] Rename vars --- crates/oxc_semantic/src/hash.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/oxc_semantic/src/hash.rs b/crates/oxc_semantic/src/hash.rs index 998c2c72b0ac7..1d7c21f63d1a7 100644 --- a/crates/oxc_semantic/src/hash.rs +++ b/crates/oxc_semantic/src/hash.rs @@ -49,7 +49,7 @@ impl TempUnresolvedReferences { let entry = self.inner.entry( hash.0, |line| line.hash == hash && line.name == name, - |entry| entry.hash.0, + |line: &Line| line.hash.0, ); match entry { Entry::Occupied(mut entry) => { From 44c6662468c58876f0d970ee67e62176a57fdce6 Mon Sep 17 00:00:00 2001 From: overlookmotel Date: Sat, 13 Jul 2024 22:28:18 +0100 Subject: [PATCH 12/15] Don't use `Entry` API --- crates/oxc_semantic/src/hash.rs | 39 ++++++++++++--------------------- 1 file changed, 14 insertions(+), 25 deletions(-) diff --git a/crates/oxc_semantic/src/hash.rs b/crates/oxc_semantic/src/hash.rs index 1d7c21f63d1a7..d7ab8a1943a01 100644 --- a/crates/oxc_semantic/src/hash.rs +++ b/crates/oxc_semantic/src/hash.rs @@ -1,6 +1,6 @@ use std::hash::Hasher; -use hashbrown::hash_table::{Drain, Entry, HashTable}; +use hashbrown::hash_table::{Drain, HashTable}; use rustc_hash::FxHasher; use oxc_span::CompactStr; @@ -46,18 +46,14 @@ impl TempUnresolvedReferences { pub fn insert(&mut self, name: CompactStr, reference_id: ReferenceId) { let hash = IdentifierHash::new(&name); - let entry = self.inner.entry( - hash.0, - |line| line.hash == hash && line.name == name, - |line: &Line| line.hash.0, - ); - match entry { - Entry::Occupied(mut entry) => { - entry.get_mut().reference_ids.push(reference_id); - } - Entry::Vacant(entry) => { - entry.insert(Line { name, hash, reference_ids: vec![reference_id] }); - } + 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, + ); } } @@ -67,18 +63,11 @@ impl TempUnresolvedReferences { hash: IdentifierHash, reference_ids: Vec, ) { - let entry = self.inner.entry( - hash.0, - |line| line.hash == hash && line.name == name, - |line| line.hash.0, - ); - match entry { - Entry::Occupied(mut entry) => { - entry.get_mut().reference_ids.extend(reference_ids); - } - Entry::Vacant(entry) => { - entry.insert(Line { name, hash, reference_ids }); - } + 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); } } From 43e0171118f455e9dd1cd4efef76158678b85f59 Mon Sep 17 00:00:00 2001 From: overlookmotel Date: Sat, 13 Jul 2024 23:06:15 +0100 Subject: [PATCH 13/15] More explicit code --- crates/oxc_semantic/src/builder.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/oxc_semantic/src/builder.rs b/crates/oxc_semantic/src/builder.rs index 91f801857e60e..12c701738b14d 100644 --- a/crates/oxc_semantic/src/builder.rs +++ b/crates/oxc_semantic/src/builder.rs @@ -111,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 { From c18e5d28294eacc52d0af6e58fbe224905c63f5c Mon Sep 17 00:00:00 2001 From: overlookmotel Date: Sun, 14 Jul 2024 03:33:12 +0100 Subject: [PATCH 14/15] Optimize empty hash maps --- crates/oxc_semantic/src/builder.rs | 24 ++++++++++++++++-------- crates/oxc_semantic/src/hash.rs | 5 +++++ 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/crates/oxc_semantic/src/builder.rs b/crates/oxc_semantic/src/builder.rs index 12c701738b14d..23e021694252b 100644 --- a/crates/oxc_semantic/src/builder.rs +++ b/crates/oxc_semantic/src/builder.rs @@ -370,15 +370,23 @@ impl<'a> SemanticBuilder<'a> { let current_refs = iter.next().unwrap(); let bindings = self.scope.get_bindings(self.current_scope_id); - 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(&line.name).copied() { - for reference_id in &line.reference_ids { - self.symbols.references[*reference_id].set_symbol_id(symbol_id); + if !bindings.is_empty() { + 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(&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(line.reference_ids); + } else { + parent_refs.extend(line.name, line.hash, line.reference_ids); } - self.symbols.resolved_references[symbol_id].extend(line.reference_ids); - } else { + } + } else if parent_refs.is_empty() { + std::mem::swap(current_refs, parent_refs); + } else { + for line in current_refs.drain() { parent_refs.extend(line.name, line.hash, line.reference_ids); } } diff --git a/crates/oxc_semantic/src/hash.rs b/crates/oxc_semantic/src/hash.rs index d7ab8a1943a01..9d1c05d3d0e9e 100644 --- a/crates/oxc_semantic/src/hash.rs +++ b/crates/oxc_semantic/src/hash.rs @@ -44,6 +44,11 @@ impl TempUnresolvedReferences { .map(|entry| &mut entry.reference_ids) } + #[inline] + pub fn is_empty(&self) -> bool { + self.inner.is_empty() + } + 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) { From 480090ca6958b26afa41ed6e9a6ee9056b4e9331 Mon Sep 17 00:00:00 2001 From: overlookmotel Date: Sun, 14 Jul 2024 03:45:35 +0100 Subject: [PATCH 15/15] Revert "Optimize empty hash maps" This reverts commit c18e5d28294eacc52d0af6e58fbe224905c63f5c. --- crates/oxc_semantic/src/builder.rs | 24 ++++++++---------------- crates/oxc_semantic/src/hash.rs | 5 ----- 2 files changed, 8 insertions(+), 21 deletions(-) diff --git a/crates/oxc_semantic/src/builder.rs b/crates/oxc_semantic/src/builder.rs index 23e021694252b..12c701738b14d 100644 --- a/crates/oxc_semantic/src/builder.rs +++ b/crates/oxc_semantic/src/builder.rs @@ -370,23 +370,15 @@ impl<'a> SemanticBuilder<'a> { let current_refs = iter.next().unwrap(); let bindings = self.scope.get_bindings(self.current_scope_id); - if !bindings.is_empty() { - 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(&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(line.reference_ids); - } else { - parent_refs.extend(line.name, line.hash, line.reference_ids); + 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(&line.name).copied() { + for reference_id in &line.reference_ids { + self.symbols.references[*reference_id].set_symbol_id(symbol_id); } - } - } else if parent_refs.is_empty() { - std::mem::swap(current_refs, parent_refs); - } else { - for line in current_refs.drain() { + self.symbols.resolved_references[symbol_id].extend(line.reference_ids); + } else { parent_refs.extend(line.name, line.hash, line.reference_ids); } } diff --git a/crates/oxc_semantic/src/hash.rs b/crates/oxc_semantic/src/hash.rs index 9d1c05d3d0e9e..d7ab8a1943a01 100644 --- a/crates/oxc_semantic/src/hash.rs +++ b/crates/oxc_semantic/src/hash.rs @@ -44,11 +44,6 @@ impl TempUnresolvedReferences { .map(|entry| &mut entry.reference_ids) } - #[inline] - pub fn is_empty(&self) -> bool { - self.inner.is_empty() - } - 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) {