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. 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)