Repository navigation
perf(javascript): build inline's flattening helpers once, not per call - #22493
Conversation
inlineIntoCall built ten closures and an arrow for every call it looked at, 79,798 times per minify of typescript.js, to decide whether the called function flattens into the scope around the call. The helpers are now module-level functions handed one state object, which nothing they read optimizes through, so a single one serves every call.
|
github-actions
Bot
added
the
area: parser
|
This PR is packaged and the instant preview is available (159dfbf). Install it locally:
npm i -D webpack@https://pkg.pr.new/webpack@159dfbf
yarn add -D webpack@https://pkg.pr.new/webpack@159dfbf
pnpm add -D webpack@https://pkg.pr.new/webpack@159dfbf |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (1)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 5 remain after this review. 📝 WalkthroughWalkthrough
ChangesCall flattening
Suggested labels: Suggested reviewers: Priority: ⬇️ Low Change: Refactor Merge Risk: ⚪ Minimal · up to The shared-state change has no identified merge-blocking issue. It is ready to merge after normal checks. 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Warning Some tools did not complete. Review the errors below. 🔧 ast-grep (0.45.3)lib/javascript/syntax-printer.jsast-grep timed out on this file Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Types CoverageCoverage after merging perf/minify-inline-call-state into main will be
Coverage Report |
Generated code sizeComparing base (
Read No asset changed size. No runtime gained or lost a runtime module. Built |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #22493 +/- ##
=======================================
Coverage 96.07% 96.07%
=======================================
Files 773 773
Lines 124097 124172 +75
Branches 40579 40608 +29
=======================================
+ Hits 119222 119301 +79
+ Misses 4875 4871 -4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Merging this PR will improve performance by 26.84%
Performance Changes
Tip Curious why performance improved? Comment Comparing 
Footnotes
|
Summary
inlineIntoCalldecides for every call whether the function called flattens into the scope around the call. It did so through ten inner functions and an arrow built on each call: 79,798 times per minify of typescript.js, 42 MB.The helpers are now module-level functions handed one
InlineStateholding what they read (the call, the compressor, the function, the scope found and the loop state). Nothing a helper reads runs the optimizer, so no call's decision runs inside another's, and a single state serves every call; it is let go of once the decision is made, before the flattened call is optimized. Thearguments.every(arrow)test for a spread became a loop.Second of the per-call allocation sites named in the plan, after #22492.
Before / after against
main:passes:2(callgrind, CodSpeed flags, warm)passes:2(callgrind, CodSpeed flags, cold)lodash gains nothing: with
topleveloff, nothing in it inlines at the top level. Output is byte-identical tomain: lodash, acorn and typescript.js under three option sets (default,passes:2+toplevel+unsafe,unused: "keep_assign"), and the terser, swc and oxc corpora (467,874 comparisons, 0 mismatches).What kind of change does this PR introduce?
perf
Did you add tests for your changes?
No new tests: output does not change, and the existing
inlinetests cover every helper moved.Does this PR introduce a breaking change?
No.
If relevant, what needs to be documented once your changes are merged or what have you already documented?
n/a
Use of AI
Written with Claude Code: it profiled the minifier's allocations, made the change, checked output against
mainand measured allocation and CPU. The author reviews every commit.🤖 Generated with Claude Code
https://claude.ai/code/session_01Tgxgyb4N35ndP6cC4KcrXC
Generated by Claude Code
Summary by CodeRabbit