Skip to content

Commit

Permalink
refactor(semantic): move Counts code into counter module
Browse files Browse the repository at this point in the history
  • Loading branch information
overlookmotel committed Sep 11, 2024
1 parent 8f438db commit 1b4412f
Show file tree
Hide file tree
Showing 5 changed files with 46 additions and 13 deletions.
7 changes: 7 additions & 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 @@ -147,6 +147,7 @@ memoffset = "0.9.1"
miette = { version = "7.2.0", features = ["fancy-no-syscall"] }
mimalloc = "0.1.43"
mime_guess = "2.0.5"
more-asserts = "0.3.1"
nonmax = "0.5.5"
num-bigint = "0.4.6"
num-traits = "0.2.19"
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 @@ -30,6 +30,7 @@ oxc_syntax = { workspace = true }
assert-unchecked = { workspace = true }
indexmap = { workspace = true }
itertools = { workspace = true }
more-asserts = { workspace = true }
phf = { workspace = true, features = ["macros"] }
rustc-hash = { workspace = true }
serde = { workspace = true, features = ["derive"], optional = true }
Expand Down
23 changes: 11 additions & 12 deletions crates/oxc_semantic/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -231,8 +231,7 @@ impl<'a> SemanticBuilder<'a> {
// Avoiding this growth produces up to 30% perf boost on our benchmarks.
// TODO: It would be even more efficient to calculate counts in parser to avoid
// this extra AST traversal.
let mut counts = Counts::default();
counts.visit_program(program);
let counts = Counts::count(program);
self.nodes.reserve(counts.nodes);
self.scope.reserve(counts.scopes);
self.symbols.reserve(counts.symbols, counts.references);
Expand All @@ -241,16 +240,16 @@ impl<'a> SemanticBuilder<'a> {
self.visit_program(program);

// Check that estimated counts accurately
debug_assert_eq!(self.nodes.len(), counts.nodes);
debug_assert_eq!(self.scope.len(), counts.scopes);
debug_assert_eq!(self.symbols.references.len(), counts.references);
// `Counts` may overestimate number of symbols, because multiple `BindingIdentifier`s
// can result in only a single symbol.
// e.g. `var x; var x;` = 2 x `BindingIdentifier` but 1 x symbol.
// This is not a big problem - allocating a `Vec` with excess capacity is cheap.
// It's allocating with *not enough* capacity which is costly, as then the `Vec`
// will grow and reallocate.
debug_assert!(self.symbols.len() <= counts.symbols);
#[cfg(debug_assertions)]
{
let actual_counts = Counts {
nodes: self.nodes.len(),
scopes: self.scope.len(),
symbols: self.symbols.len(),
references: self.symbols.references.len(),
};
Counts::assert_accurate(&actual_counts, &counts);
}

// Checking syntax error on module record requires scope information from the previous AST pass
if self.check_syntax_error {
Expand Down
27 changes: 26 additions & 1 deletion crates/oxc_semantic/src/counter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,12 @@

use std::cell::Cell;

use more_asserts::assert_le;

use oxc_ast::{
ast::{BindingIdentifier, IdentifierReference, TSEnumMemberName, TSModuleDeclarationName},
ast::{
BindingIdentifier, IdentifierReference, Program, TSEnumMemberName, TSModuleDeclarationName,
},
visit::walk::{walk_ts_enum_member_name, walk_ts_module_declaration_name},
AstKind, Visit,
};
Expand All @@ -19,6 +23,27 @@ pub(crate) struct Counts {
pub references: usize,
}

impl Counts {
pub fn count(program: &Program) -> Self {
let mut counts = Counts::default();
counts.visit_program(program);
counts
}

pub fn assert_accurate(actual: &Self, estimated: &Self) {
assert_eq!(actual.nodes, estimated.nodes, "nodes count mismatch");
assert_eq!(actual.scopes, estimated.scopes, "scopes count mismatch");
assert_eq!(actual.references, estimated.references, "references count mismatch");
// `Counts` may overestimate number of symbols, because multiple `BindingIdentifier`s
// can result in only a single symbol.
// e.g. `var x; var x;` = 2 x `BindingIdentifier` but 1 x symbol.
// This is not a big problem - allocating a `Vec` with excess capacity is cheap.
// It's allocating with *not enough* capacity which is costly, as then the `Vec`
// will grow and reallocate.
assert_le!(actual.symbols, estimated.symbols, "symbols count mismatch");
}
}

impl<'a> Visit<'a> for Counts {
#[inline]
fn enter_node(&mut self, _: AstKind<'a>) {
Expand Down

0 comments on commit 1b4412f

Please sign in to comment.