perf(codegen): take the inline bump allocator at new sites inside loops (#7469) - #7566
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThe code generator now uses inline allocation for ChangesLoop-sensitive inline allocation
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant NewLowering
participant LoopTargets
participant Environment
participant Allocator
NewLowering->>LoopTargets: Check whether the new site is inside a loop
NewLowering->>Environment: Read PERRY_INLINE_NEW
Environment-->>NewLowering: Return override state
NewLowering->>Allocator: Select inline allocation for loops or forced mode
NewLowering->>Allocator: Select outlined allocation otherwise
Possibly related PRs
Suggested reviewers: ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
…oops The outlined per-`new`-site allocator has been the default since [#bloat]: it collapses ~145 lines of per-class-constant IR per site into one js_object_alloc_class_inline_keys call. The size half of that decision still holds — measured ~268 bytes of machine code per site, +214,656 over an 800-site program. The SPEED half has inverted. The comment reads '~17% faster on an 8M-allocation loop'; today the outlined form is 1.81x SLOWER on churn_alloc and 1.78x on push_cls. Nothing about the inline bump changed — everything around the allocation got cheaper (#7474 #7486 #7487 #7501 #7525 #7532 #7535 #7536 #7552), so the surviving FFI call and the thread-local resolutions it performs now dominate what its code bloat costs. Those resolutions cannot be made cheaper on Darwin: Mach-O has no local-exec TLS model, and building the runtime with -Ztls-model=local-exec leaves the blr through the TLV descriptor byte-identical (measured 1.02x). Only their count can be reduced. So the choice becomes per site rather than global. A `new` inside a loop takes the inline bump; everything else keeps the outlined call and adds nothing to binary size. Loop membership reuses the existing loop_targets stack — switch frames push an empty continue label, every loop pushes a real one, the same discriminator Stmt::Continue already relies on. Measured: churn_alloc 1.81x, push_cls 1.81x, churn 1.56x — the full unconditional-inline ceiling. Size +0 bytes for 800 sites none of which are in loops; equal to all-inline when every site is. tree is -1.4%.
f411373 to
404985b
Compare
Post-merge audit — verified independently before mergingMerged as v0.5.1324. Recording what I checked, because the PR's own validation The validation gap. The body validates with Ran it. Green, with the subject proven live:
The hazard I went looking for, and why it is not live. The inline path loads It is nonetheless safe: Worth stating plainly since it is load-bearing and unobvious: if the class The gate does what it says — verified by per-function IR differential rather
Non-loop sites are byte-for-byte unchanged, which is the claim the "+0 bytes" |
1 participant
#7469 workstream A. Takes the inline bump allocator at
newsites inside loops and leaves everything else outlined.Why the default was worth revisiting
The outlined allocator has been the default since the
[#bloat]work, on this measurement: "win-win vs inline: −45 IR lines/site AND ~17% faster on an 8M-allocation loop (the inline bump bloated the hot loop, hurting icache/regalloc more than the saved call)."The size half still holds. I re-measured it: ~268 bytes of machine code per site (+0 / +49,536 / +214,656 bytes across 10 / 200 / 800-site programs).
The speed half has inverted. Outlined is now 1.81× slower on
churn_allocand 1.78× onpush_cls. Nothing about the inline bump changed — everything around the allocation got cheaper (#7474, #7486, #7487, #7501, #7525, #7532, #7535, #7536, #7552), so the surviving FFI call and the thread-local resolutions it performs now dominate what its bloat costs.Why not just make TLS cheaper
Recorded so it is not attempted again: Mach-O has no local-exec TLS model. I built the entire runtime with
-Ztls-model=local-exec; theblrthrough the TLV descriptor inlayout_forget_objectis byte-identical andchurn_allocmoved 1.02×.Per-call price is already at the plain-global floor:
const-initthread_local!__threadpthread_getspecificWhat is expensive is the count — ~14 resolutions per allocation, because each FFI helper resolves independently and LLVM cannot CSE across the FFI boundary. Inlining removes the allocator's outright.
The gate
Per site rather than global. Loop membership is the cheapest sound proxy for "runs many times" and bounds the size cost to loop bodies. It reuses the existing
loop_targetsstack rather than adding a counter:switchframes push an empty continue label while every loop pushes a real one — the same discriminatorStmt::Continue's scan-outward-past-switch-frames logic already uses — so anewin a bareswitchis correctly not-in-a-loop.churn_allocpush_clschurndeeplistretaincyclestreeIt reaches the full unconditional-inline ceiling on the allocation-heavy shapes.
treecosts 1.4%, reproduced across two best-of-5 runs. It allocates in loops so it inlines, but its time is dominated by copying and promotion rather than the allocation call, so it pays the bloat without the win. That is the honest cost of a static proxy; a profile-guided or size-aware refinement could exclude it, and I have not attempted one.cyclesgives up 0.35× against the ceiling because some of its sites sit outside loops.Size, measured from both sides
Zero growth for non-loop sites; never worse than unconditional inlining in the worst case.
Two earlier versions of that worst-case program measured +0 for every arm and would have been reported as a success: the first let the object be scalar-replaced (it never escaped), the second used a constant trip count that got unrolled away. Both are noted in the changelog so the next person building a size probe checks the subject is live.
Testing
test-files/programs across both arms: identical output, and identical underPERRY_GC_ZEAL=1 PERRY_GC_PROTECT_FROMSPACE=1— the arm that would catch a mis-written object header, since the inline bump writes the header and zero-fills slots in generated code rather than in the runtime. The one apparent diff was aconsole.timeduration, which varies run-to-run on a single binary.perry-codegenfailure set unchanged fromorigin/main.cargo fmt --all -- --checkandscripts/check_file_size.shclean.PERRY_INLINE_NEW=1still forces the inline form everywhere for A/B work. Its test isis_none(), soPERRY_INLINE_NEW=""enables inlining — an empty string isSome(""). That cost me one bogus A/B table and is now called out at the site.Summary by CodeRabbit
Performance
PERRY_INLINE_NEWenvironment variable.Documentation