From d2f7f16984d5fc17c78be241c74c89db37436a61 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Wed, 16 Sep 2026 10:37:41 +0000 Subject: [PATCH 1/4] perf(gc): borrow the shape record in the slot visitor, stop moving the iterator (#10362) Base: 33690c563 (main, includes the #10348 fix from #10352). gc_child_slots built a 152-byte HeapChildSlotIterator for every traced object, 40 bytes of it a lifted ShapeDescriptor (#8122). Two by-value moves of that iterator compiled to out-of-line 152-byte memcpy calls inside visit_gc_layout_slot_descriptors: * the Array and Closure arms' `Option::map(..).unwrap_or_else(..)` temporary, copied into the caller's slot (62% of the calls); * `for .. in child_slots` in the Masked arm, which moved the iterator into the loop (35%). On the #10362 retained-graph fixture that was 6,181,945 memcpy calls of exactly 152 bytes (counted with an LD_PRELOAD shim). After this change: 6. * object/shapes.rs: ShapeRecordRef, a Copy handle to one live slab record (live bound, keys word, keys slot). shape_record_by_id and object_shape_record make the same slab probe as the descriptor lookups, without lifting a copy. note_old_generation_carrier and note_full_trace_carrier take the handle. ShapeDescriptor::record_ref() adapts the one caller that holds a descriptor. keys_slot() is now used only by tests, so it is cfg(test). * object/gc_slots.rs: gc_shape_keys_edge_slot and gc_field_slot_range take the handle. * gc/layout.rs: the iterator carries Option (8 bytes; the iterator is now 120). The `_from` mask helpers take the handle. gc_child_slots' Array and Closure arms build the iterator directly in the return slot with let-else, as the ObjectFields arm already did. * gc/layout_slot_visit.rs: the Masked arm iterates `&mut child_slots`. Still one shape-table probe per receiver (#8122). The handle is read at the same points the lifted copy was: the carrier notes and the keys edge, before any visit. It relies on the same record-address validity the carrier notes already write through (#9706). The #8112 old_carrier/ephemeron gate and the #9726 full-trace note are unchanged. instructions:u, min of 5, same host, base (main 33690c563) vs this: gc3 12,572,092,967 -> 12,275,653,370 -2.36% w1000 1,090,107,336 -> 1,081,645,123 -0.78% w5000 2,049,860,743 -> 2,018,013,960 -1.55% w20000 5,094,775,666 -> 4,994,573,033 -1.97% oldyoung 1,525,484,973 -> 1,503,383,712 -1.45% alloc-only 320,266,130 -> 320,266,017 0.00% All outputs match node byte for byte. PERRY_GC_FROMSPACE_SCAN_ABORT=1 is clean on all six fixtures, and it aborts on the same gc3 built at fcd108bfb. PERRY_GC_VERIFY_EVACUATION=1 on gc3 exits 0. --- crates/perry-runtime/src/gc/layout.rs | 69 +++++------ .../perry-runtime/src/gc/layout_slot_visit.rs | 8 +- crates/perry-runtime/src/object/gc_slots.rs | 24 ++-- crates/perry-runtime/src/object/shapes.rs | 115 +++++++++++++----- 4 files changed, 137 insertions(+), 79 deletions(-) diff --git a/crates/perry-runtime/src/gc/layout.rs b/crates/perry-runtime/src/gc/layout.rs index f5fbd30a8b..72a5a7c0d2 100644 --- a/crates/perry-runtime/src/gc/layout.rs +++ b/crates/perry-runtime/src/gc/layout.rs @@ -203,22 +203,22 @@ unsafe fn with_shape_shared_descriptor( // ONE shape-table probe (#8122). This used to be two — one for the keys // edge (the retired `object_keys_array_ptr`) and one here for the live // bound — on every field store that reaches it and on every traced object. - let descriptor = crate::object::shapes::object_shape_descriptor(object); - with_shape_shared_descriptor_from(user_ptr, descriptor, f) + let shape = crate::object::shapes::object_shape_record(object); + with_shape_shared_descriptor_from(user_ptr, shape, f) } -/// [`with_shape_shared_descriptor`] against a receiver `ShapeDescriptor` the +/// [`with_shape_shared_descriptor`] against a receiver shape record the /// caller has already resolved (or found absent). The receiver MUST be an /// ObjectFields object — this skips the kind screen the probing form applies. /// -/// #8122: the collector's per-object path resolves the descriptor once in +/// #8122: the collector's per-object path resolves the record once in /// `gc_child_slots` and hands it down here through /// [`HeapChildSlotIterator::new_object`], instead of re-probing the shape /// table for the keys edge and again for the live bound. #[inline] unsafe fn with_shape_shared_descriptor_from( user_ptr: usize, - descriptor: Option, + shape: Option, f: impl Fn(&TypedLayoutDescriptor) -> R, ) -> Option { let object = user_ptr as *const crate::object::ObjectHeader; @@ -228,10 +228,8 @@ unsafe fn with_shape_shared_descriptor_from( } // Defense-in-depth: both descriptor families must agree on the exact live // bound. #8113: an unstamped receiver has no bound anywhere, so 0 — not a - // second probe (`unwrap_or` is eager). - let field_count = descriptor - .map(|descriptor| descriptor.live_inline_slot_count as usize) - .unwrap_or(0); + // second probe (`map_or`'s default is eager). + let field_count = shape.map_or(0, |shape| shape.live_inline_slot_count() as usize); if shape_layout_keyed_enabled() { let map = hot_shape_layouts().borrow(); if let Some(desc) = map.get(&shape_id) { @@ -320,19 +318,18 @@ unsafe fn shape_shared_pointer_mask( with_shape_shared_descriptor(user_ptr, |d| d.pointer_mask.clone()) } -/// [`shape_shared_pointer_mask`] for an ObjectFields receiver whose -/// `ShapeDescriptor` the caller already resolved (#8122, see -/// [`with_shape_shared_descriptor_from`]). +/// [`shape_shared_pointer_mask`] for an ObjectFields receiver whose shape record +/// the caller already resolved (#8122, see [`with_shape_shared_descriptor_from`]). #[inline] unsafe fn shape_shared_pointer_mask_from( user_ptr: usize, header: *const GcHeader, - descriptor: Option, + shape: Option, ) -> Option { if (*header)._reserved & GC_OBJ_TYPED_LAYOUT_INTACT == 0 { return None; } - with_shape_shared_descriptor_from(user_ptr, descriptor, |d| d.pointer_mask.clone()) + with_shape_shared_descriptor_from(user_ptr, shape, |d| d.pointer_mask.clone()) } /// Install `descriptor` as the canonical layout for `shape_id` and set the @@ -1525,12 +1522,12 @@ pub(crate) struct HeapChildSlotIterator { pub(super) meta_slot2: Option<*mut u64>, pub(super) payload: HeapSlotRange, pub(super) selection: HeapPayloadSlotSelection, - /// #8122: the receiver's `ShapeDescriptor`, resolved ONCE by - /// [`gc_child_slots`] for an ObjectFields object and carried here so - /// `visit_gc_layout_slot_descriptors` reads the same facts instead of + /// #8122: the receiver's shape record, resolved ONCE by [`gc_child_slots`] + /// for an ObjectFields object and borrowed here in place (#10362), so + /// `visit_gc_layout_slot_descriptors` reads the same record instead of /// probing the shape table again. `None` for every other kind, and for /// an unstamped object. - pub(super) object_shape: Option, + pub(super) object_shape: Option, } impl HeapChildSlotIterator { @@ -1561,7 +1558,7 @@ impl HeapChildSlotIterator { } } - /// [`Self::new`] for an ObjectFields receiver whose `ShapeDescriptor` the + /// [`Self::new`] for an ObjectFields receiver whose shape record the /// caller already resolved (#8122). The payload-mask selection reuses it /// instead of probing the shape table, and it is retained on the iterator /// for the slot visitor. @@ -1569,7 +1566,7 @@ impl HeapChildSlotIterator { header: *mut GcHeader, prefix_slot: Option<*mut u64>, payload: HeapSlotRange, - object_shape: Option, + object_shape: Option, ) -> Self { let selection = unsafe { heap_payload_slot_selection_from(header, payload, object_shape) }; Self { @@ -1718,16 +1715,16 @@ pub(super) unsafe fn heap_payload_slot_selection( }) } -/// [`heap_payload_slot_selection`] for an ObjectFields receiver whose -/// `ShapeDescriptor` the caller already resolved (#8122): the shared-shape +/// [`heap_payload_slot_selection`] for an ObjectFields receiver whose shape +/// record the caller already resolved (#8122): the shared-shape /// pointer-mask lookup reuses it instead of probing the shape table twice. pub(super) unsafe fn heap_payload_slot_selection_from( header: *mut GcHeader, payload: HeapSlotRange, - descriptor: Option, + shape: Option, ) -> HeapPayloadSlotSelection { heap_payload_slot_selection_impl(header, payload, |user_ptr, header| { - shape_shared_pointer_mask_from(user_ptr, header, descriptor) + shape_shared_pointer_mask_from(user_ptr, header, shape) }) } @@ -1791,6 +1788,8 @@ unsafe fn heap_payload_slot_selection_impl( } } +/// #10362: every arm returns the iterator it builds, never through an `Option` +/// combinator whose temporary is copied out — a per-object memmove per GC walk. pub(super) unsafe fn gc_child_slots(header: *mut GcHeader) -> HeapChildSlotIterator { if header.is_null() || (*header).gc_flags & GC_FLAG_FORWARDED != 0 { return HeapChildSlotIterator::empty(); @@ -1799,20 +1798,21 @@ pub(super) unsafe fn gc_child_slots(header: *mut GcHeader) -> HeapChildSlotItera match gc_type_layout_slot_kind((*header).obj_type) { GcLayoutSlotKind::ArrayElements => { let arr = user_ptr as *mut crate::array::ArrayHeader; - crate::array::gc_element_slot_range(arr) - .map(|range| HeapChildSlotIterator::new(header, None, range)) - .unwrap_or_else(HeapChildSlotIterator::empty) + let Some(range) = crate::array::gc_element_slot_range(arr) else { + return HeapChildSlotIterator::empty(); + }; + HeapChildSlotIterator::new(header, None, range) } GcLayoutSlotKind::ObjectFields => { let obj = user_ptr as *mut crate::object::ObjectHeader; - // #8122: resolve the receiver's ShapeDescriptor ONCE and thread it + // #8122: resolve the receiver's shape record ONCE and thread it // through every step that needs a shape fact — the field range, // the keys edge, the shared pointer mask (`new_object`) and the // slot visitor (`object_shape` on the iterator). These used to be // five independent `shape_descriptor_by_id` probes per traced // object, the top leaf of a traced in-place-promotion cycle. - let descriptor = crate::object::shapes::object_shape_descriptor(obj); - let Some(range) = crate::object::gc_field_slot_range(obj, descriptor) else { + let shape = crate::object::shapes::object_shape_record(obj); + let Some(range) = crate::object::gc_field_slot_range(obj, shape) else { return HeapChildSlotIterator::empty(); }; // #6812: the meta record is a raw-pointer child edge; before the @@ -1821,7 +1821,7 @@ pub(super) unsafe fn gc_child_slots(header: *mut GcHeader) -> HeapChildSlotItera // which are usually rooted elsewhere; fatal for the spill // buffer, reachable through meta alone). A second prefix slot // keeps payload slot indices aligned with the layout masks. - HeapChildSlotIterator::new_object(header, None, range, descriptor) + HeapChildSlotIterator::new_object(header, None, range, shape) .with_meta_slot(crate::object::gc_object_meta_slot(user_ptr as usize)) } GcLayoutSlotKind::RegExpFields => { @@ -1858,9 +1858,10 @@ pub(super) unsafe fn gc_child_slots(header: *mut GcHeader) -> HeapChildSlotItera } GcLayoutSlotKind::ClosureCaptures => { let closure = user_ptr as *mut crate::closure::ClosureHeader; - crate::closure::gc_capture_slot_range(closure) - .map(|range| HeapChildSlotIterator::new(header, None, range)) - .unwrap_or_else(HeapChildSlotIterator::empty) + let Some(range) = crate::closure::gc_capture_slot_range(closure) else { + return HeapChildSlotIterator::empty(); + }; + HeapChildSlotIterator::new(header, None, range) } GcLayoutSlotKind::None => HeapChildSlotIterator::empty(), } diff --git a/crates/perry-runtime/src/gc/layout_slot_visit.rs b/crates/perry-runtime/src/gc/layout_slot_visit.rs index 02d73fc689..68ef1bccb0 100644 --- a/crates/perry-runtime/src/gc/layout_slot_visit.rs +++ b/crates/perry-runtime/src/gc/layout_slot_visit.rs @@ -30,10 +30,10 @@ pub(super) unsafe fn visit_gc_layout_slot_descriptors( visit(fixed_slot(slot)); }); } - // #8112: the authoritative ordered-keys edge, taken from the descriptor + // #8112: the authoritative ordered-keys edge, taken from the shape record // `gc_child_slots` already resolved for this receiver. It is the boxed // record's OWN `keys` word, so the collector marks through it and rewrites - // it in place — the descriptor is the root and the rewritable location. + // it in place — the record is the root and the rewritable location. // // Never enumerate the HashMap BUCKET as a GC slot: dirty-page work may // retain enumerated slot addresses across budgeted resumptions, during @@ -117,7 +117,9 @@ pub(super) unsafe fn visit_gc_layout_slot_descriptors( }); } HeapPayloadSlotScan::Masked => { - for child_slot in child_slots { + // Iterate by reference: `for .. in child_slots` moves the iterator + // into the loop, a copy per traced object (#10362). + for child_slot in &mut child_slots { if let HeapChildSlot::Child(slot, layout_kind) = child_slot { visit(GcMutableSlotDescriptor::Slot(GcMutableSlot::new( slot, diff --git a/crates/perry-runtime/src/object/gc_slots.rs b/crates/perry-runtime/src/object/gc_slots.rs index 4dd728aeef..a47cbc5c87 100644 --- a/crates/perry-runtime/src/object/gc_slots.rs +++ b/crates/perry-runtime/src/object/gc_slots.rs @@ -3,10 +3,10 @@ use crate::ArrayHeader; /// The AUTHORITATIVE ordered-keys edge of a traced receiver (#8112). /// -/// This is the descriptor's own `keys` word, not a copy of it: the record is -/// boxed (`object::shapes::ShapeDescriptor`), so its address is fixed for the +/// This is the shape record's own `keys` word, not a copy of it: the record is +/// boxed (`object::shapes::ShapeRecordRef`), so its address is fixed for the /// record's lifetime and the collector can mark through it and rewrite it in -/// place like any other child slot. The address rides along on the descriptor +/// place like any other child slot. The address is the record handle /// `gc::layout::gc_child_slots` already resolved for this receiver, so the /// edge costs no extra shape-table probe (#8122's one-probe rule) and needs no /// post-visit write-back callback. @@ -20,9 +20,7 @@ use crate::ArrayHeader; /// deliberate — the alternative loses a shape whose only carrier is promoted /// during the very drain that would have emitted it. #[inline] -pub(crate) fn gc_shape_keys_edge_slot( - descriptor: Option, -) -> Option<*mut u64> { +pub(crate) fn gc_shape_keys_edge_slot(record: Option) -> Option<*mut u64> { #[cfg(test)] if shapes::test_keys_edge_suppressed() { // Sabotage arm: without this edge a keys array has no root and no @@ -31,18 +29,18 @@ pub(crate) fn gc_shape_keys_edge_slot( // the detector works, not that nothing was tried. return None; } - let descriptor = descriptor?; - if descriptor.keys == 0 { + let record = record?; + if record.keys() == 0 { return None; } - descriptor.keys_slot() + Some(record.keys_slot()) } -/// The object's inline field-slot range, given the receiver's `ShapeDescriptor` +/// The object's inline field-slot range, given the receiver's shape record /// resolved once by the collector. pub(crate) unsafe fn gc_field_slot_range( obj: *mut ObjectHeader, - descriptor: Option, + record: Option, ) -> Option { if obj.is_null() { return None; @@ -56,8 +54,8 @@ pub(crate) unsafe fn gc_field_slot_range( // escapes (`object/alloc.rs`), and every bound change is mint-then-stamp // (`shapes::publish_object_live_slot_count`), so a live object is never // observed here without one. - let field_count = descriptor - .map(|descriptor| descriptor.live_inline_slot_count as usize) + let field_count = record + .map(|record| record.live_inline_slot_count() as usize) .unwrap_or(0); if field_count > 1_000_000 { return None; diff --git a/crates/perry-runtime/src/object/shapes.rs b/crates/perry-runtime/src/object/shapes.rs index 8d7d435b0a..de30fc8f15 100644 --- a/crates/perry-runtime/src/object/shapes.rs +++ b/crates/perry-runtime/src/object/shapes.rs @@ -138,16 +138,61 @@ pub(crate) struct ShapeDescriptor { /// lifted out of the table compares equal to the record it came from. impl ShapeDescriptor { /// The one `keys` word the collector rewrites for this shape, or `None` - /// for a descriptor value that was never lifted out of the table. - /// - /// `keys` is the first field of the `#[repr(C)]` slab record, so the - /// record address IS the slot address. + /// for a descriptor value that was never lifted out of the table. The + /// collector itself asks [`ShapeRecordRef::keys_slot`] (#10362). + #[cfg(test)] #[inline] pub(crate) fn keys_slot(&self) -> Option<*mut u64> { - if self.record == 0 { - return None; - } - Some(self.record as *mut u64) + self.record_ref().map(ShapeRecordRef::keys_slot) + } + + /// The slab record this value was lifted from, or `None` for a descriptor + /// built outside the table. + #[inline] + pub(crate) fn record_ref(&self) -> Option { + std::ptr::NonNull::new(self.record as *mut ShapeRecord).map(ShapeRecordRef) + } +} + +/// One live slab record, borrowed in place rather than lifted (#10362). +/// +/// The collector asks three things of a traced receiver's shape: the live +/// inline-slot bound, the record's own `keys` word (the rewritable edge, +/// #8112), and the record's liveness bits. All three live in the record, so it +/// resolves this handle ONCE per receiver (#8122's one-probe rule) and threads +/// it through every step instead of a lifted [`ShapeDescriptor`]. The lifted +/// copy is 40 bytes and rode on the per-object `HeapChildSlotIterator`, which +/// made that iterator too large to move without an out-of-line `memmove`. +/// +/// Validity is exactly `ShapeDescriptor::record`'s, which the carrier notes +/// already write through: record addresses never move (#9706), and a record's +/// chunk is released only by `shrink_shape_tables` at the end of a major +/// collection, after every enumeration of the cycle that resolved it. +#[derive(Clone, Copy)] +pub(crate) struct ShapeRecordRef(std::ptr::NonNull); + +impl ShapeRecordRef { + /// The record's live inline-slot bound — the same fact a lifted + /// descriptor's `live_inline_slot_count` copies. + #[inline] + pub(crate) fn live_inline_slot_count(self) -> u32 { + // SAFETY: a live slab record (type docs). + unsafe { (*self.0.as_ptr()).live_inline_slot_count } + } + + /// The record's current `keys` word (0 for a keyless shape). + #[inline] + pub(crate) fn keys(self) -> u64 { + // SAFETY: a live slab record (type docs). + unsafe { (*self.0.as_ptr()).keys } + } + + /// The `keys` word's address: the slot the collector marks through and + /// rewrites in place. `keys` is the first field of the `#[repr(C)]` slab + /// record, so the record address IS the slot address. + #[inline] + pub(crate) fn keys_slot(self) -> *mut u64 { + self.0.as_ptr() as *mut u64 } } @@ -684,6 +729,14 @@ pub(crate) fn shape_descriptor_by_id(shape_id: u32) -> Option { crate::state::state().shapes.slab().lift(shape_id) } +/// The record named by `shape_id`, borrowed in place: the same slab probe as +/// [`shape_descriptor_by_id`], without lifting a copy (#10362). +#[inline] +pub(crate) fn shape_record_by_id(shape_id: u32) -> Option { + let record = crate::state::state().shapes.slab().record_ptr(shape_id)?; + std::ptr::NonNull::new(record).map(ShapeRecordRef) +} + /// Immutable ordinary-vs-class fact with a pointer-free, per-agent direct /// cache. The first observation remains the authoritative descriptor lookup_ways; /// subsequent observations avoid the hot ShapeId HashMap borrow. @@ -699,7 +752,7 @@ pub(crate) fn shape_object_kind_by_id(shape_id: u32) -> Option /// Record that a shape is carried by an OLD-generation receiver. /// -/// Called from the collector's slot visitor, which resolved the descriptor for +/// Called from the collector's slot visitor, which resolved the record for /// this receiver already, so the note costs a generation range check and a /// byte store — no second shape-table probe (#8122's one-probe rule). The /// store goes straight through the boxed record's address rather than @@ -708,25 +761,23 @@ pub(crate) fn shape_object_kind_by_id(shape_id: u32) -> Option /// /// # Safety /// -/// `descriptor.record`, when non-zero, is the address of a live slab record -/// owned by this agent's shape table. Records are retired only by the table's +/// `record` is a live slab record owned by this agent's shape table (the +/// contract on [`ShapeRecordRef`]). Records are retired only by the table's /// own retirement paths, and their chunk is released by /// `shrink_shape_tables` at the end of a major collection — after every -/// enumeration of the cycle that produced this descriptor. +/// enumeration of the cycle that resolved this record. #[inline] -pub(crate) unsafe fn note_old_generation_carrier(descriptor: Option) { - let Some(descriptor) = descriptor else { +pub(crate) unsafe fn note_old_generation_carrier(record: Option) { + let Some(record) = record else { return; }; - if descriptor.record == 0 { - return; - } - let record = descriptor.record as *mut ShapeRecord; + let keys = record.keys(); + let record = record.0.as_ptr(); let first_note_this_epoch = !(*record).has(RECORD_FLAG_OLD_CARRIER_SEEN); // GC_STORE_AUDIT(POINTER_FREE): liveness bookkeeping bits, never a heap reference. (*record).set(RECORD_FLAG_OLD_CARRIER | RECORD_FLAG_OLD_CARRIER_SEEN, true); if first_note_this_epoch { - note_shape_carrier_candidate(descriptor.keys); + note_shape_carrier_candidate(keys); } } @@ -734,13 +785,11 @@ pub(crate) unsafe fn note_old_generation_carrier(descriptor: Option) { - let Some(descriptor) = descriptor else { +pub(crate) unsafe fn note_full_trace_carrier(record: Option) { + let Some(record) = record else { return; }; - if descriptor.record != 0 { - (*(descriptor.record as *mut ShapeRecord)).set(RECORD_FLAG_CARRIED_SEEN, true); - } + (*record.0.as_ptr()).set(RECORD_FLAG_CARRIED_SEEN, true); } #[inline] @@ -808,14 +857,14 @@ pub(crate) unsafe fn stamp_object_shape_id_with_carrier_note( ) { (*obj).parent_class_id = id; if !crate::arena::pointer_in_nursery(obj as usize) { - let descriptor = shape_descriptor_by_id(id); - note_old_generation_carrier(descriptor); + let record = shape_record_by_id(id); + note_old_generation_carrier(record); // This stamp is the structural-mutation publication funnel. Re-arm // even when the descriptor was already an old carrier: an owned // Longlived keys array may have just gained a nursery key at the same // address, and its carrier flag alone cannot express that transition. - if let Some(descriptor) = descriptor { - note_shape_carrier_candidate(descriptor.keys); + if let Some(record) = record { + note_shape_carrier_candidate(record.keys()); } } } @@ -1310,7 +1359,7 @@ pub(crate) unsafe fn try_birth_stamp_preinstalled_shape( } (*obj).parent_class_id = runtime_shape_id; if !crate::arena::pointer_in_nursery(obj as usize) { - note_old_generation_carrier(Some(descriptor)); + note_old_generation_carrier(descriptor.record_ref()); } debug_assert_object_shape_parity(obj); true @@ -1747,6 +1796,14 @@ pub(crate) unsafe fn object_shape_descriptor( shape_descriptor_by_id(object_shape_stamp(obj)) } +/// [`object_shape_descriptor`]'s record, borrowed in place (#10362). +#[inline] +pub(crate) unsafe fn object_shape_record( + obj: *const crate::object::ObjectHeader, +) -> Option { + shape_record_by_id(object_shape_stamp(obj)) +} + #[inline] pub(crate) unsafe fn object_shape_id(obj: *const crate::object::ObjectHeader) -> u32 { object_shape_descriptor(obj) From 6c9e2a6abb04773b3fcbed9e2631099d56b8661a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Wed, 16 Sep 2026 13:11:59 +0200 Subject: [PATCH 2/4] changelog: fragment for #10371 --- changelog.d/10371-gc-slot-iterator-memcpy.md | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 changelog.d/10371-gc-slot-iterator-memcpy.md diff --git a/changelog.d/10371-gc-slot-iterator-memcpy.md b/changelog.d/10371-gc-slot-iterator-memcpy.md new file mode 100644 index 0000000000..d38f7df7cd --- /dev/null +++ b/changelog.d/10371-gc-slot-iterator-memcpy.md @@ -0,0 +1,7 @@ +### Performance + +- **The GC slot visitor no longer copies a 152-byte iterator for every traced object (#10362).** `gc_child_slots` built a `HeapChildSlotIterator` per object, carrying a 40-byte `ShapeDescriptor` lifted out of the shape table (#8122). Two by-value moves turned it into an out-of-line `memcpy`: the Array/Closure arms' `Option::map(..).unwrap_or_else(..)` temporary, and `for .. in child_slots` in the masked arm. On a retained-object-graph workload that was 6,181,945 copies of exactly 152 bytes, and after this change it is 6. + The iterator now carries `ShapeRecordRef`, an 8-byte handle to the shape table's slab record (the iterator shrinks to 120 bytes). The Array/Closure arms build the iterator in place with `let-else`, and the masked arm iterates `&mut child_slots`. + Soundness: this is still ONE shape-table probe per receiver (#8122). The handle is read at the same points the lifted copy was (the carrier notes and the keys edge), before any slot is visited. It depends on the same record-address validity that `note_old_generation_carrier` already writes through (#9706: slab records never move, and a chunk is released only at the end of a major collection). The #8112 old-carrier/ephemeron gate and the #9726 full-trace note are unchanged. + Measured (instructions:u, exact counts): gc3 12.572G → 12.276G (−2.36%); smaller retained sets −0.78% to −1.97%; old→young churn −1.45%; allocation-only 0.00%. + The issue's profile had put memmove at 14% because `instructions:u` sampling skids on this CPU. Precise `cycles:pp` sampling shows 1.3%, which matches the measured win. Use a precise event for attribution and exact counters for totals. From 33b6c5ef788fc98773a0c85e64a0abfde4b68b4d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Wed, 16 Sep 2026 14:24:34 +0000 Subject: [PATCH 3/4] perf(gc): relocation moves only address-keyed layout records (#10362) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Base: 6c9e2a6a (PR #10371's head), which this stacks on. `layout_transfer` runs for every evacuated object on every copying minor, both old-generation evacuations and `js_array_grow`. All four callers copy the source header's `_reserved` into the destination first — the minor through `reserved_with_copied_survival_age`, which rewrites only the age bits — so every layout fact a header carries has already arrived: the layout state, `GC_LAYOUT_ALL_POINTERS`, the raw-f64 / holes flags, `GC_ARRAY_ELEMENT_SHAPE` and `GC_OBJ_TYPED_LAYOUT_INTACT`. The funnel re-derived that half anyway, per object: two header classifications, a rewrite of bits that were already equal, an out-of-line call per array and per object, the #7510 flag-and-filter gate evaluated twice, and — for every intact object — a ShapeId-keyed `SHAPE_LAYOUTS` probe whose answer a relocation cannot change. Measured with gdb `stepi` on the #10362 retained-graph fixture: 160 instructions per moved array, 245 per moved object, 518M in total (4.2% of the run), none of which reached a side-table record. Both per-object maps held one key (`PERRY_LAYOUT_DIAG`). The contract the callers always satisfied is now the funnel's stated contract, asserted in test and debug builds, and the funnel moves only what a header cannot carry: the element-shape record (#7480), the residual static-prototype owner registry (#9304), and the per-object `TYPED_LAYOUTS` / `LAYOUT_SLOT_MASKS` entries (#7510). Each is gated inline by the bit or latch that governs it, and the record moves themselves live in a `#[cold]` slow path. * `gc/layout/transfer.rs` (new): the funnel, its gates and the contract assertion. `gc/layout.rs` drops to 1919 lines, off the 2000-line cap. * `gc/layout_tables.rs`: `per_object_layouts_may_hold_either` answers the #7510 gate for both maps and both addresses in one hot-slot resolution. * `object/prototype_chain.rs`: the registry latch is readable without the call. * `array/header.rs`, `array/element_shape.rs`: `transfer_array_numeric_layout` and `clear_element_shape_ptr` were only ever called by the header half of the funnel and are deleted. That also drops a spurious `invalidate_representation_change` a hole-tolerant array took on every move (the transfer cleared the flag it was about to set again); the counter feeds `PERRY_TYPED_FEEDBACK_TRACE` output only. Behaviour: one change, the lazy intact downgrade. The bit is a fact of the object and of two tables a move does not touch, so re-asking at move time could only downgrade objects that happen to move. The state it cleared — intact while no descriptor is reachable — is legal and handled: `shape_install_shared` poisons a shape's shared entry and leaves "any still-INTACT siblings" to fall back, #8115 clears the bit at the first contradicting store, the trace falls back to `GC_LAYOUT_UNKNOWN` and scans every slot, and the query helpers answer "no descriptor". An unmoved sibling keeps its bit today, so nothing could have depended on the move clearing it. `test_poisoned_shape_intact_and_per_object_record_survive_a_copying_minor` builds that exact state and drives it through a real copying minor: the moved receiver keeps the bit, answers every query as it did before the move, and its child survives and is rewritten, while the sibling that poisoned the shape keeps its per-object record across the move. It fails on the parent commit (the old funnel clears the bit) and fails again under a sabotaged funnel that does not move the per-object records. `test_layout_transfer_requires_the_relocation_header_copy` pins the contract. instructions:u, min of 5, base 6c9e2a6a vs this: gc3 12,275,577,365 -> 11,880,548,143 -3.22% w1000 1,081,647,372 -> 1,054,559,901 -2.50% w5000 2,018,014,139 -> 1,919,532,121 -4.88% w20000 4,994,632,943 -> 4,797,474,484 -3.95% oldyoung 1,503,368,405 -> 1,471,395,073 -2.13% alloc-only 320,266,214 -> 320,265,756 0.00% That is 154 instructions per relocation on gc3 (2,560,042 relocations on both arms, counted with uprobes), and 71% of the ceiling a full knockout of the funnel measured. The cold path is entered 1,213 times, 0.05% of relocations. --- .../perry-runtime/src/array/element_shape.rs | 12 +- crates/perry-runtime/src/array/header.rs | 19 -- crates/perry-runtime/src/array/mod.rs | 8 +- crates/perry-runtime/src/gc/layout.rs | 99 +--------- .../perry-runtime/src/gc/layout/transfer.rs | 171 ++++++++++++++++ crates/perry-runtime/src/gc/layout_tables.rs | 11 ++ .../src/gc/tests/layout_trace/array_layout.rs | 1 + .../src/gc/tests/layout_trace/typed_shape.rs | 182 ++++++++++++++++++ crates/perry-runtime/src/gc/tests/support.rs | 11 ++ .../src/object/prototype_chain.rs | 11 ++ 10 files changed, 401 insertions(+), 124 deletions(-) create mode 100644 crates/perry-runtime/src/gc/layout/transfer.rs diff --git a/crates/perry-runtime/src/array/element_shape.rs b/crates/perry-runtime/src/array/element_shape.rs index 4da435ce39..76358d587e 100644 --- a/crates/perry-runtime/src/array/element_shape.rs +++ b/crates/perry-runtime/src/array/element_shape.rs @@ -32,7 +32,7 @@ //! | fast proof | `_reserved` bit 7 | `_reserved` bit 11 | //! | rides a move | yes (`_reserved` is copied) | yes (same word) | //! | self-heals by rescan | `ensure_array_numeric_raw_f64` | [`ensure_element_shape`] | -//! | move fixup | `transfer_array_numeric_layout` | [`transfer_element_shape`] | +//! | move fixup | none — the bit IS the record | [`transfer_element_shape`] | //! | clear funnel | `clear_array_numeric_layout` | [`clear_element_shape`] | //! //! The one thing 4a does not need is a *payload*: "raw f64" is the whole @@ -443,16 +443,6 @@ pub(crate) unsafe fn clear_element_shape(arr: *const ArrayHeader) { bump_epoch(); } -/// Address-keyed sibling of [`clear_element_shape`], for the `layout_*` -/// family and other callers that hold a `usize`. -#[inline] -pub(crate) fn clear_element_shape_ptr(user_ptr: usize) { - if user_ptr == 0 { - return; - } - unsafe { clear_element_shape(user_ptr as *const ArrayHeader) } -} - /// Forget everything about an address, bit included. Used when an allocation /// dies and its address may be recycled (`layout_clear_for_ptr`). pub(crate) fn forget_element_shape(user_ptr: usize) { diff --git a/crates/perry-runtime/src/array/header.rs b/crates/perry-runtime/src/array/header.rs index 5c4ca69e2d..4d696d33fb 100644 --- a/crates/perry-runtime/src/array/header.rs +++ b/crates/perry-runtime/src/array/header.rs @@ -1307,25 +1307,6 @@ pub(crate) fn clear_array_numeric_layout_ptr(user_ptr: usize) { } } -#[inline] -pub(crate) fn transfer_array_numeric_layout(old_user: usize, new_user: usize) { - if old_user == 0 || new_user == 0 || old_user == new_user { - return; - } - unsafe { - if array_has_raw_f64_layout_flag(old_user as *const ArrayHeader) { - set_array_raw_f64_layout_flag(new_user as *const ArrayHeader); - } else if array_has_raw_f64_holes_flag(old_user as *const ArrayHeader) { - // #6011: relocation copies slot bits verbatim, so the verified - // raw-f64-or-holes invariant carries over to the new backing. - clear_array_raw_f64_layout_flag(new_user as *const ArrayHeader); - set_array_raw_f64_holes_flag(new_user as *const ArrayHeader); - } else { - clear_array_raw_f64_layout_flag(new_user as *const ArrayHeader); - } - } -} - #[inline] pub(crate) unsafe fn array_numeric_layout(arr: *const ArrayHeader) -> Option { let arr = clean_arr_ptr(arr); diff --git a/crates/perry-runtime/src/array/mod.rs b/crates/perry-runtime/src/array/mod.rs index 6141572233..c6c0f1dcaf 100644 --- a/crates/perry-runtime/src/array/mod.rs +++ b/crates/perry-runtime/src/array/mod.rs @@ -101,8 +101,8 @@ pub use self::concat_reverse::{ js_array_fill_range, js_array_reverse, js_array_reverse_value, }; pub(crate) use self::element_shape::{ - clear_element_shape_ptr, forget_element_shape, invalidate_all_element_shapes, - note_element_store, prune_dead_element_shape_owners, transfer_element_shape, + forget_element_shape, invalidate_all_element_shapes, note_element_store, + prune_dead_element_shape_owners, transfer_element_shape, }; pub use self::element_shape::{ js_array_element_shape_check, js_array_element_shape_class, js_array_element_shape_epoch, @@ -290,8 +290,8 @@ pub(crate) use self::header::{ normalize_array_receiver, note_array_slot, note_array_slot_layout_only, note_array_slot_resolved_flags, rebuild_array_layout, rebuild_array_layout_exact, refresh_array_numeric_layout, replay_array_growth_write_barriers, set_array_numeric_layout, - store_array_slot, store_array_slot_resolved, transfer_array_numeric_layout, - typed_array_receiver, value_bits_to_number, NumericArrayLayout, MIN_ARRAY_CAPACITY, + store_array_slot, store_array_slot_resolved, typed_array_receiver, value_bits_to_number, + NumericArrayLayout, MIN_ARRAY_CAPACITY, }; pub(crate) use self::named_props::{ array_has_named_properties_resolved, array_has_sparse_index_properties_resolved, diff --git a/crates/perry-runtime/src/gc/layout.rs b/crates/perry-runtime/src/gc/layout.rs index 72a5a7c0d2..cb5e79282a 100644 --- a/crates/perry-runtime/src/gc/layout.rs +++ b/crates/perry-runtime/src/gc/layout.rs @@ -1,6 +1,7 @@ -//! Per-object pointer-slot states, store maintenance, copying-GC transfer and -//! child-slot enumeration. Mask storage is in `layout/slot_mask.rs`; typed -//! descriptor installation is in `layout/typed_shape.rs`. +//! Per-object pointer-slot states, store maintenance and child-slot +//! enumeration. Mask storage is in `layout/slot_mask.rs`; typed descriptor +//! installation is in `layout/typed_shape.rs`; the relocation funnel every +//! moving-GC and growth path calls is in `layout/transfer.rs`. use super::hot_tls::{hot_layout_slot_masks, hot_shape_layouts}; use super::layout_tables::{ @@ -82,8 +83,9 @@ pub(crate) const GC_LAYOUT_ALL_POINTERS: u16 = 0x2000; // "slot K is raw-f64" from this single bit — no cross-crate guard call, no // thread-local hashmap probe — for any field K the class declares as a raw-f64 // candidate. The bit travels with `_reserved` across copying/evacuating GC (the -// collector copies the whole reserved word), and `layout_transfer` re-syncs it -// defensively after moving the descriptor. +// collector copies the whole reserved word), which is all a relocation owes it: +// `layout/transfer.rs` re-sets it only for an object whose per-object +// descriptor moved, and never re-derives it (#10362). pub const GC_OBJ_TYPED_LAYOUT_INTACT: u16 = 0x1000; #[inline] @@ -111,9 +113,11 @@ pub(super) fn clear_typed_layout_intact_for_user(user_ptr: usize) { } mod slot_mask; +mod transfer; mod typed_shape; pub(in crate::gc) use slot_mask::LayoutSlotMask; +pub(crate) use transfer::layout_transfer; pub use typed_shape::{ js_gc_declare_typed_shape_layout, js_gc_init_typed_shape_layout, js_gc_typed_shape_id_for_keys, }; @@ -1254,91 +1258,6 @@ pub(crate) unsafe fn layout_rebuild_exact_from_slots( layout_rebuild_from_slots_with_policy(user_ptr, slots, slot_count, true); } -pub(crate) unsafe fn layout_transfer(old_user: *mut u8, new_user: *mut u8) { - if old_user.is_null() || new_user.is_null() || old_user == new_user { - return; - } - let Some(old_header) = layout_header_for_user(old_user as usize) else { - return; - }; - let Some(new_header) = layout_header_for_user(new_user as usize) else { - return; - }; - let state = (*old_header)._reserved & GC_LAYOUT_STATE_MASK; - let all_pointers = (*old_header)._reserved & GC_LAYOUT_ALL_POINTERS != 0; - set_layout_state(new_header, state); - if all_pointers { - (*new_header)._reserved |= GC_LAYOUT_ALL_POINTERS; - } - if (*old_header).obj_type == GC_TYPE_ARRAY && (*new_header).obj_type == GC_TYPE_ARRAY { - crate::array::transfer_array_numeric_layout(old_user as usize, new_user as usize); - // #7480: the element-shape bit rides `_reserved` for free, but its - // record is address-keyed and has to follow the move — same split, - // and same call site, as `TYPED_LAYOUTS` below. - crate::array::transfer_element_shape(old_user as usize, new_user as usize); - // #9304: real arrays keep explicit [[Prototype]] values in the - // residual address-keyed registry. Array growth and moving GC both - // replace the owner allocation through this transfer hook. - crate::object::prototype_chain::object_static_prototype_owner_moved( - old_user as usize, - new_user as usize, - ); - } else { - crate::array::clear_array_numeric_layout_ptr(new_user as usize); - crate::array::clear_element_shape_ptr(new_user as usize); - } - // Read the source object's intact bit BEFORE the transfer clears it — it is - // the per-object half of the shape-keyed resolution below. `_reserved` is - // untouched by `set_forwarding_address` (which writes gc_flags and the first - // payload word), so it is still authoritative here even though the - // evacuation callers forward the original before calling us. - let old_intact = (*old_header)._reserved & GC_OBJ_TYPED_LAYOUT_INTACT != 0; - // #7510: with both per-object maps provably empty there is nothing to - // move, and every relocated object would otherwise pay two `RefCell` - // round-trips plus two hashes during evacuation. The shape-keyed half - // below is unaffected — it needs no move at all. - let new_has_typed = transfer_per_object_descriptor(old_user as usize, new_user as usize); - // #6964: the canonical descriptor may live in EITHER map, exactly as the - // query helpers resolve it (#6957/#6963). The per-object `TYPED_LAYOUTS` - // entry is keyed by ADDRESS, so it has to be moved (above). The shape-keyed - // `SHAPE_LAYOUTS` entry (#6893/#8289) is keyed by immutable runtime - // ShapeId, which the relocated copy carries verbatim — it needs no move, - // but it only describes THIS object while the object is still INTACT. - // - // Probing only `TYPED_LAYOUTS` missed for every object #6893 actually moved - // (i.e. every class instance: it carries a keys_array and therefore has NO - // per-object entry), so `new_has_typed` was false and the relocated copy had - // a still-valid intact bit CLEARED — permanently deopting its typed guards. - // Latent until an evacuating minor became reachable (#6950); the fourth - // caller, array growth in `array/push_pop.rs`, is `GC_TYPE_ARRAY`, which is - // not `GcLayoutSlotKind::ObjectFields` and so never had a shape-keyed - // descriptor to lose. - // - // Read the shape through `new_user`: the evacuation callers install the - // forwarding pointer over the ORIGINAL's first payload word, which for an - // ObjectFields object overlaps the header fields this lookup reads. - // - // Mirrors #6963's split: the per-object half stays ungated (so a forged or - // stale intact bit cannot manufacture a descriptor), the shared half is - // gated on the source object's intact bit (so an object that diverged from - // its shape does not silently re-adopt the shape's stale descriptor by - // moving). - let new_has_shape_typed = !new_has_typed - && old_intact - && with_shape_shared_descriptor(new_user as usize, |_| ()).is_some(); - // Keep the intact bit in lock-step with the moved descriptor. Copying GC - // normally propagates `_reserved` (so the bit already rode along), but - // re-sync defensively for callers that allocate the destination fresh - // (e.g. array growth) so a stale/missing bit can never desync from the map. - if new_has_typed || new_has_shape_typed { - header_set_typed_layout_intact(new_header); - } else { - header_clear_typed_layout_intact(new_header); - } - header_clear_typed_layout_intact(old_header); - transfer_per_object_slot_mask(old_user as usize, new_user as usize); -} - pub(super) fn layout_visit_pointer_slots( user_ptr: usize, slot_count: usize, diff --git a/crates/perry-runtime/src/gc/layout/transfer.rs b/crates/perry-runtime/src/gc/layout/transfer.rs new file mode 100644 index 0000000000..b99cce0cf2 --- /dev/null +++ b/crates/perry-runtime/src/gc/layout/transfer.rs @@ -0,0 +1,171 @@ +//! The relocation funnel: what follows an object when its storage moves. +//! +//! Split out of `gc/layout.rs` (#10362), which sits on the repo's 2000-line +//! cap, and narrowed to the contract its callers have always satisfied. +//! +//! # The contract +//! +//! Four paths replace an object's storage: the copying nursery's `move_young`, +//! the two old-generation evacuations in `gc/oldgen.rs`, and `js_array_grow`. +//! **Every one of them copies the source header's `_reserved` into the +//! destination before calling** — the copying minor through +//! `reserved_with_copied_survival_age`, which rewrites only the age bits. +//! +//! So every layout fact the header carries has already arrived at the +//! destination by construction: the layout state, `GC_LAYOUT_ALL_POINTERS`, +//! the raw-f64 / holes flags, `GC_ARRAY_ELEMENT_SHAPE` and +//! `GC_OBJ_TYPED_LAYOUT_INTACT`. What cannot ride a header is a record keyed +//! by the object's ADDRESS, and that is all this funnel moves: +//! +//! * the element-shape proof record (#7480), gated by the header bit that is +//! authoritative for it; +//! * the residual static-prototype owner registry (#9304), gated by its +//! process-global latch (#7733/#7737); +//! * the per-object `TYPED_LAYOUTS` and `LAYOUT_SLOT_MASKS` entries, gated by +//! #7510's emptiness flag and address filter. +//! +//! Until #10362 the funnel re-derived the header half too — rewriting bits +//! that were already equal, and re-resolving the intact bit through a +//! ShapeId-keyed `SHAPE_LAYOUTS` probe — once per relocated object. Measured +//! on #10362's retained-graph workload that was 160 instructions per moved +//! array and 245 per moved object, 518M instructions (4.2% of the run), of +//! which zero reached a side-table record: both per-object maps held one key. +//! The gates below answer the same questions from the header word and two +//! flags the caller has already brought into cache. +//! +//! # Why the intact bit is not re-derived +//! +//! `GC_OBJ_TYPED_LAYOUT_INTACT` asks whether a canonical typed descriptor is +//! reachable for this object. Its inputs are the receiver's stamped ShapeId +//! (copied verbatim with the payload), `SHAPE_LAYOUTS`, the process-global +//! registered typed-shape registry (#8405) and the per-object map — and a +//! relocation changes none of them. Re-asking at move time could therefore +//! only apply a LAZY downgrade, and only to the objects that happen to move. +//! +//! The state that downgrade cleared — intact while no descriptor is reachable +//! — is legal and handled. `shape_install_shared` poisons a shape's shared +//! entry to `None` and deliberately leaves "any still-INTACT siblings" to fall +//! back; #8115 clears the bit at the first contradicting store; the trace path +//! resolves no mask, sets `GC_LAYOUT_UNKNOWN` and scans every slot; the query +//! helpers answer "no descriptor". An unmoved sibling in exactly that state +//! keeps its bit today, so an argument that needed the move to clear it would +//! already be broken for every object that does not move. +//! `gc/tests/layout_trace/typed_shape.rs` pins the pair across a real copying +//! minor: the moved object and its unmoved peer must answer identically, and +//! the child behind the poisoned shape must survive the cycle. + +use super::*; +use crate::gc::layout_tables::per_object_layouts_may_hold_either; + +/// Move the address-keyed layout records of a relocated object. +/// +/// # Safety +/// +/// `old_user` and `new_user` are user pointers of live allocations, and the +/// caller has already made the destination header a copy of the source's (see +/// the module docs). The precondition is asserted in test and debug builds. +#[inline] +pub(crate) unsafe fn layout_transfer(old_user: *mut u8, new_user: *mut u8) { + if old_user.is_null() || new_user.is_null() || old_user == new_user { + return; + } + // Kinds with no layout metadata at all (strings, meta records, RegExps) + // leave before anything else, exactly as before #10362. The destination + // carries the same `obj_type`, so one classification answers for both. + let Some(old_header) = layout_header_for_user(old_user as usize) else { + return; + }; + assert_relocation_copied_the_header(old_header, new_user); + + let reserved = (*old_header)._reserved; + let is_array = (*old_header).obj_type == GC_TYPE_ARRAY; + // Three gates, all answered from words already in registers or in the one + // hot thread-local slot #7510 keeps them in. Each is the same question the + // record mover behind it asks first, hoisted so the common case — no + // record anywhere near either address — never leaves this function. + let per_object = per_object_layouts_may_hold_either(old_user as usize, new_user as usize); + let element_shape = is_array && reserved & GC_ARRAY_ELEMENT_SHAPE != 0; + let static_prototype = + is_array && crate::object::prototype_chain::object_static_prototypes_maybe_nonempty(); + if per_object || element_shape || static_prototype { + transfer_address_keyed_records( + old_user as usize, + new_user as usize, + header_from_user_ptr(new_user as *const u8), + is_array, + ); + } + + // The source is a dead evacuation original or a growth forwarding stub the + // moment we return. Drop its claim to a descriptor rather than leave the + // bit readable at an address whose records now belong to the destination. + header_clear_typed_layout_intact(old_header); +} + +/// The record moves themselves. Cold: on a workload holding no per-object +/// layout record, no element-shape proof and no re-prototyped array — the +/// steady state of every monomorphic program — it is never reached. +#[cold] +#[inline(never)] +unsafe fn transfer_address_keyed_records( + old_user: usize, + new_user: usize, + new_header: *mut GcHeader, + is_array: bool, +) { + if is_array { + // #7480: the proof record is keyed by the array's address while the + // header bit is what a read consults. `transfer_element_shape` decides + // from both headers and fails closed — it clears the destination bit + // when no record follows the move. + crate::array::transfer_element_shape(old_user, new_user); + // #9304: a real array keeps an explicit [[Prototype]] in the residual + // address-keyed registry; moving GC and growth both replace the owner + // allocation through this hook. + crate::object::prototype_chain::object_static_prototype_owner_moved(old_user, new_user); + } + // #7510's two per-object maps. Both re-test the gate above for their own + // address pair, so calling them when only a sibling gate fired costs one + // predictable branch each. + // + // Re-setting the intact bit for a moved per-object descriptor is parity + // with the pre-#10362 funnel rather than a fact the copy lost: a source + // whose descriptor existed while its own bit was clear had the bit SET by + // the move. Keeping that leaves the lazy downgrade (module docs) as the + // single behavioural difference of #10362. + if transfer_per_object_descriptor(old_user, new_user) { + header_set_typed_layout_intact(new_header); + } + transfer_per_object_slot_mask(old_user, new_user); +} + +/// The funnel's precondition: the destination header is the source's copy. +/// +/// Checked in test and debug builds — including `cargo test --release`, which +/// is how the GC suites run — so a future relocation path that allocates a +/// destination without copying `_reserved` fails loudly here instead of +/// silently losing a layout state, an `ALL_POINTERS` bit or an element-shape +/// proof at the first collection. +#[inline] +unsafe fn assert_relocation_copied_the_header(old_header: *mut GcHeader, new_user: *mut u8) { + #[cfg(any(test, debug_assertions))] + { + let new_header = header_from_user_ptr(new_user as *const u8); + assert_eq!( + (*new_header).obj_type, + (*old_header).obj_type, + "layout_transfer: a relocation must not change the object type" + ); + assert_eq!( + (*new_header)._reserved & !GC_COPY_SURVIVAL_AGE_MASK, + (*old_header)._reserved & !GC_COPY_SURVIVAL_AGE_MASK, + "layout_transfer: the caller must copy `_reserved` into the destination before \ + relocating (only the copied-survival age may differ) — every header-carried \ + layout fact rides that copy" + ); + } + #[cfg(not(any(test, debug_assertions)))] + { + let _ = (old_header, new_user); + } +} diff --git a/crates/perry-runtime/src/gc/layout_tables.rs b/crates/perry-runtime/src/gc/layout_tables.rs index 76a9f298b0..dc006c52c8 100644 --- a/crates/perry-runtime/src/gc/layout_tables.rs +++ b/crates/perry-runtime/src/gc/layout_tables.rs @@ -1037,6 +1037,17 @@ pub(in crate::gc) fn per_object_layouts_maybe_nonempty() -> bool { hot_per_object_layout_hint().nonempty.get() } +/// Can EITHER address carry a per-object record? The relocation funnel +/// (`gc/layout/transfer.rs`) asks once for both maps and both ends of a move, +/// where the two `transfer_*` entry points below each resolve the hot slot +/// again for their own pair (#10362). Same answer, one thread-local +/// resolution: the flag and the filter live in the same slot. +#[inline] +pub(in crate::gc) fn per_object_layouts_may_hold_either(old_user: usize, new_user: usize) -> bool { + let hint = hot_per_object_layout_hint(); + hint.nonempty.get() && (hint_may_hold(hint, old_user) || hint_may_hold(hint, new_user)) +} + /// Arm the flag. Called by anything that inserts into either map — including /// the one insert site that holds its own `borrow_mut` and so cannot go /// through the wrappers below. diff --git a/crates/perry-runtime/src/gc/tests/layout_trace/array_layout.rs b/crates/perry-runtime/src/gc/tests/layout_trace/array_layout.rs index 1d6009797f..9ea1587006 100644 --- a/crates/perry-runtime/src/gc/tests/layout_trace/array_layout.rs +++ b/crates/perry-runtime/src/gc/tests/layout_trace/array_layout.rs @@ -66,6 +66,7 @@ fn test_layout_mask_overflow_fields_and_array_grow_transfer() { let moved = crate::array::js_array_alloc_with_length(4); unsafe { + model_relocation_header_copy(grown as usize, moved as usize); layout_transfer(grown as *mut u8, moved as *mut u8); } assert_eq!(test_layout_pointer_slot_count(moved as usize, 4), Some(1)); diff --git a/crates/perry-runtime/src/gc/tests/layout_trace/typed_shape.rs b/crates/perry-runtime/src/gc/tests/layout_trace/typed_shape.rs index c63e2e193b..10fb7b0040 100644 --- a/crates/perry-runtime/src/gc/tests/layout_trace/typed_shape.rs +++ b/crates/perry-runtime/src/gc/tests/layout_trace/typed_shape.rs @@ -352,6 +352,184 @@ fn test_shape_keyed_typed_layout_survives_copying_minor() { assert!(layout_slot_is_raw_f64_typed(after, 0)); } +/// #10362: a relocation carries `GC_OBJ_TYPED_LAYOUT_INTACT` in the `_reserved` +/// copy and no longer re-derives it. This pins the exact state that the old +/// re-derivation used to clear — an INTACT receiver whose shape's SHARED +/// descriptor has since been poisoned to `None` — across a real copying minor, +/// together with the address-keyed half the funnel must still move. +/// +/// The two receivers are one fixture because they are one cycle: +/// +/// * `poisoned` installs the shared descriptor first and keeps its bit. An +/// unmoved sibling in this state keeps its bit too (`shape_install_shared` +/// leaves "any still-INTACT siblings" to fall back), so clearing it on the +/// copy alone was never a correctness rule. What the collector owes the +/// object is that its pointer field survive: with no descriptor resolvable +/// the trace falls back to scanning every slot. +/// * `per_object` is the receiver whose different layout POISONED the shape, +/// so its canonical descriptor is a per-object record — the address-keyed +/// half `transfer_address_keyed_records` still has to move. Its pointer +/// mask must answer the same after the move; a funnel that skips the record +/// move fails exactly that assertion. +#[test] +fn test_poisoned_shape_intact_and_per_object_record_survive_a_copying_minor() { + // Two rooted receivers, so two shadow slots: a slot index outside the + // pushed frame is bounds-checked into a silent no-op (#7184), which would + // leave the second receiver unrooted and every verdict below vacuous. + let _guard = CopyingNurseryTestGuard::new(2); + let _trigger_guard = GcTriggerThresholdTestGuard::suppress_automatic_triggers(); + + let packed = b"x\0y\0"; + let keys = crate::object::js_build_class_keys_array( + 0x1036_20, + 2, + packed.as_ptr(), + packed.len() as u32, + ); + + // Slot 0 raw-f64, slot 1 a declared pointer: the shape's first descriptor. + let poisoned = crate::object::js_object_alloc_class_inline_keys(0x1036_20, 0, 2, keys); + let poisoned_child = crate::string::js_string_from_bytes(b"poisoned-child".as_ptr(), 14); + crate::object::js_object_set_field(poisoned, 0, crate::value::JSValue::number(1.5)); + crate::object::js_object_set_field( + poisoned, + 1, + crate::value::JSValue::string_ptr(poisoned_child), + ); + let raw_mask = [0b01u64]; + let pointer_mask = [0b10u64]; + js_gc_init_typed_shape_layout( + poisoned as u64, + 2, + raw_mask.as_ptr(), + raw_mask.len() as u32, + pointer_mask.as_ptr(), + pointer_mask.len() as u32, + ); + assert!(layout_typed_intact_for_user(poisoned as usize)); + assert!(layout_typed_raw_f64_slot_for_user(poisoned as usize, 0)); + + // Same keys, DIFFERENT layout (slot 0 carries no raw-f64 proof): the + // install poisons the shared entry and falls back to a per-object record. + let per_object = crate::object::js_object_alloc_class_inline_keys(0x1036_20, 0, 2, keys); + let per_object_child = crate::string::js_string_from_bytes(b"per-object-child".as_ptr(), 16); + crate::object::js_object_set_field(per_object, 0, crate::value::JSValue::number(2.5)); + crate::object::js_object_set_field( + per_object, + 1, + crate::value::JSValue::string_ptr(per_object_child), + ); + js_gc_init_typed_shape_layout( + per_object as u64, + 2, + std::ptr::null(), + 0, + pointer_mask.as_ptr(), + pointer_mask.len() as u32, + ); + + // The fixture must START in the state under test, or every verdict below + // is vacuous: the shared descriptor is gone for the first receiver while + // its intact bit stands, and the second receiver answers from a record. + assert!( + layout_typed_intact_for_user(poisoned as usize), + "the poisoning install must not clear a sibling's intact bit" + ); + assert!( + !layout_typed_raw_f64_slot_for_user(poisoned as usize, 0), + "fixture precondition: the shared descriptor must be poisoned, so no \ + descriptor is resolvable for the first receiver" + ); + assert_eq!( + test_layout_pointer_slot_count(poisoned as usize, 2), + None, + "fixture precondition: an unresolvable descriptor means the conservative scan" + ); + assert_eq!( + test_layout_pointer_slot_count(per_object as usize, 2), + Some(1), + "fixture precondition: the second receiver's layout is a per-object record" + ); + + js_shadow_slot_set(0, ptr_bits(poisoned as usize)); + js_shadow_slot_set(1, ptr_bits(per_object as usize)); + + let trace = collect_minor_trace(GcTriggerKind::Direct); + assert_copied_minor_trace(&trace, true, CopiedMinorFallbackReason::None, false); + + let poisoned_after = (js_shadow_slot_get(0) & POINTER_MASK) as usize; + let per_object_after = (js_shadow_slot_get(1) & POINTER_MASK) as usize; + assert_ne!( + poisoned_after, poisoned as usize, + "the minor must actually relocate the first receiver — an inert arm proves nothing" + ); + assert_ne!( + per_object_after, per_object as usize, + "the minor must actually relocate the second receiver" + ); + + // The bit rides `_reserved`. Before #10362 the funnel re-probed + // `SHAPE_LAYOUTS` here, found the poisoned `None`, and cleared it on the + // copy — a downgrade no unmoved sibling ever received. + assert!( + layout_typed_intact_for_user(poisoned_after), + "the relocated receiver keeps the intact bit its `_reserved` copy carried" + ); + assert!( + !layout_typed_raw_f64_slot_for_user(poisoned_after, 0), + "and still resolves no descriptor, exactly as before the move" + ); + assert_eq!( + test_layout_pointer_slot_count(poisoned_after, 2), + None, + "so the trace still falls back to scanning every slot" + ); + + // What the collector owes it: the field behind the unresolvable descriptor + // is marked and rewritten. + let moved_child = + crate::object::js_object_get_field(poisoned_after as *const crate::object::ObjectHeader, 1); + assert!(moved_child.is_string()); + let moved_child_ptr = moved_child.as_string_ptr(); + assert_ne!( + moved_child_ptr as usize, poisoned_child as usize, + "the child moved too, so the slot proves the rewrite, not just the mark" + ); + unsafe { + assert_string_bytes(moved_child_ptr, b"poisoned-child"); + } + + // The address-keyed half: the per-object record followed the move. + assert_eq!( + test_layout_pointer_slot_count(per_object_after, 2), + Some(1), + "the per-object layout record must be keyed by the post-move address" + ); + assert!(layout_typed_intact_for_user(per_object_after)); + let moved_per_object_child = crate::object::js_object_get_field( + per_object_after as *const crate::object::ObjectHeader, + 1, + ); + assert!(moved_per_object_child.is_string()); + unsafe { + assert_string_bytes(moved_per_object_child.as_string_ptr(), b"per-object-child"); + } +} + +/// #10362: the relocation contract is asserted in test and debug builds, so a +/// future move path that allocates a destination without copying `_reserved` +/// fails here instead of silently losing a layout state, an `ALL_POINTERS` bit +/// or an element-shape proof at the first collection. +#[test] +#[should_panic(expected = "the caller must copy `_reserved`")] +fn test_layout_transfer_requires_the_relocation_header_copy() { + let src = crate::array::js_array_alloc_pointer_elements(2); + let dst = crate::array::js_array_alloc(2); + unsafe { + layout_transfer(src as *mut u8, dst as *mut u8); + } +} + #[test] fn test_typed_shape_raw_numeric_slots_accept_pointer_like_f64_bits() { clear_marks(); @@ -488,6 +666,7 @@ fn test_typed_shape_descriptor_transfers_on_object_move() { ); unsafe { + model_relocation_header_copy(src as usize, dst as usize); layout_transfer(src as *mut u8, dst as *mut u8); } @@ -511,6 +690,9 @@ fn test_all_pointer_layout_transfers_on_array_move() { let src = crate::array::js_array_alloc_pointer_elements(2); let dst = crate::array::js_array_alloc(2); unsafe { + // `GC_LAYOUT_ALL_POINTERS` rides `_reserved`, so since #10362 the + // header copy is what carries it and the funnel must leave it alone. + model_relocation_header_copy(src as usize, dst as usize); layout_transfer(src as *mut u8, dst as *mut u8); } diff --git a/crates/perry-runtime/src/gc/tests/support.rs b/crates/perry-runtime/src/gc/tests/support.rs index ec6e4409ce..bfd3a3ddd1 100644 --- a/crates/perry-runtime/src/gc/tests/support.rs +++ b/crates/perry-runtime/src/gc/tests/support.rs @@ -24,6 +24,17 @@ impl Drop for ShadowAndGlobalRootResetGuard { } } +/// Model what every relocation caller does before `layout_transfer`: make the +/// destination header a copy of the source's, which is the funnel's contract +/// (#10362, `gc/layout/transfer.rs`). A test that calls the funnel directly +/// has to do it for the same reason `move_young` and `js_array_grow` do — +/// every header-carried layout fact rides this word. +pub(super) unsafe fn model_relocation_header_copy(src_user: usize, dst_user: usize) { + let src = header_from_user_ptr(src_user as *const u8); + let dst = header_from_user_ptr(dst_user as *const u8); + (*dst)._reserved = (*src)._reserved; +} + pub(super) unsafe fn test_heap_child_slots_for_user(user_ptr: *mut u8) -> Vec { let header = header_from_user_ptr(user_ptr as *const u8); gc_child_slots(header).collect() diff --git a/crates/perry-runtime/src/object/prototype_chain.rs b/crates/perry-runtime/src/object/prototype_chain.rs index d5469546d4..ec6a23189e 100644 --- a/crates/perry-runtime/src/object/prototype_chain.rs +++ b/crates/perry-runtime/src/object/prototype_chain.rs @@ -469,6 +469,17 @@ pub(crate) fn prune_dead_object_prototype_owners(is_dead_owner: &dyn Fn(usize) - } } +/// Can the residual owner registry hold an entry at all? +/// +/// The latch is stored (`Release`) before the first insert, so `false` proves +/// the registry empty — the same proof [`object_static_prototype_owner_moved`] +/// makes on entry, exposed so the relocation funnel +/// (`gc/layout/transfer.rs`) can decide without the call (#10362). +#[inline] +pub(crate) fn object_static_prototypes_maybe_nonempty() -> bool { + OBJECT_PROTOTYPES_NONEMPTY.load(Ordering::Acquire) +} + /// Migrate the residual side-table entry when an owner's allocation address /// changes, either through moving GC or an `ArrayHeader` growth replacement. /// Mirrors `closure_dynamic_props_owner_moved`. From af304be5492f752e6c1769a63f70866f16cf2333 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Wed, 16 Sep 2026 17:22:56 +0200 Subject: [PATCH 4/4] changelog: fragment for #10381 --- ...381-gc-relocation-address-keyed-records.md | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 changelog.d/10381-gc-relocation-address-keyed-records.md diff --git a/changelog.d/10381-gc-relocation-address-keyed-records.md b/changelog.d/10381-gc-relocation-address-keyed-records.md new file mode 100644 index 0000000000..ae309e305e --- /dev/null +++ b/changelog.d/10381-gc-relocation-address-keyed-records.md @@ -0,0 +1,28 @@ +### Performance + +### Performance + +- **A moving collection no longer re-derives an object's layout header (#10362).** `layout_transfer` + runs for every evacuated object on every copying minor, both old-generation evacuations and + `js_array_grow`. All four callers copy the source header's `_reserved` into the destination + first, so the layout state, `GC_LAYOUT_ALL_POINTERS`, the raw-f64 / holes flags, the + element-shape bit and `GC_OBJ_TYPED_LAYOUT_INTACT` have already arrived — yet the funnel rewrote + those bits anyway, classified both headers, evaluated #7510's flag-and-filter gate twice, and + for every intact object re-resolved the intact bit through a ShapeId-keyed `SHAPE_LAYOUTS` + probe whose answer a relocation cannot change. Measured by single-stepping the #10362 + retained-graph fixture: 160 instructions per moved array and 245 per moved object, 518M in all + (4.2% of the run), none of which reached a side-table record. + That contract is now stated and asserted, and the funnel moves only what a header cannot carry: + the element-shape record (#7480), the residual static-prototype registry (#9304) and the + per-object `TYPED_LAYOUTS` / `LAYOUT_SLOT_MASKS` entries (#7510) — each behind the bit or latch + that governs it, with the record moves in a `#[cold]` path entered on 0.05% of relocations. + The one behavioural change is that the lazy intact downgrade is gone: the bit is a fact of the + object and of tables a move does not touch, the state it cleared is legal and handled + (`shape_install_shared` leaves still-INTACT siblings to fall back, #8115 clears at the first + contradicting store, the trace falls back to scanning every slot), and an unmoved sibling keeps + its bit today. A new test builds a poisoned-shape intact receiver, moves it through a real + copying minor, and checks the bit, every query answer and the survival and rewrite of its child; + it fails on the parent commit and under a funnel that skips the per-object record move. + instructions:u, min of 5: gc3 12.276G -> 11.881G (-3.22%), retained-set variants -2.50% to + -4.88%, old->young churn -2.13%, allocation-only unchanged. 2,560,042 relocations on both arms. +