perf(gc): borrow the shape record in the slot visitor, stop moving the iterator (#10362)

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.

crates/perry-runtime/src/gc/layout.rs

@@ -203,22 +203,22 @@ unsafe fn with_shape_shared_descriptor<R>(
     // 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<R>(
     user_ptr: usize,
-    descriptor: Option<crate::object::shapes::ShapeDescriptor>,
+    shape: Option<crate::object::shapes::ShapeRecordRef>,
     f: impl Fn(&TypedLayoutDescriptor) -> R,
 ) -> Option<R> {
     let object = user_ptr as *const crate::object::ObjectHeader;
@@ -228,10 +228,8 @@ unsafe fn with_shape_shared_descriptor_from<R>(
     }
     // 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<crate::object::shapes::ShapeDescriptor>,
+    shape: Option<crate::object::shapes::ShapeRecordRef>,
 ) -> Option<LayoutSlotMask> {
     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<crate::object::shapes::ShapeDescriptor>,
+    pub(super) object_shape: Option<crate::object::shapes::ShapeRecordRef>,
 }
 
 impl HeapChildSlotIterator {
@@ -1561,15 +1558,15 @@ 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.
     pub(super) fn new_object(
         header: *mut GcHeader,
         prefix_slot: Option<*mut u64>,
         payload: HeapSlotRange,
-        object_shape: Option<crate::object::shapes::ShapeDescriptor>,
+        object_shape: Option<crate::object::shapes::ShapeRecordRef>,
     ) -> 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<crate::object::shapes::ShapeDescriptor>,
+    shape: Option<crate::object::shapes::ShapeRecordRef>,
 ) -> 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(),
     }

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,

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<shapes::ShapeDescriptor>,
-) -> Option<*mut u64> {
+pub(crate) fn gc_shape_keys_edge_slot(record: Option<shapes::ShapeRecordRef>) -> 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<shapes::ShapeDescriptor>,
+    record: Option<shapes::ShapeRecordRef>,
 ) -> Option<crate::gc::HeapSlotRange> {
     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;