diff --git a/changelog.d/10669-zero-slot-skip.md b/changelog.d/10669-zero-slot-skip.md new file mode 100644 index 0000000000..c9757c2763 --- /dev/null +++ b/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. diff --git a/crates/perry-runtime/src/gc/copying.rs b/crates/perry-runtime/src/gc/copying.rs index 7a510d1d64..54751c82cc 100644 --- a/crates/perry-runtime/src/gc/copying.rs +++ b/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); diff --git a/crates/perry-runtime/src/gc/layout.rs b/crates/perry-runtime/src/gc/layout.rs index cb5e79282a..5726984e28 100644 --- a/crates/perry-runtime/src/gc/layout.rs +++ b/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 { diff --git a/crates/perry-runtime/src/gc/tests/layout_trace.rs b/crates/perry-runtime/src/gc/tests/layout_trace.rs index ff187248aa..0bbf10e94e 100644 --- a/crates/perry-runtime/src/gc/tests/layout_trace.rs +++ b/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] diff --git a/crates/perry-runtime/src/gc/tests/mod.rs b/crates/perry-runtime/src/gc/tests/mod.rs index 7a7e8a70e3..63d50b4fa6 100644 --- a/crates/perry-runtime/src/gc/tests/mod.rs +++ b/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; diff --git a/crates/perry-runtime/src/gc/tests/zero_slot_skip.rs b/crates/perry-runtime/src/gc/tests/zero_slot_skip.rs new file mode 100644 index 0000000000..792dbb85d8 --- /dev/null +++ b/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::(), + 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." + ); +} diff --git a/crates/perry-runtime/src/gc/trace.rs b/crates/perry-runtime/src/gc/trace.rs index 669dde6f88..18894b7c90 100644 --- a/crates/perry-runtime/src/gc/trace.rs +++ b/crates/perry-runtime/src/gc/trace.rs @@ -1359,8 +1359,27 @@ pub(super) unsafe fn mark_field_into_worklist( let forwarded = flags & GC_FLAG_FORWARDED != 0; #[cfg(test)] let forwarded = flags & GC_FLAG_FORWARDED != 0 && !leaf_mark_sabotage::ignoring_forwarding(); + // #10362: a pointer-free array yields no slot either, and on a chain-node + // heap it is half the traced objects — the obj_type-keyed leaf test above + // cannot see them, because they are arrays and not the strings it was + // written for. + // + // ONLY WHEN NO PROXY TRACE IS ACTIVE. `trace_heap_rewrite_slots` reads + // every word of a POINTER-FREE payload through `gc_observe_traced_value` + // when `proxy_trace_active`, because a proxy id is a POINTER_TAG value in + // the proxy-id band and not a heap pointer — which is exactly why the + // layout mask calls that payload pointer free. Skipping the object would + // leave the entry unobserved, `gc_finish_full_trace` would prune it, and a + // LIVE proxy's target and handler would be collected. The other two + // consumers of this predicate ignore `PointerFreeRange` and carry no such + // term; the asymmetry is deliberate. + #[cfg(not(test))] + let proxy_gate = proxy_trace_active; + #[cfg(test)] + let proxy_gate = proxy_trace_active && !zero_slot_skip_sabotage::respecting_proxy_gate(); if !forwarded - && gc_type_rewrite_descriptor_kind((*header).obj_type) == GcRewriteDescriptorKind::Leaf + && (gc_type_rewrite_descriptor_kind((*header).obj_type) == GcRewriteDescriptorKind::Leaf + || (!proxy_gate && gc_object_yields_no_child_slots(header))) { return true; } @@ -1373,6 +1392,41 @@ pub(super) unsafe fn mark_field_into_worklist( true } +/// Sabotage switches for the zero-slot skip (#10362). Test builds only. +/// +/// `respecting_proxy_gate` DISARMS the `!proxy_trace_active` term, i.e. makes +/// the full mark skip a pointer-free array even while a proxy trace is running. +/// That is the defect the gate exists to prevent, and +/// `gc::tests::zero_slot_skip` requires it to strand a live proxy's target. +#[cfg(test)] +pub(crate) mod zero_slot_skip_sabotage { + use std::cell::Cell; + + thread_local! { + static IGNORE_PROXY_GATE: Cell = const { Cell::new(false) }; + } + + #[inline] + pub(crate) fn respecting_proxy_gate() -> bool { + IGNORE_PROXY_GATE.with(Cell::get) + } + + pub(crate) struct Guard(bool); + + impl Guard { + pub(crate) fn arm() -> Self { + Self(IGNORE_PROXY_GATE.with(|s| s.replace(true))) + } + } + + impl Drop for Guard { + fn drop(&mut self) { + let prior = self.0; + IGNORE_PROXY_GATE.with(|s| s.set(prior)); + } + } +} + /// Sabotage switch for the leaf-mark test: a forwarded pointer-free object is /// not queued either, so its forwarding hop is never followed. Test builds /// only. diff --git a/crates/perry-runtime/src/gc/verify.rs b/crates/perry-runtime/src/gc/verify.rs index 48a642a65d..12e94ba7d0 100644 --- a/crates/perry-runtime/src/gc/verify.rs +++ b/crates/perry-runtime/src/gc/verify.rs @@ -303,6 +303,12 @@ pub(super) unsafe fn remember_evacuated_old_copy_young_slots( if !crate::arena::pointer_in_old_gen(user_ptr as usize) { return; } + // #10362: no child slot means no old->young edge to remember. This pass + // ignores `PointerFreeRange`, so unlike the full mark it needs no proxy + // term. + if crate::gc::gc_object_yields_no_child_slots(header) { + return; + } visit_gc_rewrite_slots(header, |slot| unsafe { if crate::weakref::is_weak_target_trace_slot(header, slot.slot) { return;