perf(minifier): incremental scoping refresh, delete LiveUsageCollector#23197
Conversation
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
Monitor OxcCommit:
|
Merging this PR will improve performance by 81.25%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ⚡ | Simulation | linter[RadixUIAdoptionSection.jsx] |
10,290.4 µs | 884.8 µs | ×12 |
| ⚡ | Simulation | linter[App.tsx] |
211 ms | 173.8 ms | +21.39% |
| ⚡ | Simulation | minifier[kitchen-sink.tsx] |
63.8 ms | 52.6 ms | +21.32% |
| ⚡ | Simulation | pipeline[kitchen-sink.tsx] |
122.2 ms | 110.8 ms | +10.32% |
| ⚡ | Simulation | minifier[App.tsx] |
12.6 ms | 12.1 ms | +3.51% |
Tip
Curious why this is faster? Comment @codspeedbot explain why this is faster on this PR, or directly use the CodSpeed MCP with your agent.
Comparing minifier-stack/incremental-scoping (db7b22e) with minifier-stack/mutation-helpers (8e9ace2)
Footnotes
-
9 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
afeb19d to
c6c47fa
Compare
06958d3 to
a60d97b
Compare
c6c47fa to
a3194a0
Compare
a60d97b to
a4fe763
Compare
427a542 to
0944303
Compare
Boshen
left a comment
There was a problem hiding this comment.
Please run this through ecosystem ci and monitor-oxc fully.
Please merge after next week's release so we have more time to evaluate.
602742f to
26ed2e0
Compare
0c8070c to
d357321
Compare
|
Verified against rolldown (patched rolldown's oxc deps at this branch locally):
So no behavioral change in rolldown from this PR. 👍 |
Both verified in monitor-oxc and Rolldown, there are no tests broken.
OK, I am still polishing it |
d357321 to
db7b22e
Compare
a4fe763 to
8e9ace2
Compare
db7b22e to
14f0081
Compare
8e9ace2 to
09176f8
Compare
Merge activity
|
14f0081 to
0cc8c0b
Compare
#23197) > Developed with AI assistance (Claude Code); reviewed, tested, and benchmarked by the contributor. ## Summary Deletes the per-pass `LiveUsageCollector` walk over the entire live program. The typed mutation helpers (previous PR in this stack) now walk only the dropped/replaced subtrees in place, accumulating dead `ReferenceId`s into a per-pass `PassDirty` arena `BitSet` that the `Compressor` driver consumes in one batch (after `Normalize` and after each peephole pass) via the new `Scoping::retain_resolved_references_excluding`. Direct-eval flag refresh runs only when an `eval(...)` call was actually dropped. Cross-crate support riding in this PR: `Scoping::retain_resolved_references_excluding` (oxc_semantic) and `BitSet::capacity()` / `is_empty()` / `contains()` (oxc_allocator). ## Breaking Removes `oxc_semantic::Scoping::retain_resolved_references` — its only consumer was `LiveUsageCollector`, deleted here. No other in-repo or known downstream callers. ## Benchmarks (CodSpeed, this PR vs stack base) | Benchmark | Base → Head | Efficiency | |---|---|---| | `minifier[kitchen-sink.tsx]` | 63.6 ms → 52.5 ms | **+21.2%** | | `pipeline[kitchen-sink.tsx]` | 122.3 ms → 110.7 ms | +10.5% | | `minifier[App.tsx]` | 12.5 ms → 12.1 ms | +3.3% | | everything else | — | unchanged | > **Why earlier runs showed ~2× on kitchen-sink:** the initial cut of this branch had a reference-leak bug — dropped subtrees weren't always marked dead, leaked references made symbols look used, and the minifier silently skipped most of kitchen-sink's DCE, producing output **22% larger than main** (invisible to minsize, which doesn't include that file). Those runs measured a minifier doing less work, not a faster minifier. The leak is fixed (see below); the numbers above are measured at byte-identical output to main. Memory: kitchen-sink.tsx arena allocs 100230 → 90553, sys allocs 2917 → 2639; all other tracked files at or slightly below base. ## Relation to #22736 This re-cuts the approach prototyped and discussed in #22736 (kept open during development as the design record), with three deviations from the POC: 1. **Mark-only `DropDiff`** — the POC's "Resurrect" mode existed for one call site (`substitute_is_object_and_not_null`) that reused `ReferenceId`s in replacement values. That site now mints fresh references (same symbol/resolution, new id), so the resurrect walk and its cross-call aliasing invariant are gone entirely. 2. **`mutated: bool` instead of a `mutations: u64` snapshot counter** — after this PR the signal's only reader is the fixed-point loop, so a private bool with read-and-reset `take_mutated()` suffices. 3. **Bounds-checked mark path** — references minted mid-pass and dropped in the same pass are skipped (treated live → conservative output) instead of the POC's debug-assert + potential release panic. Safety net: `debug_assert_no_over_prune` walks the live program in debug builds after each dirty pass, so the whole test + conformance corpus doubles as an over-prune detector at zero release cost. ## Reference-leak fix + simplification follow-ups The initial cut leaked references at three classes of drop sites. The leak direction is safe (stale extra references block optimizations, never corrupt output), but it cost real output size on `void ident`-heavy inputs — kitchen-sink.tsx minified 22% larger than main: 1. **`Normalize` raw drops** (`void x` → `void 0`, `drop_console`) never reached `PassDirty`. `Normalize` now records drops through the same typed helpers, and the driver flushes once before the fixed-point loop so pass 1 already observes pruned reference counts (no extra peephole pass). 2. **`remove_unused_template_literal`** silently drained elements that `remove_unused_expression` collapsed to nothing, without a drop walk. 3. **Dropped variable declarators** walked only `init`; references in the binding's TS type annotation (e.g. computed keys in a type literal) leaked. New `drop_variable_declarator` helper walks the whole declarator. Each class has a leak-regression test; kitchen-sink.tsx output is verified **byte-identical to main**. Two simplification passes followed: flush hoisted into the `Compressor` driver (no cross-traversal coupling), `DropDiff` reduced to a thin `Visit` over `&mut PassDirty` (no scoping lookup on the mark hot path; marks for unresolved refs are inert), `BitSet::clear()` reuse instead of a per-pass arena realloc, a shared `as_direct_eval_call` predicate for producer and consumer, plain `clone_in` (id-less) at the one site that cloned semantic ids, and removal of dead `drop_console` repair code. ## Verification - `cargo test -p oxc_minifier -p oxc_mangler -p oxc_semantic -p oxc_allocator` — all pass, incl. the new leak-regression tests - `just minsize` — **bit-identical output**; kitchen-sink.tsx (not in minsize) separately verified byte-identical to main - `cargo coverage -- minifier` — test262 44825/44829, babel 1785/1785, snapshots unchanged - clippy (all features/targets), fmt clean ## Test alignment in oxc_transformer_plugins The `replace_global_defines` integration-test helper previously reused RGD's returned scoping directly for DCE — a path production does not take: `crates/oxc/src/compiler.rs` marks scoping dirty whenever RGD changed the AST (`scoping_dirty |= ret.changed`) and rebuilds before compress/DCE, and rolldown minifies freshly-parsed chunks. With the full-walk `LiveUsageCollector` gone, that test-only path surfaced stale ambient-`declare` references. The helper now mirrors the production pipeline (rebuild when `ret.changed`, reuse otherwise). No `ReplaceGlobalDefines` source changes. Two `declare`-define contract tests added. ## Known trade-off: defines-heavy DCE pays O(dropped) drop-walks Profiling `pipeline[react.development.js]` (with `process.env.NODE_ENV` defines) shows the worst case of walk-what-you-drop: DCE drops one `if` subtree covering ~90% of the program's nodes, so the drop-walk approaches a full-program traversal where the old per-pass live walk only covered the small remainder (−3% CodSpeed / +7% local wall on that bench; attribution: ~72% drop-walks, 0% double-marking, ~0% bitset cost). A bounded-walk + live-walk-fallback mitigation was prototyped and validated (recovers to +1.4%, all other wins preserved), but we decided the added mechanism wasn't worth the maintenance cost for a one-bench regression — the same file's plain minification *improves* 12%, and kitchen-sink-class inputs (the load-bearing case) improve ~2×. Revisit if real-world defines-heavy workloads surface it.
0cc8c0b to
bcb3894
Compare
### 💥 BREAKING CHANGES - 7a76cd3 estree: [**BREAKING**] Make whether to include TS fields a runtime option (#23574) (overlookmotel) - e7b6b68 estree: [**BREAKING**] `ESTree` config use methods not consts (#23573) (overlookmotel) ### 🚀 Features - 556cc6d data_structures: Add `CodeBuffer::as_str` method (#23571) (overlookmotel) - 38c4b06 parser: Add friendly error for adjacent JSX elements (#23378) (sapphi-red) - 53509a8 minifier: Treeshake pure typed arrays and Set/Map array literals (#23469) (Dunqing) - 09762d9 minifier: Inline const value for read-only vars (#22593) (Dunqing) ### 🐛 Bug Fixes - 20375f9 react_compiler: Keep imports referenced only by a computed key (#23586) (Boshen) - 31bfd9b minifier: Keep Object introspection calls on a possible Proxy (#23483) (Dunqing) - 837a395 parser: Treat a line comment after ':' as leading, not trailing (#23515) (Dunqing) - e409fe0 minifier: Keep `new Map`/`WeakSet`/`WeakMap` with a string argument (#23470) (Dunqing) - ae02b4e ci/parser: Use `minimal` for vitest reporter (#23457) (camc314) ### ⚡ Performance - cf24329 mangler: Compile slot sort once instead of per CAPACITY (#23577) (Boshen) - 4058a6a parser: Reduce code bloat from verify_modifiers monomorphization (#23576) (Boshen) - 053b0c1 estree: Remove pointless `mem::take` (#23572) (overlookmotel) - dfb52b6 transformer: Pre-size statement vecs in TS enum & namespace lowering (#23516) (Yunfei He) - 970e09a minifier: Compute template-literal inline checks in a single pass (#23467) (Yunfei He) - 3170c0e semantic,mangler,minifier: Fix `Semantic::stats` node count and reuse stats in mangler builds (#23352) (Boshen) - d1fa6e0 minifier: Evaluate ternary branches once in minimize_conditional_expression (#23479) (Yunfei He) - 3fa8051 transformer: Pre-size JSX props vec to attribute count (#23466) (Yunfei He) - 488b382 react_compiler: Borrow binding names in prefilter instead of allocating (#23471) (Yunfei He) - bcb3894 minifier: Incremental scoping refresh, delete LiveUsageCollector (#23197) (Dunqing) ### 📚 Documentation - f68641e data_structures: Improve docs on safety contract (#23575) (overlookmotel) Co-authored-by: Boshen <1430279+Boshen@users.noreply.github.com>
#23197) > Developed with AI assistance (Claude Code); reviewed, tested, and benchmarked by the contributor. ## Summary Deletes the per-pass `LiveUsageCollector` walk over the entire live program. The typed mutation helpers (previous PR in this stack) now walk only the dropped/replaced subtrees in place, accumulating dead `ReferenceId`s into a per-pass `PassDirty` arena `BitSet` that the `Compressor` driver consumes in one batch (after `Normalize` and after each peephole pass) via the new `Scoping::retain_resolved_references_excluding`. Direct-eval flag refresh runs only when an `eval(...)` call was actually dropped. Cross-crate support riding in this PR: `Scoping::retain_resolved_references_excluding` (oxc_semantic) and `BitSet::capacity()` / `is_empty()` / `contains()` (oxc_allocator). ## Breaking Removes `oxc_semantic::Scoping::retain_resolved_references` — its only consumer was `LiveUsageCollector`, deleted here. No other in-repo or known downstream callers. ## Benchmarks (CodSpeed, this PR vs stack base) | Benchmark | Base → Head | Efficiency | |---|---|---| | `minifier[kitchen-sink.tsx]` | 63.6 ms → 52.5 ms | **+21.2%** | | `pipeline[kitchen-sink.tsx]` | 122.3 ms → 110.7 ms | +10.5% | | `minifier[App.tsx]` | 12.5 ms → 12.1 ms | +3.3% | | everything else | — | unchanged | > **Why earlier runs showed ~2× on kitchen-sink:** the initial cut of this branch had a reference-leak bug — dropped subtrees weren't always marked dead, leaked references made symbols look used, and the minifier silently skipped most of kitchen-sink's DCE, producing output **22% larger than main** (invisible to minsize, which doesn't include that file). Those runs measured a minifier doing less work, not a faster minifier. The leak is fixed (see below); the numbers above are measured at byte-identical output to main. Memory: kitchen-sink.tsx arena allocs 100230 → 90553, sys allocs 2917 → 2639; all other tracked files at or slightly below base. ## Relation to #22736 This re-cuts the approach prototyped and discussed in #22736 (kept open during development as the design record), with three deviations from the POC: 1. **Mark-only `DropDiff`** — the POC's "Resurrect" mode existed for one call site (`substitute_is_object_and_not_null`) that reused `ReferenceId`s in replacement values. That site now mints fresh references (same symbol/resolution, new id), so the resurrect walk and its cross-call aliasing invariant are gone entirely. 2. **`mutated: bool` instead of a `mutations: u64` snapshot counter** — after this PR the signal's only reader is the fixed-point loop, so a private bool with read-and-reset `take_mutated()` suffices. 3. **Bounds-checked mark path** — references minted mid-pass and dropped in the same pass are skipped (treated live → conservative output) instead of the POC's debug-assert + potential release panic. Safety net: `debug_assert_no_over_prune` walks the live program in debug builds after each dirty pass, so the whole test + conformance corpus doubles as an over-prune detector at zero release cost. ## Reference-leak fix + simplification follow-ups The initial cut leaked references at three classes of drop sites. The leak direction is safe (stale extra references block optimizations, never corrupt output), but it cost real output size on `void ident`-heavy inputs — kitchen-sink.tsx minified 22% larger than main: 1. **`Normalize` raw drops** (`void x` → `void 0`, `drop_console`) never reached `PassDirty`. `Normalize` now records drops through the same typed helpers, and the driver flushes once before the fixed-point loop so pass 1 already observes pruned reference counts (no extra peephole pass). 2. **`remove_unused_template_literal`** silently drained elements that `remove_unused_expression` collapsed to nothing, without a drop walk. 3. **Dropped variable declarators** walked only `init`; references in the binding's TS type annotation (e.g. computed keys in a type literal) leaked. New `drop_variable_declarator` helper walks the whole declarator. Each class has a leak-regression test; kitchen-sink.tsx output is verified **byte-identical to main**. Two simplification passes followed: flush hoisted into the `Compressor` driver (no cross-traversal coupling), `DropDiff` reduced to a thin `Visit` over `&mut PassDirty` (no scoping lookup on the mark hot path; marks for unresolved refs are inert), `BitSet::clear()` reuse instead of a per-pass arena realloc, a shared `as_direct_eval_call` predicate for producer and consumer, plain `clone_in` (id-less) at the one site that cloned semantic ids, and removal of dead `drop_console` repair code. ## Verification - `cargo test -p oxc_minifier -p oxc_mangler -p oxc_semantic -p oxc_allocator` — all pass, incl. the new leak-regression tests - `just minsize` — **bit-identical output**; kitchen-sink.tsx (not in minsize) separately verified byte-identical to main - `cargo coverage -- minifier` — test262 44825/44829, babel 1785/1785, snapshots unchanged - clippy (all features/targets), fmt clean ## Test alignment in oxc_transformer_plugins The `replace_global_defines` integration-test helper previously reused RGD's returned scoping directly for DCE — a path production does not take: `crates/oxc/src/compiler.rs` marks scoping dirty whenever RGD changed the AST (`scoping_dirty |= ret.changed`) and rebuilds before compress/DCE, and rolldown minifies freshly-parsed chunks. With the full-walk `LiveUsageCollector` gone, that test-only path surfaced stale ambient-`declare` references. The helper now mirrors the production pipeline (rebuild when `ret.changed`, reuse otherwise). No `ReplaceGlobalDefines` source changes. Two `declare`-define contract tests added. ## Known trade-off: defines-heavy DCE pays O(dropped) drop-walks Profiling `pipeline[react.development.js]` (with `process.env.NODE_ENV` defines) shows the worst case of walk-what-you-drop: DCE drops one `if` subtree covering ~90% of the program's nodes, so the drop-walk approaches a full-program traversal where the old per-pass live walk only covered the small remainder (−3% CodSpeed / +7% local wall on that bench; attribution: ~72% drop-walks, 0% double-marking, ~0% bitset cost). A bounded-walk + live-walk-fallback mitigation was prototyped and validated (recovers to +1.4%, all other wins preserved), but we decided the added mechanism wasn't worth the maintenance cost for a one-bench regression — the same file's plain minification *improves* 12%, and kitchen-sink-class inputs (the load-bearing case) improve ~2×. Revisit if real-world defines-heavy workloads surface it.
### 💥 BREAKING CHANGES - 7a76cd3 estree: [**BREAKING**] Make whether to include TS fields a runtime option (#23574) (overlookmotel) - e7b6b68 estree: [**BREAKING**] `ESTree` config use methods not consts (#23573) (overlookmotel) ### 🚀 Features - 556cc6d data_structures: Add `CodeBuffer::as_str` method (#23571) (overlookmotel) - 38c4b06 parser: Add friendly error for adjacent JSX elements (#23378) (sapphi-red) - 53509a8 minifier: Treeshake pure typed arrays and Set/Map array literals (#23469) (Dunqing) - 09762d9 minifier: Inline const value for read-only vars (#22593) (Dunqing) ### 🐛 Bug Fixes - 20375f9 react_compiler: Keep imports referenced only by a computed key (#23586) (Boshen) - 31bfd9b minifier: Keep Object introspection calls on a possible Proxy (#23483) (Dunqing) - 837a395 parser: Treat a line comment after ':' as leading, not trailing (#23515) (Dunqing) - e409fe0 minifier: Keep `new Map`/`WeakSet`/`WeakMap` with a string argument (#23470) (Dunqing) - ae02b4e ci/parser: Use `minimal` for vitest reporter (#23457) (camc314) ### ⚡ Performance - cf24329 mangler: Compile slot sort once instead of per CAPACITY (#23577) (Boshen) - 4058a6a parser: Reduce code bloat from verify_modifiers monomorphization (#23576) (Boshen) - 053b0c1 estree: Remove pointless `mem::take` (#23572) (overlookmotel) - dfb52b6 transformer: Pre-size statement vecs in TS enum & namespace lowering (#23516) (Yunfei He) - 970e09a minifier: Compute template-literal inline checks in a single pass (#23467) (Yunfei He) - 3170c0e semantic,mangler,minifier: Fix `Semantic::stats` node count and reuse stats in mangler builds (#23352) (Boshen) - d1fa6e0 minifier: Evaluate ternary branches once in minimize_conditional_expression (#23479) (Yunfei He) - 3fa8051 transformer: Pre-size JSX props vec to attribute count (#23466) (Yunfei He) - 488b382 react_compiler: Borrow binding names in prefilter instead of allocating (#23471) (Yunfei He) - bcb3894 minifier: Incremental scoping refresh, delete LiveUsageCollector (#23197) (Dunqing) ### 📚 Documentation - f68641e data_structures: Improve docs on safety contract (#23575) (overlookmotel) Co-authored-by: Boshen <1430279+Boshen@users.noreply.github.com>

Summary
Deletes the per-pass
LiveUsageCollectorwalk over the entire live program. The typed mutation helpers (previous PR in this stack) now walk only the dropped/replaced subtrees in place, accumulating deadReferenceIds into a per-passPassDirtyarenaBitSetthat theCompressordriver consumes in one batch (afterNormalizeand after each peephole pass) via the newScoping::retain_resolved_references_excluding. Direct-eval flag refresh runs only when aneval(...)call was actually dropped.Cross-crate support riding in this PR:
Scoping::retain_resolved_references_excluding(oxc_semantic) andBitSet::capacity()/is_empty()/contains()(oxc_allocator).Breaking
Removes
oxc_semantic::Scoping::retain_resolved_references— its only consumer wasLiveUsageCollector, deleted here. No other in-repo or known downstream callers.Benchmarks (CodSpeed, this PR vs stack base)
minifier[kitchen-sink.tsx]pipeline[kitchen-sink.tsx]minifier[App.tsx]Memory: kitchen-sink.tsx arena allocs 100230 → 90553, sys allocs 2917 → 2639; all other tracked files at or slightly below base.
Relation to #22736
This re-cuts the approach prototyped and discussed in #22736 (kept open during development as the design record), with three deviations from the POC:
DropDiff— the POC's "Resurrect" mode existed for one call site (substitute_is_object_and_not_null) that reusedReferenceIds in replacement values. That site now mints fresh references (same symbol/resolution, new id), so the resurrect walk and its cross-call aliasing invariant are gone entirely.mutated: boolinstead of amutations: u64snapshot counter — after this PR the signal's only reader is the fixed-point loop, so a private bool with read-and-resettake_mutated()suffices.Safety net:
debug_assert_no_over_prunewalks the live program in debug builds after each dirty pass, so the whole test + conformance corpus doubles as an over-prune detector at zero release cost.Reference-leak fix + simplification follow-ups
The initial cut leaked references at three classes of drop sites. The leak direction is safe (stale extra references block optimizations, never corrupt output), but it cost real output size on
void ident-heavy inputs — kitchen-sink.tsx minified 22% larger than main:Normalizeraw drops (void x→void 0,drop_console) never reachedPassDirty.Normalizenow records drops through the same typed helpers, and the driver flushes once before the fixed-point loop so pass 1 already observes pruned reference counts (no extra peephole pass).remove_unused_template_literalsilently drained elements thatremove_unused_expressioncollapsed to nothing, without a drop walk.init; references in the binding's TS type annotation (e.g. computed keys in a type literal) leaked. Newdrop_variable_declaratorhelper walks the whole declarator.Each class has a leak-regression test; kitchen-sink.tsx output is verified byte-identical to main.
Two simplification passes followed: flush hoisted into the
Compressordriver (no cross-traversal coupling),DropDiffreduced to a thinVisitover&mut PassDirty(no scoping lookup on the mark hot path; marks for unresolved refs are inert),BitSet::clear()reuse instead of a per-pass arena realloc, a sharedas_direct_eval_callpredicate for producer and consumer, plainclone_in(id-less) at the one site that cloned semantic ids, and removal of deaddrop_consolerepair code.Verification
cargo test -p oxc_minifier -p oxc_mangler -p oxc_semantic -p oxc_allocator— all pass, incl. the new leak-regression testsjust minsize— bit-identical output; kitchen-sink.tsx (not in minsize) separately verified byte-identical to maincargo coverage -- minifier— test262 44825/44829, babel 1785/1785, snapshots unchangedTest alignment in oxc_transformer_plugins
The
replace_global_definesintegration-test helper previously reused RGD's returned scoping directly for DCE — a path production does not take:crates/oxc/src/compiler.rsmarks scoping dirty whenever RGD changed the AST (scoping_dirty |= ret.changed) and rebuilds before compress/DCE, and rolldown minifies freshly-parsed chunks. With the full-walkLiveUsageCollectorgone, that test-only path surfaced stale ambient-declarereferences. The helper now mirrors the production pipeline (rebuild whenret.changed, reuse otherwise). NoReplaceGlobalDefinessource changes. Twodeclare-define contract tests added.Known trade-off: defines-heavy DCE pays O(dropped) drop-walks
Profiling
pipeline[react.development.js](withprocess.env.NODE_ENVdefines) shows the worst case of walk-what-you-drop: DCE drops oneifsubtree covering ~90% of the program's nodes, so the drop-walk approaches a full-program traversal where the old per-pass live walk only covered the small remainder (−3% CodSpeed / +7% local wall on that bench; attribution: ~72% drop-walks, 0% double-marking, ~0% bitset cost). A bounded-walk + live-walk-fallback mitigation was prototyped and validated (recovers to +1.4%, all other wins preserved), but we decided the added mechanism wasn't worth the maintenance cost for a one-bench regression — the same file's plain minification improves 12%, and kitchen-sink-class inputs (the load-bearing case) improve ~2×. Revisit if real-world defines-heavy workloads surface it.