perf(semantic,mangler,minifier): fix Semantic::stats node count and reuse stats in mangler builds#23352
Conversation
Merging this PR will not alter performance
Comparing Footnotes
|
Merge activity
|
… reuse stats in mangler builds (#23352) ## What Fix three allocation problems in the semantic rebuilds performed by the minify/mangle paths, found while auditing the compiler pipeline's scoping rebuilds. ## Details - **`Semantic::stats` reported `nodes: 0` after node-less builds.** It read `self.nodes.len()`, which is empty when the builder runs in ancestry mode (`with_build_nodes(false)` — the compiler pipeline default). `Semantic` now tracks `node_count` (from `AstNodeStore::node_count`, the source of truth in both modes) and `stats()` returns it. The mangler build inside `Minifier::build` previously received `nodes: 0` and grew the `AstNodes` store from zero capacity by repeated doubling; it now pre-allocates correctly. - **`Minifier::build` with `compress: None` fed the mangler `Stats::default()`.** All-zero stats are worse than no stats: they skip the `Stats::count` fallback *and* reserve zero capacity for every table. Stats are now threaded as `Option<Stats>` and only passed when the compress phase produced them; the mangle-only path falls back to the counting pass and sizes correctly. - **`Mangler::build` always re-counted the AST.** Added `Mangler::with_stats`, and `CompilerInterface::mangle` now receives the stats from the initial semantic build, so the mangler's internal semantic rebuild skips its full-AST counting traversal. Output is unchanged — stats only affect pre-allocation. ## Note `CompilerInterface::mangle` gains a `stats: Stats` parameter — a breaking change for external implementors that override it (same shape as the `compress` change in #23153). 🤖 Generated with [Claude Code](https://claude.com/claude-code)
853b6ef to
be2bccd
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 853b6ef528
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
… reuse stats in mangler builds (#23352) Fix three allocation problems in the semantic rebuilds performed by the minify/mangle paths, found while auditing the compiler pipeline's scoping rebuilds. - **`Semantic::stats` reported `nodes: 0` after node-less builds.** It read `self.nodes.len()`, which is empty when the builder runs in ancestry mode (`with_build_nodes(false)` — the compiler pipeline default). `Semantic` now tracks `node_count` (from `AstNodeStore::node_count`, the source of truth in both modes) and `stats()` returns it. The mangler build inside `Minifier::build` previously received `nodes: 0` and grew the `AstNodes` store from zero capacity by repeated doubling; it now pre-allocates correctly. - **`Minifier::build` with `compress: None` fed the mangler `Stats::default()`.** All-zero stats are worse than no stats: they skip the `Stats::count` fallback *and* reserve zero capacity for every table. Stats are now threaded as `Option<Stats>` and only passed when the compress phase produced them; the mangle-only path falls back to the counting pass and sizes correctly. - **`Mangler::build` always re-counted the AST.** Added `Mangler::with_stats`, and `CompilerInterface::mangle` now receives the stats from the initial semantic build, so the mangler's internal semantic rebuild skips its full-AST counting traversal. Output is unchanged — stats only affect pre-allocation. `CompilerInterface::mangle` gains a `stats: Stats` parameter — a breaking change for external implementors that override it (same shape as the `compress` change in #23153). 🤖 Generated with [Claude Code](https://claude.com/claude-code)
be2bccd to
3170c0e
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3170c0e279
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let mut builder = | ||
| SemanticBuilder::new().with_build_nodes(true).with_class_table(true); | ||
| if let Some(stats) = stats { | ||
| builder = builder.with_stats(stats); |
There was a problem hiding this comment.
Recount after compression before mangling
When compress is enabled and removes most of a large file before mangle, the stats captured before compressor.build_with_scoping/DCE still describe the pre-compression program. Because this commit makes Semantic::stats() include node_count, passing those stale stats here makes the semantic rebuild for mangling reserve original-size node/scoping tables instead of counting the current AST, which can turn compression-heavy inputs into a large memory regression; please refresh stats after compression or skip with_stats for this path.
Useful? React with 👍 / 👎.
### 💥 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>
What
Fix three allocation problems in the semantic rebuilds performed by the minify/mangle paths, found while auditing the compiler pipeline's scoping rebuilds.
Details
Semantic::statsreportednodes: 0after node-less builds. It readself.nodes.len(), which is empty when the builder runs in ancestry mode (with_build_nodes(false)— the compiler pipeline default).Semanticnow tracksnode_count(fromAstNodeStore::node_count, the source of truth in both modes) andstats()returns it. The mangler build insideMinifier::buildpreviously receivednodes: 0and grew theAstNodesstore from zero capacity by repeated doubling; it now pre-allocates correctly.Minifier::buildwithcompress: Nonefed the manglerStats::default(). All-zero stats are worse than no stats: they skip theStats::countfallback and reserve zero capacity for every table. Stats are now threaded asOption<Stats>and only passed when the compress phase produced them; the mangle-only path falls back to the counting pass and sizes correctly.Mangler::buildalways re-counted the AST. AddedMangler::with_stats, andCompilerInterface::manglenow receives the stats from the initial semantic build, so the mangler's internal semantic rebuild skips its full-AST counting traversal.Output is unchanged — stats only affect pre-allocation.
Note
CompilerInterface::manglegains astats: Statsparameter — a breaking change for external implementors that override it (same shape as thecompresschange in #23153).🤖 Generated with Claude Code