perf(semantic): share one clone_in across symbol creation and binding insertion#22666
Closed
Dunqing wants to merge 1 commit into
Closed
perf(semantic): share one clone_in across symbol creation and binding insertion#22666Dunqing wants to merge 1 commit into
Dunqing wants to merge 1 commit into
Conversation
… insertion `declare_symbol_on_scope` and `declare_shadow_symbol` both called `create_symbol(name, ...)` followed by `add_binding` / `insert_binding` on the same name. Each function did its own `name.clone_in(allocator)`, copying the same identifier bytes into the scoping arena twice on every new binding. Add a private helper `Scoping::create_symbol_with_binding` that performs both inserts inside a single `with_dependent_mut` call, sharing one cloned `Ident<'cell>` between the symbol-names vector and the bindings map. The returned `SymbolId` is computed from `symbol_table.len()` before `push`, matching the id `push` would return. For shadow symbols the bind-scope can differ from the symbol's owner scope (catch parameters), so the helper takes both separately. Benchmark (`cargo bench --bench semantic`): - RadixUIAdoptionSection.jsx: -1.68% - react.development.js: -1.98% - cal.com.tsx: -3.10% - binder.ts: -2.73% - kitchen-sink.tsx: -1.07% Removes the now-unused `Scoping::insert_binding`.
Merging this PR will not alter performance
Comparing 
Footnotes
|
Member
Author
|
Duplicate of #22663 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
declare_symbol_on_scopeanddeclare_shadow_symbolboth calledcreate_symbol(name, …)followed byadd_binding/insert_bindingon the samename. Each function did its ownname.clone_in(allocator), copying the same identifier bytes into the scoping arena twice on every new binding.Add a private helper
Scoping::create_symbol_with_bindingthat performs both inserts inside a singlewith_dependent_mutcall, sharing one clonedIdent<'cell>between thesymbol_namesvector and thebindingsmap. The returnedSymbolIdis computed fromsymbol_table.len()beforepush, matching the idsymbol_table.pushwould return —symbol_namesis kept in lock-step withsymbol_table.For shadow symbols (e.g.
catchparameters) the bind-scope can differ from the symbol's owner scope, so the helper takes both separately.Scoping::insert_bindingis removed — its only caller (declare_shadow_symbol) now goes through the combined helper.Measurements
cargo bench --bench semantic --no-default-features --features compileronTestFiles::minimal(), criterion baseline vs. this PR, M3-class machine:All five deltas are statistically significant per criterion (
p < 0.05).Verified by
cargo test -p oxc_semantic --features cfg(72 tests),cargo clippy -p oxc_semantic --all-features, downstreamcargo test -p oxc_transformer/-p oxc_minifier, andcargo coverage -- semantic(semantic snapshot counts unchanged: 45575/47095 test262, 2035/2236 babel, 2671/4773 typescript, 52/66 misc).AI assisted.