feat(minifier): remove unreachable recursive functions#24125
Conversation
Merging this PR will regress 1 benchmark
Warning Please fix the performance issues or acknowledge them on CodSpeed. Performance Changes
Tip Investigate this regression by commenting Comparing Footnotes
|
90fbef6 to
16e26dd
Compare
0ac62bc to
cca73a6
Compare
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. |
cca73a6 to
8c1d74b
Compare
16e26dd to
54ad60c
Compare
Monitor OxcCommit:
|
e7c262d to
8fb1d34
Compare
8fb1d34 to
5ab2b8c
Compare
7defb37 to
0f64006
Compare
0f64006 to
78da987
Compare
5ab2b8c to
c651b5b
Compare
78da987 to
1d41780
Compare
1931f80 to
b6de9be
Compare
1d41780 to
111c30d
Compare
d2cb6e2 to
2fa0d96
Compare
8ad3e07 to
84438bb
Compare
86e3737 to
c405a12
Compare
84438bb to
5e130c9
Compare
c405a12 to
5abf9a7
Compare
5e130c9 to
bff6562
Compare
5abf9a7 to
6dab15b
Compare
bf11ebf to
e8873c9
Compare
6dab15b to
a46ba8a
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
I paired with Codex and Claude Code to review much of this PR from different angles. I think I've got a solid understanding of the algorithm and its logic. I made several follow-up commits, mostly focused on naming, simplification, and comments to make the implementation easier to follow. How did I do it? I worked through the parts I found difficult with both tools. I asked them to explain the reasoning, challenge proposed changes, point out why some ideas would not work, and suggest simpler alternatives. When their suggestions conflicted, I compared their reasoning and asked follow-up questions. I only made changes after I understood the context and trade-offs and agreed with the conclusion. |
|
@armano2 I am happy you are reviewing this! I made several changes locally while reviewing this PR. And now I think this PR is ready for review. Most of the changes I made after your review are making this PR into a good state for reviewing and there are no behavior changes, as I mentioned in #24125 (comment). Feel free to review again! |
Merge activity
|
Reference counts cannot remove unreachable recursive functions because their self- and mutual references keep the counts nonzero even when no executable code can reach them.
This change computes function reachability after scoping is flushed. Normalize registers function declarations and bindings with implicit runtime observers. References outside registered functions make their targets live; references owned by a live registered function propagate liveness to their targets.
Graph deadness is used only to remove function declarations. Other transformations continue to use ordinary reference counts together with implicit-observability checks. Direct `eval` disables the analysis for the whole program while present. Exports, Script-root bindings, Annex B aliases, and `using` disposal remain observable even when they have no resolved AST reference.
## Example
Input:
```js
function self() { self() }
function a() { b() }
function b() { a() }
export {};
```
Output:
```js
export {};
```
## Supported
- Self-recursive and mutually recursive function declarations.
- References nested in blocks, arrows, function expressions, classes, and parameter defaults. The nearest registered function declaration owns each reference.
- ES modules, CommonJS, and local functions in Scripts. Script-root bindings remain visible to later scripts.
- Full compression and DCE-only mode.
- Reads, writes, exports, `using` disposal, and direct `eval` preserve live functions.
- Exact self-recursive variable initializers whose value is a `FunctionExpression` or `ArrowFunctionExpression`, including classic `for` initializers.
## Conservatively retained
- Mutual cycles between variable declarators, such as `const a = () => b(); const b = () => a()`.
- Recursive class cycles and mixed function/declarator/class cycles.
- Wrapped or arbitrary function-valued initializers, such as `const f = (effect(), () => f())`.
- Recursive declarators in `for-in` and `for-of` heads.
- Sloppy unhoisted Annex B block functions, whose runtime alias write is not represented by their block-scoped symbol.
## Performance
[CodSpeed's report for `e71c392`](#24125 (comment)) measured a 3.74% regression on `binder.ts`, where the analysis finds no dead graph. On `kitchen-sink.tsx`, removing 39 function declarations covering 25.3 kB of source early improved the minifier benchmark by 21.01% and the full pipeline by 8.76%. A separate 15-run Script/CommonJS corpus measured a 1.67% median increase overall, below the 2% acceptance threshold.
Verified with all-feature workspace tests, lint, rustdoc, formatting, AST generation, minsize and allocation snapshots, and the repeat-compression corpus.
Closes #13105
Closes #17364
Implemented with AI assistance (OpenAI Codex). The contributor remains responsible for reviewing, understanding, and submitting the changes under the repository's AI usage policy.

Reference counts cannot remove unreachable recursive functions because their self- and mutual references keep the counts nonzero even when no executable code can reach them.
This change computes function reachability after scoping is flushed. Normalize registers function declarations and bindings with implicit runtime observers. References outside registered functions make their targets live; references owned by a live registered function propagate liveness to their targets.
Graph deadness is used only to remove function declarations. Other transformations continue to use ordinary reference counts together with implicit-observability checks. Direct
evaldisables the analysis for the whole program while present. Exports, Script-root bindings, Annex B aliases, andusingdisposal remain observable even when they have no resolved AST reference.Example
Input:
Output:
Supported
usingdisposal, and directevalpreserve live functions.FunctionExpressionorArrowFunctionExpression, including classicforinitializers.Conservatively retained
const a = () => b(); const b = () => a().const f = (effect(), () => f()).for-inandfor-ofheads.Performance
CodSpeed's report for
e71c392measured a 3.74% regression onbinder.ts, where the analysis finds no dead graph. Onkitchen-sink.tsx, removing 39 function declarations covering 25.3 kB of source early improved the minifier benchmark by 21.01% and the full pipeline by 8.76%. A separate 15-run Script/CommonJS corpus measured a 1.67% median increase overall, below the 2% acceptance threshold.Verified with all-feature workspace tests, lint, rustdoc, formatting, AST generation, minsize and allocation snapshots, and the repeat-compression corpus.
Closes #13105
Closes #17364
Implemented with AI assistance (OpenAI Codex). The contributor remains responsible for reviewing, understanding, and submitting the changes under the repository's AI usage policy.