perf(gc): don't enumerate child slots for objects that have none (#10362)

changelog.d/10669-zero-slot-skip.md

@@ -0,0 +1 @@
+Skip child-slot enumeration for objects that cannot have child slots. The copying minor, the full mark and the remembered-set rebuild each paid ~206 instructions per zero-slot object — iterator construction, the descriptor body, a worklist push and a drain entry — only to discover there was nothing to visit. The full mark and the remembered-set rebuild now walk half as many objects; gc3 spends 7.1% fewer instructions and 4.4% less peak RSS.

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

@@ -608,7 +608,19 @@ impl CopyingNurseryCollector {
         (*header).gc_flags &= !GC_FLAG_MARKED;
         gc_type_after_payload_move((*header).obj_type, old_user as usize, new_user as usize);
 
-        self.worklist.push(new_header);
+        // #10362: an object that provably yields no child slot is marked and
+        // moved, but not QUEUED — the drain would build an iterator and find
+        // nothing. See `gc_object_yields_no_child_slots` for what "no child
+        // slot" has to mean for this to be sound; the copying minor needs no
+        // proxy term because it ignores `PointerFreeRange`.
+        //
+        // `moved_headers` below is NOT part of this and must keep EVERY
+        // survivor: `clear_marks` walks it, so a header missing from it carries
+        // GC_FLAG_MARKED past the end of the cycle and reads as live to the
+        // next full sweep. Only the worklist push is skipped.
+        if !gc_object_yields_no_child_slots(new_header) {
+            self.worklist.push(new_header);
+        }
         self.survival_push();
         if let Some(d) = self.survival.as_mut() {
             d.record((*new_header).obj_type, total, promote);

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

@@ -481,6 +481,76 @@ pub(super) unsafe fn layout_header_for_user(user_ptr: usize) -> Option<*mut GcHe
     }
 }
 
+/// True when a traced object provably yields NO child slot to any collector
+/// walk, so the walk can be SKIPPED rather than performed and found empty. On a
+/// chain-node heap half the traced objects are of this shape.
+///
+/// This is a claim about four independent edge sources, and every one of them
+/// needs its own term. `GC_LAYOUT_POINTER_FREE` alone is NOT enough, because it
+/// describes the PAYLOAD and nothing else:
+///
+/// * **the payload** — `GC_LAYOUT_POINTER_FREE`, which
+///   `heap_payload_slot_selection` already trusts to skip the whole payload
+///   without consulting a mask;
+/// * **the kind's prefix and meta edges** — the reason for the kind term, and
+///   the reason it comes first. `gc_child_slots` builds `ArrayElements` as
+///   `new(header, None, range)`: no prefix, no meta, no meta2. Every other
+///   layout kind carries at least one. `ObjectFields` carries the meta record,
+///   which #6812 records as "fatal for the spill buffer, reachable through meta
+///   alone"; `RegExpFields` and `ObjectMeta` carry a prefix and two meta edges
+///   each. And POINTER_FREE is emphatically not an array-only bit: a closure is
+///   ALLOCATED pointer-free (`symbol/properties.rs`, #7154) and only leaves that
+///   state when a capture store records a pointer, and a typed object whose
+///   shape has an empty pointer mask acquires it (`gc/layout/typed_shape.rs`).
+///   Skipping either would drop edges the payload bit says nothing about — the
+///   closure's dynamic property values and static `.prototype`, the object's
+///   meta record, shape `keys` edge and overflow fields;
+/// * **the array's named-property reserve slots** — `GC_ARRAY_NAMED_PROPS`,
+///   which live in front of element 0, outside every layout range;
+/// * **a residual `Object.setPrototypeOf` entry** — the per-owner header bit
+///   from #10611, which is what makes this affordable to ask per object.
+///
+/// A FORWARDED header is never skippable, whatever its layout: array growth
+/// installs PERMANENT forwarding stubs, and walking the stub is what propagates
+/// liveness across the hop (#6228). The same guard on the sibling leaf skip in
+/// `gc/trace.rs` is there for this reason.
+///
+/// NOT SUFFICIENT ON ITS OWN FOR THE FULL MARK. `gc/trace.rs` reads every word
+/// of a pointer-free payload through `proxy::gc_observe_traced_value` when a
+/// proxy trace is active, because a proxy id is a `POINTER_TAG` value in the
+/// proxy-id band rather than a heap pointer — which is precisely why the layout
+/// mask is entitled to call a payload holding one pointer free. The full mark's
+/// call site therefore ANDs in `!proxy_trace_active`; the copying minor and the
+/// remembered-set rebuild both ignore `PointerFreeRange` and need no such term.
+#[inline]
+pub(crate) unsafe fn gc_object_yields_no_child_slots(header: *const GcHeader) -> bool {
+    // ORDER IS LOAD-BEARING, and it is a measurement, not a preference. Every
+    // object the copying minor moves asks this, and most say no; a first
+    // version that asked the type table first cost +0.07% to +0.12% on the
+    // three fixtures where almost nothing qualifies. The three header-word
+    // terms fold into ONE mask compare on a word `move_young` has already
+    // loaded, so a non-candidate is rejected in two instructions.
+    let reserved = (*header)._reserved;
+    if reserved
+        & (GC_LAYOUT_STATE_MASK
+            | crate::gc::GC_ARRAY_NAMED_PROPS
+            | crate::gc::GC_RESIDUAL_PROTO_OWNER)
+        != GC_LAYOUT_POINTER_FREE
+    {
+        return false;
+    }
+    if (*header).gc_flags & GC_FLAG_FORWARDED != 0 {
+        return false;
+    }
+    // Keyed on the TYPE rather than the rewrite kind, so the surviving path is
+    // a byte compare instead of a table load. This is conservative in the safe
+    // direction: a future type that also had no prefix/meta edge and no
+    // uncovered sibling would simply not be admitted here, costing a walk it
+    // could have skipped. `the_array_type_still_pairs_with_the_prefix_free_
+    // layout_kind` pins the two table facts this leans on.
+    (*header).obj_type == crate::gc::GC_TYPE_ARRAY
+}
+
 #[inline]
 pub(crate) unsafe fn layout_init_pointer_free(user_ptr: *mut u8) {
     let Some(header) = layout_header_for_user(user_ptr as usize) else {

crates/perry-runtime/src/gc/tests/layout_trace.rs

@@ -229,11 +229,21 @@ fn test_raw_numeric_array_layout_transfers_on_copying_minor_and_skips_payload()
     }
     assert_eq!(test_layout_pointer_slot_count(after, 4), Some(0));
     assert_eq!(test_heap_child_slot_count(after as *mut u8), 0);
-    assert!(
-        trace.layout_scans.raw_numeric_array_slots_skipped >= 4,
-        "copied raw numeric array payload should be skipped by layout scan: {:?}",
-        trace.layout_scans
-    );
+    // #10362 changed WHERE this payload stops being scanned, and therefore what
+    // the evidence for it is. The layout-scan counters are charged BY the walk;
+    // a copied raw-numeric array is now not walked at all, so
+    // `raw_numeric_array_slots_skipped` no longer counts it. The subject of this
+    // test is unchanged and in fact stronger — the payload is not scanned — so
+    // it is asserted against the mechanism that now decides it, which is
+    // falsifiable in a way `>= 0` would not be.
+    unsafe {
+        assert!(
+            crate::gc::gc_object_yields_no_child_slots(header),
+            "a copied raw numeric array must be admitted by the zero-slot skip, \
+             which is what now keeps its payload off the scan: reserved={:#x}",
+            (*header)._reserved
+        );
+    }
 }
 
 #[test]

crates/perry-runtime/src/gc/tests/mod.rs

@@ -97,3 +97,4 @@ mod u8_inline_cache;
 mod weak_read_barrier;
 mod young_leaf_route;
 mod young_log_tests;
+mod zero_slot_skip;

crates/perry-runtime/src/gc/tests/zero_slot_skip.rs

@@ -0,0 +1,230 @@
+//! The zero-slot skip (#10362): an object that provably yields no child slot is
+//! marked and moved but never queued for a walk that would find nothing.
+//!
+//! Skipping a walk is skipping every edge that walk would have produced, so the
+//! witnesses here are organised by EDGE SOURCE, not by fixture. Each term of
+//! `gc_object_yields_no_child_slots` gets a case that fails without it, and the
+//! full mark's extra `!proxy_trace_active` term — the one no ordinary GC
+//! fixture can see — gets a real collection and a sabotaged twin.
+
+use super::super::trace::zero_slot_skip_sabotage;
+use super::super::*;
+use super::support::*;
+
+fn full_collect() {
+    let trigger = GcTriggerSnapshot {
+        kind: GcTriggerKind::Manual,
+        steps_before: Some(GcStepSnapshot::current()),
+    };
+    let _ = GcCycleState::new_full(trigger).run_to_completion();
+}
+
+fn alloc_proxy_endpoint() -> (*mut u8, f64) {
+    let ptr = gc_malloc(
+        std::mem::size_of::<crate::closure::ClosureHeader>(),
+        GC_TYPE_CLOSURE,
+    );
+    unsafe {
+        init_test_closure(ptr);
+    }
+    (ptr, f64::from_bits(ptr_bits(ptr as usize)))
+}
+
+/// A plain pointer-free array: the population the skip exists for.
+unsafe fn pointer_free_array(length: u32) -> (*mut crate::array::ArrayHeader, *mut u64) {
+    let (arr, elements) = alloc_old_test_array(length);
+    layout_init_pointer_free(arr as *mut u8);
+    (arr, elements)
+}
+
+unsafe fn header_of(user: usize) -> *mut GcHeader {
+    header_from_user_ptr(user as *const u8) as *mut GcHeader
+}
+
+// ------------------------------------------------------------ the predicate --
+
+/// The predicate keys on `GC_TYPE_ARRAY` for speed, which is only sound while
+/// that type is the one whose rewrite arm has no uncovered sibling and whose
+/// layout kind has no prefix or meta edge. Both are table facts, so both are
+/// pinned here rather than argued in a comment.
+#[test]
+fn the_array_type_still_pairs_with_the_prefix_free_layout_kind() {
+    assert_eq!(
+        gc_type_rewrite_descriptor_kind(GC_TYPE_ARRAY),
+        GcRewriteDescriptorKind::Array,
+        "the skip assumes GC_TYPE_ARRAY takes the Array rewrite arm, whose only \
+         siblings are named props and the residual prototype"
+    );
+    assert_eq!(
+        gc_type_layout_slot_kind(GC_TYPE_ARRAY),
+        GcLayoutSlotKind::ArrayElements,
+        "the skip assumes GC_TYPE_ARRAY's layout kind yields no prefix or meta \
+         child edge, which is what gc_child_slots builds for ArrayElements"
+    );
+}
+
+#[test]
+fn a_plain_pointer_free_array_is_admitted() {
+    let _guard = GcTestIsolationGuard::new();
+    unsafe {
+        let (arr, _) = pointer_free_array(4);
+        assert!(
+            gc_object_yields_no_child_slots(header_of(arr as usize)),
+            "a pointer-free array with no named props and no residual prototype \
+             is exactly the population this skip is for"
+        );
+    }
+}
+
+#[test]
+fn an_array_that_still_holds_pointers_is_refused() {
+    let _guard = GcTestIsolationGuard::new();
+    unsafe {
+        let (arr, _) = alloc_old_test_array(4);
+        assert!(
+            !gc_object_yields_no_child_slots(header_of(arr as usize)),
+            "without GC_LAYOUT_POINTER_FREE the payload may hold anything"
+        );
+    }
+}
+
+#[test]
+fn named_properties_refuse_the_skip() {
+    let _guard = GcTestIsolationGuard::new();
+    unsafe {
+        let (arr, _) = pointer_free_array(4);
+        let header = header_of(arr as usize);
+        assert!(gc_object_yields_no_child_slots(header), "premise");
+        (*header)._reserved |= crate::gc::GC_ARRAY_NAMED_PROPS;
+        assert!(
+            !gc_object_yields_no_child_slots(header),
+            "named-property reserve slots sit in front of element 0, outside \
+             every layout range, so POINTER_FREE says nothing about them"
+        );
+    }
+}
+
+#[test]
+fn a_residual_prototype_owner_refuses_the_skip() {
+    let _guard = GcTestIsolationGuard::new();
+    unsafe {
+        let (arr, _) = pointer_free_array(4);
+        let header = header_of(arr as usize);
+        assert!(gc_object_yields_no_child_slots(header), "premise");
+        (*header)._reserved |= crate::gc::GC_RESIDUAL_PROTO_OWNER;
+        assert!(
+            !gc_object_yields_no_child_slots(header),
+            "an explicit Object.setPrototypeOf value is a child edge of its \
+             owner whatever the payload holds (#10493)"
+        );
+    }
+}
+
+#[test]
+fn a_forwarded_array_refuses_the_skip() {
+    let _guard = GcTestIsolationGuard::new();
+    unsafe {
+        let (arr, _) = pointer_free_array(4);
+        let header = header_of(arr as usize);
+        assert!(gc_object_yields_no_child_slots(header), "premise");
+        (*header).gc_flags |= GC_FLAG_FORWARDED;
+        assert!(
+            !gc_object_yields_no_child_slots(header),
+            "array growth installs PERMANENT forwarding stubs and walking the \
+             stub is what propagates liveness across the hop (#6228)"
+        );
+    }
+}
+
+/// The kind term, and the reason it is a term at all: `GC_LAYOUT_POINTER_FREE`
+/// is NOT an array-only bit. A closure is allocated pointer-free
+/// (`symbol/properties.rs`, #7154) and a typed object with an empty pointer mask
+/// acquires it. Both carry child edges outside the payload, so admitting them
+/// on the payload bit alone would drop those edges silently.
+#[test]
+fn a_pointer_free_non_array_is_refused_whatever_its_payload_says() {
+    let _guard = GcTestIsolationGuard::new();
+    unsafe {
+        let (obj, _) = alloc_old_test_object(1);
+        layout_init_pointer_free(obj as *mut u8);
+        let obj_header = header_of(obj as usize);
+        assert_eq!(
+            (*obj_header)._reserved & GC_LAYOUT_STATE_MASK,
+            GC_LAYOUT_POINTER_FREE,
+            "premise: the object really is marked pointer-free"
+        );
+        assert!(
+            !gc_object_yields_no_child_slots(obj_header),
+            "an object carries the meta record edge, the shape keys edge and \
+             its overflow fields, none of which the payload bit describes"
+        );
+
+        let (closure_ptr, _) = alloc_proxy_endpoint();
+        layout_init_pointer_free(closure_ptr);
+        assert!(
+            !gc_object_yields_no_child_slots(header_of(closure_ptr as usize)),
+            "a closure is ALLOCATED pointer-free and still has dynamic property \
+             values and a static prototype edge"
+        );
+    }
+}
+
+// --------------------------------------------- the full mark's proxy term ---
+
+/// A live proxy reachable ONLY through a pointer-free array, which is itself
+/// reached as a FIELD (so the mark takes `mark_field_into_worklist`, the skip
+/// site, rather than the root path). Returns whether the proxy survived.
+fn proxy_behind_a_pointer_free_array(sabotaged: bool) -> bool {
+    let _guard = CopyingNurseryTestGuard::new(1);
+    let _triggers = GcTriggerThresholdTestGuard::suppress_automatic_triggers();
+    let (_target_ptr, target) = alloc_proxy_endpoint();
+    let (_handler_ptr, handler) = alloc_proxy_endpoint();
+    let proxy = crate::proxy::js_proxy_new(target, handler);
+
+    let (arr, elements) = unsafe { pointer_free_array(1) };
+    unsafe {
+        *elements = proxy.to_bits();
+        assert!(
+            gc_object_yields_no_child_slots(header_of(arr as usize)),
+            "premise: the carrier must be a skip candidate, or this proves nothing"
+        );
+    }
+    // Reached as a FIELD, not as a root: the skip lives in
+    // `mark_field_into_worklist`, and the root path does not go through it.
+    let (holder, fields) = unsafe { alloc_old_test_array(1) };
+    unsafe {
+        *fields = ptr_bits(arr as usize);
+        layout_init_all_pointer_slots(holder as *mut u8);
+    }
+    js_shadow_slot_set(0, ptr_bits(holder as usize));
+
+    {
+        let _sabotage = sabotaged.then(zero_slot_skip_sabotage::Guard::arm);
+        full_collect();
+    }
+    let live = crate::proxy::test_proxy_slot_is_live(proxy);
+    js_shadow_slot_set(0, crate::value::TAG_UNDEFINED);
+    live
+}
+
+#[test]
+fn a_proxy_behind_a_pointer_free_array_survives_a_full_trace() {
+    assert!(
+        proxy_behind_a_pointer_free_array(false),
+        "the full mark must still read every word of a pointer-free payload \
+         while a proxy trace is active: a proxy id is a POINTER_TAG value in \
+         the proxy-id band, not a heap pointer, which is why the layout mask \
+         calls that payload pointer-free in the first place"
+    );
+}
+
+#[test]
+fn sabotaging_the_proxy_gate_strands_that_proxys_target() {
+    assert!(
+        !proxy_behind_a_pointer_free_array(true),
+        "with the !proxy_trace_active term removed the array is skipped, the \
+         registry entry is never observed, gc_finish_full_trace prunes it and \
+         a LIVE proxy loses its target and handler. If this twin ever passes, \
+         the term is unwitnessed."
+    );
+}