fix(gc/shape): root and rewrite the keys edge from the ShapeId descriptor, not ObjectHeader.keys_array - #8221
Conversation
|
Warning Review limit reached
Next review available in: 55 seconds Limit details: You’ve used all 8 included reviews currently available under your plan. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (15)
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 |
7b8257e to
3cfd4e1
Compare
…ptor, not ObjectHeader.keys_array `ObjectHeader.keys_array` was the last blocker for #8047's header shrink: the descriptor was already authoritative for the keys VALUE, but the header word was the only thing that rooted the keys array and the only rewritable location the evacuator could hand to a slot visitor. Descriptor records are now boxed, so each record's address is fixed for its lifetime, and the collector enumerates `&mut record.keys` as an ordinary child slot. The post-visit write-back callback and the header fact-capture block it fed are deleted; the header word is a derived mirror. Liveness is an ephemeron gate with two halves, because per-receiver emission alone is unsound for a generational minor: a minor never enumerates old carriers, and the record is shared, so one sibling's rewrite creates an old->young edge no per-parent remembered-set page can describe. `PERRY_GC_VERIFY_EVACUATION` established this by aborting on exactly that edge. `ShapeDescriptor::old_carrier` is armed by the slot visitor for any non-nursery receiver, rooted by `scan_shape_table_rekey_mut`, and recomputed by every full trace, so the gate never becomes unconditional table rooting. Closes #8112.
3cfd4e1 to
86feda4
Compare
Closes #8112. Gate on #8047.
ObjectHeader.keys_arraywas the last blocker for #8047's header shrink. #8086made the
ShapeIddescriptor authoritative for the keys value, but theheader word was still the only thing that rooted the keys array and the only
rewritable location the evacuator could hand to a slot visitor — so deleting
it would have unrooted every keys array in the heap. This is the missing GC
protocol.
ObjectHeader's size is unchanged.What changed
The edge lives in the descriptor.
ShapeDescriptorrecords are BOXED, soeach record's address is fixed for its lifetime, and a lifted descriptor carries
record— the address it came from — alongside thekeyssnapshot.object::gc_shape_keys_edge_slothands the collector&mut record.keys, anordinary child slot it marks through and rewrites in place. The address rides on
the descriptor
gc_child_slotsalready resolved for the receiver, so the edgecosts no extra shape-table probe (#8122's one-probe rule).
synchronize_live_object_shape_descriptor_after_header_visitis deleted,along with the fact-capture block in
visit_gc_layout_slot_descriptorsthat fedit. Nothing in the slot visitor reads
keys_arrayfor a fact any more — which iswhat makes #8047 a deletion rather than a rewrite. The header word is demoted to
a derived mirror the collector refreshes and keeps forwarding.
Liveness is a real ephemeron gate, and it had to be. The obvious
simplification — emit the descriptor edge once per TRACED receiver and let
per-object liveness be the ephemeron relation — is wrong for a generational
minor, and
PERRY_GC_VERIFY_EVACUATIONsaid so on the first end-to-end run:A minor never enumerates old objects, so an old carrier's edge is never emitted;
and the record is SHARED, so one sibling's rewrite creates an old→young edge for
a parent the minor never visits and which no per-parent remembered-set page can
describe. The gate is therefore per-descriptor state:
old_carrier, armed by theslot visitor for any non-nursery receiver, consulted by
scan_shape_table_rekey_mut, which roots exactly those records and leaves therest to metadata rewriting. A full trace enumerates every live object, so
rotate_old_carrier_epoch_after_full_tracerecomputes the gate from what thattrace saw: it over-approximates by at most one full collection — the generational
contract — and never becomes unconditional table rooting.
gc/shape_keys_edge.rs(its own file:barrier/mod.rsis at 1995 lines andcycle.rsat 1991) answers the one question the old→young verifier has to askabout a shared word.
Two findings worth the issue's time
The census came first. Peak descriptor population over the 14
gc_ratchetprobes plus a 1 MB JSON-parse kernel is ~4 250 descriptors naming ~1 150
distinct keys arrays, dominated by a fixed bootstrap cohort; steady-state
workload contribution is 1–229 per cycle. So the protocol is not a scaling
problem and no exotic data structure is warranted.
The issue's requirement (1) — stable-address descriptor storage — is
necessary after all. The lift/write-back
scan_shape_table_rekey_mutalreadyperforms covers the rewrite only. A strong, rewritable edge needs an address
the collector can hold across a budgeted resumption, and boxing the record is the
cheapest way to get one: the bucket may rehash, the record does not.
Validation
shape_churn.ts— 400 rounds × 400 records over 7 shapes, with a 120-recordretained window whose every key is re-read by name after each collection
window, so a lost or mis-rewritten keys array is a thrown error rather than a
silent counter — under
PERRY_GC_SCHEDULE_RATE+PERRY_GC_SCHEDULE_SEED={1,7,23}+PERRY_GC_FORCE_EVACUATE+PERRY_GC_VERIFY_EVACUATION+PERRY_GC_PROTECT_FROMSPACE. Instruments proven armed, not assumed:benchmarks/gc_ratchet/gc_ratchet.py check --profile shared_ci: OK. Everygating structural cell —
heap_used_bytes,heap_total_bytes,minor_cycles,step_cycles,copied_objects,copied_bytes,promoted_objects,promoted_bytes,freed_bytes— is +0.00% across all 14 probes (two probesimprove
heap_used_bytesby ~2%). Onlywall_msdrifts, non-gating: measuredwith a
perry-devbinary on a host at load 48 versus a release binary at load2.2 when the baseline was captured. Baseline not re-pinned.
Suites:
perry-runtime --lib2527 / 0 / 4 (2523 on this base + 4 new);perry --bin perry987 / 0;perry-codegen --no-fail-fast1493 / 9,the failure set identical by name to this base — all nine are buffer-view /
typed-feedback / loop-poll codegen facts. They cannot be this PR's:
perry-codegendoes not depend on
perry-runtimeat all, and this diff is runtime-only.Fixtures
gc/tests/shape_keys_descriptor_edge.rs, four cases, each gating oncopied_objects > 0and on the receiver having actually moved:and siblings of one shape share exactly one edge;
descriptor survives a copying minor and the record is rewritten to the live
array — the perf(object/GC): finish the common-object header shrink from 56 B to 40 B after shape-transition migration #8047 rehearsal;
workload leaves the record stale or pruned. Without this arm a green run of the
previous case could not distinguish "the edge works" from "something else kept
the array alive";
as real as a use-after-free.
The suppression switches are
#[cfg(test)]thread-locals, not env knobs: theGC-knob kill policy requires a required CI arm for every shipped knob's off-state,
and neither state may be reachable in a shipped binary.
scripts/shape_descriptor_census.pygains four authority surfaces and threesabotage arms — un-boxing the record, un-gating the rooting arm, visiting the
derived mirror before the authoritative edge, and reading the mirror for a fact
are each red. The two-armed rooting expression is pinned structurally, not just
by which APIs it calls. Raw
keys_arraycallsites drop 183 → 180.object/shapes.rsreached 2048 lines, so its three unit suites moved verbatim toobject/shapes_tests.rs.Left for #8047
Deleting
ObjectHeader.keys_arraynow removesgc_keys_array_slot, its callsite, and the mirror refresh — nothing else. The one invariant #8047 inherits:
an old receiver's shape stamp store must keep dirtying its page, since that is
what puts the receiver in front of the dirty scan that arms this gate.