perf(gc): skip the per-object layout side tables while they are provably empty (#7510) - #7525
Merged
Merged
Conversation
added 3 commits
August 6, 2026 13:54
…bly empty #6893 moved the canonical typed layout of a well-behaved object into the shape-keyed SHAPE_LAYOUTS map, leaving LAYOUT_SLOT_MASKS and TYPED_LAYOUTS holding only objects that diverged from their shape — on a monomorphic workload, nothing at all. Every allocation, typed-shape install, object death and relocation still probed both maps to clear a record that was not there: two RefCell round-trips plus two hashes each time. layout_forget_object was 14.5% of self time on the churn_alloc object-construction profile (#7510), nearly twice the allocator it was bookkeeping for. PER_OBJECT_LAYOUTS_NONEMPTY answers 'is there anything in either map' in one load. Its false state is a proof of emptiness — only an insert can break that, and every insert now routes through the guarded accessors in the new gc/layout_tables module, which arm it; the removal paths re-test both maps and clear it again. A stale true costs exactly the old probe, so the flag is an accelerator and never an authority. layout_note_slot also stops cloning the descriptor out of the map on every store: it computes a SlotVerdict inside the borrow and acts after it, so a Heap mask no longer allocates a Vec per write.
…t per element The emptiness fast path from the previous commit fired ONCE in 40 million calls on churn_alloc. One entry was holding both per-object maps hostage: the canonical keys_array of the program's single object shape. js_build_class_keys_array fills it with interned key strings and notes each element, which grows a per-array pointer mask; the shape cache then anchors that array for the program's lifetime (#179), so the mask never drains. Since ~every program builds at least one shape, the fast path was dead on arrival for essentially all of them. Once the last key is stored, the mask is replaced by the GC_LAYOUT_ALL_POINTERS header declaration — exactly true of a keys array (every slot in 0..length holds an interned string) and immutable for the rest of the program (growing a shape builds a NEW array, shape_keys_grown). The per-element notes during the fill stay: they keep the already-stored prefix traceable if allocating the next key string triggers a GC, and the declaration can only be made once the last slot is filled. churn_alloc now runs with both maps at zero entries and the fast path on ~100% of calls. The removal paths key their re-test on remove(..).is_some() so a workload that genuinely holds records (tree) runs the pre-#7510 instruction sequence plus a load.
|
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 (2)
📝 WalkthroughWalkthroughThe runtime centralizes per-object GC layout tables, adds an emptiness flag with hot-TLS access, updates layout operations and relocation, marks completed class key arrays as all-pointer layouts, and adds invariant and tracing tests. ChangesPer-object layout fast path
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant GCLayout
participant layout_tables
participant HotTls
participant GCTracer
GCLayout->>layout_tables: insert or query per-object descriptor or slot mask
layout_tables->>HotTls: read PER_OBJECT_LAYOUTS_NONEMPTY
layout_tables-->>GCLayout: return guarded layout metadata
GCLayout->>GCTracer: select pointer slots for tracing
Possibly related issues
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
This was referenced Aug 6, 2026
proggeramlug
pushed a commit
that referenced
this pull request
Aug 7, 2026
…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%.
This was referenced Aug 7, 2026
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Partial fix for #7510 (construction/death half of #5094). The ticket's acceptance bar is not met — see "Where this lands" — so #7510 stays open.
What was wrong
LAYOUT_SLOT_MASKSandTYPED_LAYOUTSare address-keyed thread-local maps. Since #6893 moved the canonical typed layout of a well-behaved object into the shape-keyedSHAPE_LAYOUTS, what is left in them is the residue: objects that diverged from their shape, objects with nokeys_array, ambiguous shapes. On a monomorphic workload that residue is empty for the whole run.Empty was not free. Every allocation (
layout_init_pointer_free), every typed-shape install, every object death (layout_clear_for_ptr) and every relocation (layout_transfer) probed both maps to clear whatever a previous tenant of a recycled address might have left — twoRefCellround-trips plus two hashes, per object, to remove nothing.Commit 1 — an emptiness proof, in one load
PER_OBJECT_LAYOUTS_NONEMPTYanswers "is there anything in either map at all". Itsfalsestate is a proof of emptiness: only an insert can break it, and every insert routes through the guarded accessors in the newgc/layout_tablesmodule, which arm it; the removal paths re-test and clear it again once the maps drain. A staletruecosts exactly the pre-#7510 probe — the flag is an accelerator, never an authority, andassert_flag_soundin the new tests asserts the implication after every transition that can populate or drain either map.layout_note_slotalso stops cloning the descriptor out of the map on every store. The clone existed only so thatlayout_set_typed_unknowncould not re-enter a liveRefCellborrow; it now computes a two-stateSlotVerdictinside the borrow and acts after it, so aHeapmask no longer allocates aVecper write.Commit 2 — the reason commit 1 was worth nothing on its own
Measured before believing it: the fast path fired once in 40 million calls on
churn_alloc. One entry was holding both maps hostage — the canonicalkeys_arrayof the program's single object shape.js_build_class_keys_arrayfills it with interned key strings and notes each element, which grows a per-array pointer mask; the shape cache then anchors that array for the program's lifetime (#179), so the mask never drains. Since ~every program builds at least one shape, the emptiness fast path was dead on arrival for essentially all of them — including, retroactively, theis_empty()guard #7469 added tolayout_forget_object.Once the last key is stored the mask is replaced by the
GC_LAYOUT_ALL_POINTERSheader declaration, which is exactly true of a keys array (every slot in0..lengthholds an interned string) and immutable for the rest of the program (growing a shape builds a new array —shape_keys_grown). The per-element notes during the fill stay: they are what keeps the already-stored prefix traceable if allocating the next key string triggers a GC, and the declaration can only be made once the last slot is filled.churn_allocnow runs with both maps at zero entries and the fast path on ~100% of calls.Measurements
Symbolicated leaf profile of
churn_alloc.ts(20M{v, w}literals): thegc::layoutfamily falls from 26.0% → 20.8%,layout_forget_object3.0% → 1.6%,js_gc_init_typed_shape_layout13.6% → 9.5%.Interleaved A/B (arms alternating per round, best-of-9 user CPU, corroborated on the pinned bench host):
push_numchurn_allocchurn,push_cls,deeplist,churn_readtreetreeis the one arm that can only lose: it genuinely holds per-object records, so its flag stays armed and it pays the fast-path test without getting the fast path. The removal paths key their re-test onremove(…).is_some()so an armed workload runs the pre-#7510 instruction sequence plus a load; that tooktreefrom −2.2% back to within noise on the pinned host, and the residual is at the edge of what either host can resolve.No GC regression.
PERRY_GC_TRACEis field-identical between the arms:churn105 cycles / 0.0036 GB copied,tree43 cycles / 0.0159 GB, promoted bytes byte-for-byte equal, peak RSS within ±2 MB.Where this lands
This does not meet #7510's acceptance bar (≥1.5× on
churn_alloc,gc::layoutbelow 8%), and the reason is worth recording: the profile the ticket was filed from is out of date.layout_forget_objectis no longer 14.5% ofchurn_alloc— after #7469/#7474/#7486/#7487/#7501 it is 3.0%, so removing it entirely could never have delivered 1.5×.What the profile now shows as the remaining layout cost is not the side tables at all. It is
js_gc_init_typed_shape_layout+shape_install_shared(~13% combined), which still rebuild both masks, re-probeSHAPE_LAYOUTSand re-compare the descriptor on every construction of an already-installed shape. That is #7510's item 1 verbatim ("construction should become a header bit-set, not a side-table insert") and is the next lever; the emptiness fast path this PR establishes is a prerequisite for it, not a substitute.Testing
cargo test --release -p perry-runtime— 1761 pass, 0 fail.gc/tests/layout_trace/per_object_tables.rs: the flag invariant across install / partial drain / typed downgrade / in-place mask growth, plus a witness that a shape's keys array declares all-pointer slots instead of a mask and still enumerates and traces all three key strings — so a green test means the declaration is as precise as the mask it replaced, not that the entry merely disappeared.test-files/compiled and run under both arms: byte-identical output, and identical again underPERRY_GC_ZEAL=1 PERRY_GC_PROTECT_FROMSPACE=1(the arm that would catch an imprecise all-pointer declaration by reclaiming or relocating a key string out from under the array).cargo fmt --all -- --checkclean;scripts/check_file_size.shclean (layout.rswas 75 lines under the 2000-line cap, hence thelayout_tablessplit).Summary by CodeRabbit
Performance
Bug Fixes
Tests