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/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. + 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 f5fbd30a8b..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, }; @@ -203,22 +207,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 +232,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 +322,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 @@ -1257,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, @@ -1525,12 +1441,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 +1477,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 +1485,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 +1634,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 +1707,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 +1717,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 +1740,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 +1777,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/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_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/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/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/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`. 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)