From 07899c99ada7e1328752bf88dd97ab20d7ca8187 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Thu, 9 Jul 2026 22:46:42 +0200 Subject: [PATCH] =?UTF-8?q?perf(gc):=20minor=20collections=20skip=20the=20?= =?UTF-8?q?whole-heap=20old=E2=86=92young=20RS=20rebuild=20(#6181)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A minor collection ran the from-scratch OldToYoungRememberedRebuildState walk in AtomicFinalize->RememberedSetRebuild — a full ArenaObjectCursor over every arena object plus the entire malloc registry, on every minor (with require_marked=false, so it even resurrected dead-but-unswept old parents). That makes minor pauses grow with total heap / process lifetime. A minor's old→young remembered set is already maintained without that walk: - the codegen + runtime write barriers record every old→young edge at mutation time; - this cycle's evacuation-created edges are re-remembered by evacuation_sticky (rebuild_evacuated_old_to_young_remembered_set over the cycle's moved headers, built in atomic_finalize_minor_prelude); - reclaim's restore_surviving_dirty_coverage (#5029) rescans the pre-clear dirty snapshot with the same slot predicate the evacuation verifier uses and re-remembers any slot still pointing into the nursery. So for minors, skip the whole-heap rebuild and leave live_old_to_young_sticky None; reclaim then restores only evacuation_sticky + the dirty snapshot. Full cycles are unchanged: they still run the authoritative require_marked whole-heap rebuild (they are inherently O(heap)). Safety (no dropped old→young edge / UAF): every old→young edge present at reclaim is either (a) mutator-created — recorded by the barrier, hence in the pre-clear dirty snapshot and re-remembered by restore_surviving_dirty_coverage when the child is still young, or (b) evacuation-created — covered by evacuation_sticky. The state-machine minor never promotes into OLD_ARENA except via tracked evacuation (age-bump only flags TENURED in place, staying nursery-classified), so no untracked promotion introduces an edge. The copying fast path is a separate code path and already handles its promoted headers via its own rebuild_evacuated_old_to_young_remembered_set(moved). Observability: GcCycleTrace gains remembered_set.rebuild_objects_scanned (the rebuild walk's object-visit count) so the skip is provable — 0 for a minor, O(heap) for a full cycle. Tests: - test_minor_skips_whole_heap_old_to_young_rebuild: a minor over a large pinned old-gen reports rebuild_objects_scanned == 0; a full cycle over the same heap reports >= the old-object count. - test_minor_preserves_old_to_young_edge_across_minors: an old→young edge recorded before a minor survives 4 successive minors (RS root marking keeps reaching the young child), i.e. the under-remembering guard. Fix 1 from #6181 (restrict the minor SWEEP's old-gen object walk) is NOT in this change — see the PR description for the specific mark-invariant blocker. Reference: #6181; 2026-07-09 GC audit. --- crates/perry-runtime/src/gc/cycle.rs | 35 +++-- crates/perry-runtime/src/gc/telemetry.rs | 7 + crates/perry-runtime/src/gc/tests/oldgen.rs | 165 ++++++++++++++++++++ crates/perry-runtime/src/gc/verify.rs | 11 ++ 4 files changed, 207 insertions(+), 11 deletions(-) diff --git a/crates/perry-runtime/src/gc/cycle.rs b/crates/perry-runtime/src/gc/cycle.rs index 2756b3ef9..679d83d17 100644 --- a/crates/perry-runtime/src/gc/cycle.rs +++ b/crates/perry-runtime/src/gc/cycle.rs @@ -1245,14 +1245,29 @@ impl GcCycleState { } } AtomicFinalizeSubphase::RememberedSetRebuild => { - let require_marked = self.minor.is_none(); + // Fix 2 (#6181): only FULL cycles rebuild the old→young + // remembered set from a whole-heap walk. A minor's old→young + // RS is maintained incrementally by the write barriers during + // mutation, plus this cycle's `evacuation_sticky` (edges the + // evacuation created — built in `atomic_finalize_minor_prelude`) + // and reclaim's `restore_surviving_dirty_coverage` snapshot + // repair (#5029). The from-scratch O(all-objects) walk is + // redundant for a minor — and, with `require_marked=false`, it + // even resurrects dead-but-unswept old parents. Skip it and + // leave `live_old_to_young_sticky` None; reclaim then restores + // only `evacuation_sticky` + the pre-clear dirty snapshot. + if self.minor.is_some() { + self.atomic_finalize = None; + self.phase = GcCyclePhase::Sweep; + return; + } let done = { let state = self .atomic_finalize .as_mut() .expect("atomic finalize state exists"); let rebuild = state.remembered_rebuild.get_or_insert_with(|| { - OldToYoungRememberedRebuildState::new(require_marked) + OldToYoungRememberedRebuildState::new(/* require_marked = */ true) }); rebuild.step(budget) }; @@ -1264,16 +1279,14 @@ impl GcCycleState { .remembered_rebuild .take() .expect("remembered rebuild state exists"); - self.live_old_to_young_sticky = Some(rebuild.finish()); - if self.minor.is_some() { - self.atomic_finalize = None; - self.phase = GcCyclePhase::Sweep; - } else { - self.atomic_finalize - .as_mut() - .expect("atomic finalize state exists") - .subphase = AtomicFinalizeSubphase::WeakProcessing; + if let Some(trace) = self.trace.as_mut() { + trace.old_to_young_rebuild_objects_scanned = rebuild.objects_scanned(); } + self.live_old_to_young_sticky = Some(rebuild.finish()); + self.atomic_finalize + .as_mut() + .expect("atomic finalize state exists") + .subphase = AtomicFinalizeSubphase::WeakProcessing; } } AtomicFinalizeSubphase::DisableBarrier => { diff --git a/crates/perry-runtime/src/gc/telemetry.rs b/crates/perry-runtime/src/gc/telemetry.rs index 7f4e7b669..717a4370b 100644 --- a/crates/perry-runtime/src/gc/telemetry.rs +++ b/crates/perry-runtime/src/gc/telemetry.rs @@ -614,6 +614,11 @@ pub(super) struct GcCycleTrace { pub(super) malloc_before: usize, pub(super) remembered_set_before: usize, pub(super) remembered_set: RememberedSetTraceStats, + /// Objects visited by the whole-heap old→young remembered-set rebuild + /// (#6181). Full cycles walk every arena+malloc object here; minors skip + /// the walk entirely (0) — their RS is maintained by the barriers plus + /// evacuation_sticky + restore_surviving_dirty_coverage. + pub(super) old_to_young_rebuild_objects_scanned: usize, pub(super) old_young_edge_verifier: OldYoungEdgeVerifyStats, pub(super) old_pages: crate::arena::OldPageSummary, pub(super) conservative_root_count: usize, @@ -674,6 +679,7 @@ impl GcCycleTrace { malloc_before: malloc_object_count(), remembered_set_before: remembered_set_size(), remembered_set: RememberedSetTraceStats::default(), + old_to_young_rebuild_objects_scanned: 0, old_young_edge_verifier: OldYoungEdgeVerifyStats::default(), old_pages: crate::arena::OldPageSummary::default(), conservative_root_count: 0, @@ -794,6 +800,7 @@ impl GcCycleTrace { "dirty_slot_pages_considered": self.remembered_set.dirty_slot_pages_considered, "dirty_slot_ranges_scanned": self.remembered_set.dirty_slot_ranges_scanned, "dirty_slots_scanned": self.remembered_set.dirty_slots_scanned, + "rebuild_objects_scanned": self.old_to_young_rebuild_objects_scanned, }); let old_pages_json = serde_json::json!({ "pages": self.old_pages.pages, diff --git a/crates/perry-runtime/src/gc/tests/oldgen.rs b/crates/perry-runtime/src/gc/tests/oldgen.rs index 5829d7036..814e20ea3 100644 --- a/crates/perry-runtime/src/gc/tests/oldgen.rs +++ b/crates/perry-runtime/src/gc/tests/oldgen.rs @@ -1017,3 +1017,168 @@ fn test_old_gen_in_use_bytes_delta_matches_recompute_across_alloc_and_reclaim() clear_marks(); remembered_set_clear(); } + +/// #6181 (2026-07-09 GC audit): a minor collection must NOT run the +/// whole-heap old→young remembered-set rebuild. A minor's old→young RS is +/// maintained by the write barriers plus this cycle's `evacuation_sticky` +/// and reclaim's `restore_surviving_dirty_coverage`, so the from-scratch +/// O(all-objects) walk is skipped. A FULL cycle still runs it. Proven via +/// the trace's `remembered_set.rebuild_objects_scanned` object-visit counter, +/// which scales with the heap for a full cycle and is 0 for a minor. +#[test] +fn test_minor_skips_whole_heap_old_to_young_rebuild() { + let _isolation = copying_nursery_isolation_lock(); + let _trigger_guard = GcTriggerThresholdTestGuard::suppress_automatic_triggers(); + reset_remembered_set(); + clear_marks(); + clear_mark_seeds(); + crate::arena::old_pages_begin_gc_cycle(); + + // A substantial old-gen live set (pinned so it survives and is not + // finalized). If a minor walked the old gen for the RS rebuild, the + // object-visit counter would scale with this set. + const OLD_OBJECTS: usize = 64; + let mut old_headers = Vec::with_capacity(OLD_OBJECTS); + for _ in 0..OLD_OBJECTS { + let (obj, _fields) = unsafe { alloc_old_test_object(2) }; + let header = unsafe { header_from_user_ptr(obj as *const u8) }; + unsafe { + (*header).gc_flags |= GC_FLAG_PINNED; + } + old_headers.push(header); + } + // A little nursery churn so the minor has real young work. + for _ in 0..8 { + let _ = unsafe { alloc_nursery_test_object(1) }; + } + + let minor_trace = collect_minor_trace(GcTriggerKind::Direct); + assert_eq!( + minor_trace.old_to_young_rebuild_objects_scanned, 0, + "a minor must not run the whole-heap old→young RS rebuild" + ); + let minor_event = minor_trace.into_json(GcStepSnapshot::current()); + assert_eq!( + minor_event["remembered_set"]["rebuild_objects_scanned"].as_u64(), + Some(0), + "minor RS rebuild object-visit count must be 0 in the trace JSON" + ); + + // A full cycle DOES run the rebuild — the counter proves it is wired and + // scales with the (now-large) heap, so the minor's 0 is a genuine skip + // rather than the counter being dead. + let full_outcome = gc_collect_full_mark_sweep_with_trigger(GcTriggerSnapshot { + kind: GcTriggerKind::Direct, + steps_before: Some(GcStepSnapshot::current()), + }); + let full_trace = full_outcome.trace.expect("full GC trace requested"); + assert!( + full_trace.old_to_young_rebuild_objects_scanned >= OLD_OBJECTS, + "a full cycle must walk the whole heap for the RS rebuild (got {}, expected >= {OLD_OBJECTS})", + full_trace.old_to_young_rebuild_objects_scanned, + ); + + for header in old_headers { + unsafe { + (*header).gc_flags &= !GC_FLAG_PINNED; + } + } + clear_marks(); + remembered_set_clear(); +} + +/// #6181: an old→young edge recorded before a minor must survive the minor +/// (young child kept via the remembered set) across repeated minors, even +/// though the minor no longer rebuilds the RS from a whole-heap walk. The +/// edge is carried entirely by the write barrier → reclaim's +/// `restore_surviving_dirty_coverage`. This is the under-remembering +/// (use-after-free) guard for Fix 2: if any minor dropped the edge, the +/// child would be swept and the RS root mark below would not reach it. +#[test] +fn test_minor_preserves_old_to_young_edge_across_minors() { + let _isolation = copying_nursery_isolation_lock(); + let _trigger_guard = GcTriggerThresholdTestGuard::suppress_automatic_triggers(); + let _barrier_guard = GeneratedWriteBarrierTestGuard::active(); + reset_remembered_set(); + clear_marks(); + clear_mark_seeds(); + crate::arena::old_pages_begin_gc_cycle(); + + // Pinned old parent (stable address, always kept) holding a young child + // reachable ONLY through the parent's field. + let (parent, fields) = unsafe { alloc_old_test_object(1) }; + let parent_user = parent as usize; + let parent_header = unsafe { header_from_user_ptr(parent as *const u8) }; + unsafe { + (*parent_header).gc_flags |= GC_FLAG_PINNED; + } + // Unrelated large old-gen set with no young children (makes the old gen + // big enough that a whole-heap rebuild would be visibly costly). + let mut other_old = Vec::new(); + for _ in 0..48 { + let (obj, _f) = unsafe { alloc_old_test_object(1) }; + let h = unsafe { header_from_user_ptr(obj as *const u8) }; + unsafe { + (*h).gc_flags |= GC_FLAG_PINNED; + } + other_old.push(h); + } + + let child = crate::arena::arena_alloc_gc(40, 8, GC_TYPE_OBJECT) as usize; + let child_header = unsafe { header_from_user_ptr(child as *const u8) }; + assert!(crate::arena::pointer_in_nursery(child)); + unsafe { + *fields = ptr_bits(child); + layout_note_slot(parent_user, 0, ptr_bits(child)); + } + js_write_barrier_slot(ptr_bits(parent_user), fields as u64, ptr_bits(child)); + assert!( + remembered_set_size() > 0, + "barrier must record the old→young edge" + ); + + for cycle in 0..4 { + let trace = collect_minor_trace(GcTriggerKind::Direct); + assert_eq!( + trace.old_to_young_rebuild_objects_scanned, 0, + "cycle {cycle}: minor must skip the whole-heap RS rebuild" + ); + // The child (reachable only via the old parent) must still be covered + // by the remembered set: RS root marking reaches and marks it. + assert!( + remembered_set_size() > 0, + "cycle {cycle}: old→young edge must survive the minor" + ); + assert_eq!( + unsafe { *fields }, + ptr_bits(child), + "cycle {cycle}: non-moving minor must leave the parent's slot intact" + ); + clear_marks(); + let valid_ptrs = build_valid_pointer_set(); + let stats = mark_remembered_set_roots(&valid_ptrs); + assert!( + stats.newly_marked > 0, + "cycle {cycle}: RS root marking must reach the child" + ); + unsafe { + assert_ne!( + (*child_header).gc_flags & GC_FLAG_MARKED, + 0, + "cycle {cycle}: young child must be markable via the surviving RS edge" + ); + // Clear the child's mark so the next minor re-derives its coverage + // from the remembered set alone (not a stale mark). + (*child_header).gc_flags &= !GC_FLAG_MARKED; + } + } + + unsafe { + (*parent_header).gc_flags &= !GC_FLAG_PINNED; + for h in other_old { + (*h).gc_flags &= !GC_FLAG_PINNED; + } + } + clear_marks(); + remembered_set_clear(); +} diff --git a/crates/perry-runtime/src/gc/verify.rs b/crates/perry-runtime/src/gc/verify.rs index 92b9fda5c..d453fb5ae 100644 --- a/crates/perry-runtime/src/gc/verify.rs +++ b/crates/perry-runtime/src/gc/verify.rs @@ -286,6 +286,7 @@ pub(super) struct OldToYoungRememberedRebuildState { arena_cursor: Option, arena_done: bool, malloc_index: usize, + objects_scanned: usize, done: bool, } @@ -299,10 +300,18 @@ impl OldToYoungRememberedRebuildState { )), arena_done: false, malloc_index: 0, + objects_scanned: 0, done: false, } } + /// Number of heap objects this whole-heap rebuild walk has visited. Used + /// by the GC trace to prove that minors do NOT run this O(all-objects) + /// walk (#6181): full cycles report the walked object count, minors 0. + pub(super) fn objects_scanned(&self) -> usize { + self.objects_scanned + } + pub(super) fn step(&mut self, budget: usize) -> bool { if self.done { return true; @@ -320,6 +329,7 @@ impl OldToYoungRememberedRebuildState { break; }; remaining -= 1; + self.objects_scanned += 1; let header = header_ptr as *mut GcHeader; unsafe { remember_retained_old_to_young_slots(&mut self.sticky, header, self.require_marked); @@ -337,6 +347,7 @@ impl OldToYoungRememberedRebuildState { }; self.malloc_index += 1; remaining -= 1; + self.objects_scanned += 1; unsafe { remember_retained_old_to_young_slots(&mut self.sticky, header, self.require_marked); }