From 78c2c3ab361f44a17ce4fc6e74d27cb4fdf7087c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Wed, 12 Aug 2026 05:45:38 +0200 Subject: [PATCH 1/2] fix(gc): restore old-page relocation contract --- .../collectors/proven_this_routing_tests.rs | 45 ++++ crates/perry-codegen/src/expr/mod.rs | 3 +- .../src/expr/scalar_slot_root.rs | 27 +++ .../src/lower_call/method_override.rs | 5 +- .../perry-codegen/src/lower_call/new_alloc.rs | 4 +- .../property_get/dynamic_dispatch.rs | 2 +- .../src/lower_call/scalar_method.rs | 2 +- .../perry-codegen/src/testing/temp_slots.rs | 68 +++++- .../tests/temp_root_operand_temporaries.rs | 14 +- crates/perry-runtime/src/arena/mod.rs | 1 + crates/perry-runtime/src/arena/page_meta.rs | 17 ++ crates/perry-runtime/src/gc/mod.rs | 19 +- crates/perry-runtime/src/gc/oldgen.rs | 67 +++--- crates/perry-runtime/src/gc/oldgen_defrag.rs | 52 +++-- .../gc/tests/copying/survival_and_malloc.rs | 6 + crates/perry-runtime/src/gc/tests/oldgen.rs | 191 ++++++++++++++-- .../src/gc/tests/runtime_roots.rs | 1 + .../runtime_roots/old_defrag_contract.rs | 83 +++++++ crates/perry-runtime/src/json/mod.rs | 25 +++ .../src/node_submodules/diagnostics.rs | 55 +++++ .../perry-runtime/src/node_submodules/mod.rs | 4 + crates/perry-runtime/src/perf_hooks.rs | 19 ++ scripts/gc_root_dominance_check.py | 209 ++++-------------- scripts/gc_runtime_root_holders.json | 28 +-- scripts/gc_runtime_root_holders.py | 8 + 25 files changed, 665 insertions(+), 290 deletions(-) create mode 100644 crates/perry-runtime/src/gc/tests/runtime_roots/old_defrag_contract.rs diff --git a/crates/perry-codegen/src/collectors/proven_this_routing_tests.rs b/crates/perry-codegen/src/collectors/proven_this_routing_tests.rs index eb28cdf8c9..99b386d710 100644 --- a/crates/perry-codegen/src/collectors/proven_this_routing_tests.rs +++ b/crates/perry-codegen/src/collectors/proven_this_routing_tests.rs @@ -780,6 +780,7 @@ fn tower_case_routes_to_proven_this_clone() { /// guard block → `icmp eq i64` → the branch that enters the clone's block. #[test] fn tower_route_is_guarded_by_the_class_keys_token() { + let _shadow = crate::codegen::helpers::NativeRootsPin::shadow(); let ir = emit(&tower_site_module(), false); let bs = blocks(&ir); let clone = pshape_definitions(&ir) @@ -812,6 +813,22 @@ fn tower_route_is_guarded_by_the_class_keys_token() { .find(|l| l.contains(&format!("store i64 {}, ptr ", global_reg))) .unwrap_or_else(|| panic!("the hoisted keys token is never stored:\n{ir}")); let slot = store.rsplit(' ').next().expect("slot name"); + let store_pos = ir + .find(store) + .expect("the hoisted class-keys store should be in the function"); + let bind_pos = ir + .lines() + .find(|line| line.contains("call void @js_shadow_slot_bind") && line.contains(slot)) + .and_then(|line| ir.find(line)) + .unwrap_or_else(|| { + panic!( + "the cached class-keys pointer is not a mutable shadow root; old-page moves would leave this copy stale:\n{ir}" + ) + }); + assert!( + store_pos < bind_pos, + "the class-keys slot must be initialized before the root scanner can read it:\n{ir}" + ); // 3. … which the guard block reloads … let expected = guard_body .iter() @@ -843,6 +860,34 @@ fn tower_route_is_guarded_by_the_class_keys_token() { ); } +#[test] +fn tower_class_keys_cache_is_a_native_mutable_root() { + let _native = crate::codegen::helpers::NativeRootsPin::native(); + let ir = emit(&tower_site_module(), false); + let global_load = ir + .lines() + .find(|line| line.contains("= load i64, ptr @perry_class_keys_")) + .unwrap_or_else(|| panic!("the class keys token is never read:\n{ir}")); + let global_reg = global_load.trim().split(' ').next().expect("ssa name"); + let cast = ir + .lines() + .find(|line| line.contains(&format!("= inttoptr i64 {global_reg} to ptr addrspace(1)"))) + .unwrap_or_else(|| { + panic!("the cached class-keys pointer never enters a native GC root slot:\n{ir}") + }); + let cast_reg = cast.trim().split(' ').next().expect("cast ssa name"); + let root_store = ir + .lines() + .find(|line| line.contains(&format!("store ptr addrspace(1) {cast_reg}, ptr "))) + .unwrap_or_else(|| panic!("the native class-keys root is never stored:\n{ir}")); + let slot = root_store.rsplit(' ').next().expect("root slot name"); + assert!( + ir.lines() + .any(|line| line.contains(&format!("{slot} = alloca ptr addrspace(1)"))), + "the class-keys cache alloca must be in the collector address space:\n{ir}" + ); +} + /// Profitability ratchet (#7142): a clone that deletes exactly ONE guarded /// field site does not earn the tower's inline re-check, so the tower keeps /// calling the public body. diff --git a/crates/perry-codegen/src/expr/mod.rs b/crates/perry-codegen/src/expr/mod.rs index d1c5d5180e..14f7b426b6 100644 --- a/crates/perry-codegen/src/expr/mod.rs +++ b/crates/perry-codegen/src/expr/mod.rs @@ -176,7 +176,8 @@ pub(crate) use slot_rep::{ pub(crate) use dispatch::{lower_expr, lower_math_operand}; pub(crate) use scalar_slot_root::{ - root_entry_alloca, root_scalar_replaced_slot, root_scalar_replaced_slot_unconditional, + entry_init_load_rooted_global, root_entry_alloca, root_scalar_replaced_slot, + root_scalar_replaced_slot_unconditional, }; pub(crate) use shadow_slot::{ current_closure_ptr_value, emit_persistent_shadow_root_barrier, diff --git a/crates/perry-codegen/src/expr/scalar_slot_root.rs b/crates/perry-codegen/src/expr/scalar_slot_root.rs index 91f25397df..16de41008e 100644 --- a/crates/perry-codegen/src/expr/scalar_slot_root.rs +++ b/crates/perry-codegen/src/expr/scalar_slot_root.rs @@ -99,6 +99,33 @@ pub(crate) fn root_scalar_replaced_slot_unconditional(ctx: &mut FnCtx<'_>, slot: root_entry_alloca(ctx, slot); } +/// Cache a GC-pointer global in a function-entry slot and make that cached +/// copy a mutable root. +/// +/// Registering the global itself keeps the object live and rewrites the +/// global, but an old-page move must also rewrite every function-local copy +/// loaded before the move. The bind is emitted in the same post-init setup +/// region, immediately after `entry_init_load_global` stores the initialized +/// value, so the collector never observes an uninitialized slot. There is no +/// per-store incremental barrier: the registered global already owns +/// liveness, and this immutable duplicate exists only to receive rewrites. +pub(crate) fn entry_init_load_rooted_global( + ctx: &mut FnCtx<'_>, + global_name: &str, + ty: crate::types::LlvmType, +) -> String { + let slot = ctx.func.entry_init_load_global(global_name, ty); + let Some(idx) = ctx.func.reserve_shadow_slot() else { + return slot; + }; + ctx.scalar_slot_shadow_slots.insert(slot.clone(), idx); + ctx.func.entry_setup_call_void( + "js_shadow_slot_bind", + &[(I32, &idx.to_string()), (PTR, &slot)], + ); + slot +} + /// Make an arbitrary entry-block alloca a **rewritten** GC root (#7202). /// /// Scalar replacement is not the only producer of storage that holds a heap diff --git a/crates/perry-codegen/src/lower_call/method_override.rs b/crates/perry-codegen/src/lower_call/method_override.rs index 7fccea165e..fda988e7b2 100644 --- a/crates/perry-codegen/src/lower_call/method_override.rs +++ b/crates/perry-codegen/src/lower_call/method_override.rs @@ -238,7 +238,8 @@ pub(super) fn emit_guarded_direct_method_call( .unwrap_or_else(|| crate::codegen::generic_method_body_name(direct_fn)); let expected_class_id_str = expected_class_id.to_string(); - let expected_keys_slot = ctx.func.entry_init_load_global(&keys_global_name, I64); + let expected_keys_slot = + crate::expr::entry_init_load_rooted_global(ctx, &keys_global_name, I64); let expected_keys = ctx.block().load(I64, &expected_keys_slot); let key_idx = ctx.strings.intern(property); @@ -263,7 +264,7 @@ pub(super) fn emit_guarded_direct_method_call( let subclass_keys: Vec = subclass_arms .iter() .map(|arm| { - let slot = ctx.func.entry_init_load_global(&arm.keys_global, I64); + let slot = crate::expr::entry_init_load_rooted_global(ctx, &arm.keys_global, I64); ctx.block().load(I64, &slot) }) .collect(); diff --git a/crates/perry-codegen/src/lower_call/new_alloc.rs b/crates/perry-codegen/src/lower_call/new_alloc.rs index 35c93fc545..dce17528f3 100644 --- a/crates/perry-codegen/src/lower_call/new_alloc.rs +++ b/crates/perry-codegen/src/lower_call/new_alloc.rs @@ -330,7 +330,7 @@ fn emit_instance_alloc_inner( let keys_slot = if let Some(s) = ctx.class_keys_slots.get(class_name).cloned() { s } else { - let s = ctx.func.entry_init_load_global(&keys_global_name, I64); + let s = crate::expr::entry_init_load_rooted_global(ctx, &keys_global_name, I64); ctx.class_keys_slots .insert(class_name.to_string(), s.clone()); s @@ -447,7 +447,7 @@ fn emit_instance_alloc_inner( let keys_slot = if let Some(s) = ctx.class_keys_slots.get(class_name).cloned() { s } else { - let s = ctx.func.entry_init_load_global(&keys_global_name, I64); + let s = crate::expr::entry_init_load_rooted_global(ctx, &keys_global_name, I64); ctx.class_keys_slots .insert(class_name.to_string(), s.clone()); s diff --git a/crates/perry-codegen/src/lower_call/property_get/dynamic_dispatch.rs b/crates/perry-codegen/src/lower_call/property_get/dynamic_dispatch.rs index af88ec3e23..4ce487763a 100644 --- a/crates/perry-codegen/src/lower_call/property_get/dynamic_dispatch.rs +++ b/crates/perry-codegen/src/lower_call/property_get/dynamic_dispatch.rs @@ -108,7 +108,7 @@ fn emit_tower_pshape_call( ) -> String { // The global is read ONCE per function (entry-hoisted); the case block only // reloads it from the stack slot, which mem2reg folds away. - let keys_slot = ctx.func.entry_init_load_global(&route.keys_global, I64); + let keys_slot = crate::expr::entry_init_load_rooted_global(ctx, &route.keys_global, I64); let expected_keys = ctx.block().load(I64, &keys_slot); let proven_idx = ctx.new_block(&format!("idispatch.case{}.pshape", case_no)); diff --git a/crates/perry-codegen/src/lower_call/scalar_method.rs b/crates/perry-codegen/src/lower_call/scalar_method.rs index e354b5f20a..b6ecabc0ad 100644 --- a/crates/perry-codegen/src/lower_call/scalar_method.rs +++ b/crates/perry-codegen/src/lower_call/scalar_method.rs @@ -550,7 +550,7 @@ fn materialize_scalar_receiver( let keys_slot = if let Some(slot) = ctx.class_keys_slots.get(class_name).cloned() { slot } else { - let slot = ctx.func.entry_init_load_global(&keys_global_name, I64); + let slot = crate::expr::entry_init_load_rooted_global(ctx, &keys_global_name, I64); ctx.class_keys_slots .insert(class_name.to_string(), slot.clone()); slot diff --git a/crates/perry-codegen/src/testing/temp_slots.rs b/crates/perry-codegen/src/testing/temp_slots.rs index 74c0e9afc5..c621c90fc3 100644 --- a/crates/perry-codegen/src/testing/temp_slots.rs +++ b/crates/perry-codegen/src/testing/temp_slots.rs @@ -369,10 +369,10 @@ pub fn derives_from_slot_load(fn_ir: &str, reg: &str, depth: usize) -> bool { /// a null `addrspace(1)` store. /// /// Required as well as the alloca type because an `alloca i64` is also how -/// codegen spells an unrelated scratch cell — the per-class inline-keys cache -/// in `@main` is one, and without this filter it read as a temp root and made -/// `a_class_that_runs_no_user_code_emits_no_instance_root` fail for a slot that -/// holds a static keys pointer. +/// codegen spells unrelated scratch cells. The per-class inline-keys cache is +/// now a precise function-lifetime root (#7876), so it has the same seed and +/// alloca type as a temp root; [`temp_root_slots`] excludes that one by the +/// provenance of the value stored into it. pub fn zero_seeded_slots(fn_ir: &str) -> std::collections::BTreeSet { let mut touched: std::collections::BTreeSet = std::collections::BTreeSet::new(); let mut seeded: std::collections::BTreeSet = std::collections::BTreeSet::new(); @@ -432,6 +432,13 @@ pub fn temp_root_slots(fn_ir: &str) -> Vec { let defs = defs(fn_ir); let undefined = undefined_literal(); let seeded = zero_seeded_slots(fn_ir); + let class_key_loads: Vec<&str> = defs + .iter() + .filter_map(|(®, def)| { + def.starts_with("load i64, ptr @perry_class_keys_") + .then_some(reg) + }) + .collect(); slot_traffic(fn_ir) .into_iter() .filter(|(slot, _)| seeded.contains(slot)) @@ -460,6 +467,20 @@ pub fn temp_root_slots(fn_ir: &str) -> Vec { _ => false, }) }) + .filter(|(_, events)| { + // #7876: the registered class-keys global owns liveness, while a + // function-local immutable copy is a precise root solely so an + // old-page move can rewrite it. It lasts for the whole function; + // it is not one of #7487's scoped expression temporaries. Match + // the exact registered-global provenance so an ordinary temp that + // happens to lack its closing clear remains visible to this gate. + !events.iter().any(|event| match event { + SlotEvent::Store { value, .. } => class_key_loads + .iter() + .any(|load| derives_from(&defs, value, load, 4)), + _ => false, + }) + }) .map(|(slot, _)| slot) .collect() } @@ -633,4 +654,43 @@ entry.0: "…in both lowerings" ); } + + #[test] + fn a_function_lifetime_class_keys_root_is_not_a_temp_root() { + let shadow = "\ +define i32 @main() { +entry.0: + %keys = alloca i64 + store i64 0, ptr %keys + %r1 = load i64, ptr @perry_class_keys_fixture + store i64 %r1, ptr %keys + %r2 = load i64, ptr %keys + call void @consume(i64 %r2) + ret i32 0 +} +"; + let native = "\ +define i32 @main() gc \"statepoint-example\" { +entry.0: + %keys = alloca ptr addrspace(1) + store ptr addrspace(1) null, ptr %keys + %r1 = load i64, ptr @perry_class_keys_fixture + %r1.rs4p = inttoptr i64 %r1 to ptr addrspace(1) + store ptr addrspace(1) %r1.rs4p, ptr %keys + %r2.rs4p = load ptr addrspace(1), ptr %keys + %r2 = ptrtoint ptr addrspace(1) %r2.rs4p to i64 + call void @consume(i64 %r2) + ret i32 0 +} +"; + assert_no_temp_rooting(shadow, "class-key cache shadow root"); + assert_no_temp_rooting(native, "class-key cache native root"); + + let unrelated = shadow.replace("@perry_class_keys_fixture", "@some_other_global"); + assert_eq!( + temp_root_slots(&unrelated), + vec!["%keys".to_string()], + "only the registered class-key provenance is exempt; a missing temp-root clear must stay visible" + ); + } } diff --git a/crates/perry-codegen/tests/temp_root_operand_temporaries.rs b/crates/perry-codegen/tests/temp_root_operand_temporaries.rs index e65cfd0463..7e8aa469d8 100644 --- a/crates/perry-codegen/tests/temp_root_operand_temporaries.rs +++ b/crates/perry-codegen/tests/temp_root_operand_temporaries.rs @@ -24,7 +24,9 @@ //! `a_collection_free_construction_emits_no_this_slot_root` needed the pin even //! though it was *passing*, because under the post-#7370 native-roots default //! its `!contains("@js_shadow_slot_bind")` is true of every program (hazard 4: -//! the gate ran, its subject did not). +//! the gate ran, its subject did not). Since #7876 every class-key cache also +//! has one function-lifetime bind; that negative asserts there is exactly that +//! one bind rather than no bind anywhere in the function. //! //! The rest of this file is lowering-INDEPENDENT and deliberately unpinned. //! @@ -1312,9 +1314,11 @@ fn a_collection_free_construction_emits_no_this_slot_root() { instance temp root — the `this`-slot bind is gated on the same \ predicate", ); - assert!( - !f.contains("@js_shadow_slot_bind"), - "an inert construction must not grow the shadow frame for a `this` \ - slot that cannot go stale (#7202):\n{f}" + assert_eq!( + f.matches("@js_shadow_slot_bind").count(), + 1, + "an inert construction needs only #7876's function-lifetime class-key \ + root; it must not grow the shadow frame for a `this` slot that cannot \ + go stale (#7202):\n{f}" ); } diff --git a/crates/perry-runtime/src/arena/mod.rs b/crates/perry-runtime/src/arena/mod.rs index 12a5855c9d..13ec129590 100644 --- a/crates/perry-runtime/src/arena/mod.rs +++ b/crates/perry-runtime/src/arena/mod.rs @@ -140,5 +140,6 @@ pub(crate) use page_meta::{ pub(crate) use page_meta::{ deferred_old_page_registrations_len, generation_page_base, old_arena_page_index_clear_for_tests, old_page_meta_for_tests, + old_page_meta_snapshot_calls_for_tests, reset_old_page_meta_snapshot_calls_for_tests, DEFERRED_OLD_PAGE_REGISTRATION_CAP, GENERATION_CLASS_SHIFT, GENERATION_PAGE_SIZE, }; diff --git a/crates/perry-runtime/src/arena/page_meta.rs b/crates/perry-runtime/src/arena/page_meta.rs index b751a2e1dc..394d3c5aca 100644 --- a/crates/perry-runtime/src/arena/page_meta.rs +++ b/crates/perry-runtime/src/arena/page_meta.rs @@ -338,6 +338,11 @@ thread_local! { static OLD_GEN_PAGE_DIRTY_EPOCH: Cell = const { Cell::new(1) }; } +#[cfg(test)] +thread_local! { + static OLD_PAGE_META_SNAPSHOT_CALLS: Cell = const { Cell::new(0) }; +} + // --- #7469 hot-TLS address providers. See `crate::tls_hot`. --- /// Address of this thread's `PAGE_GENERATION_CACHE`. @@ -1178,6 +1183,8 @@ pub(crate) fn old_page_summary() -> OldPageSummary { } pub(crate) fn old_page_meta_snapshot() -> Vec { + #[cfg(test)] + OLD_PAGE_META_SNAPSHOT_CALLS.with(|calls| calls.set(calls.get().saturating_add(1))); // #7624 READER (`OLD_GEN_PAGE_META`): this one drives real policy — // `gc/oldgen_defrag.rs` selects evacuation pages from it. flush_deferred_old_page_registrations(); @@ -1194,6 +1201,16 @@ pub(crate) fn old_page_meta_snapshot() -> Vec { }) } +#[cfg(test)] +pub(crate) fn old_page_meta_snapshot_calls_for_tests() -> usize { + OLD_PAGE_META_SNAPSHOT_CALLS.with(Cell::get) +} + +#[cfg(test)] +pub(crate) fn reset_old_page_meta_snapshot_calls_for_tests() { + OLD_PAGE_META_SNAPSHOT_CALLS.with(|calls| calls.set(0)); +} + /// Fold a stale `dirty_slots` stamp down to the effective value so a copied /// `OldPageMeta` handed to a caller always reports this cycle's dirty-slot /// count directly in `dirty_slots`, without the caller needing the epoch (#6181). diff --git a/crates/perry-runtime/src/gc/mod.rs b/crates/perry-runtime/src/gc/mod.rs index e9cd96e7e9..5ceea48683 100644 --- a/crates/perry-runtime/src/gc/mod.rs +++ b/crates/perry-runtime/src/gc/mod.rs @@ -303,13 +303,6 @@ fn gc_collect_minor_with_trigger_inner( // recorded in the cycle trace and read back by the evacuation-policy tests. let evacuation_policy_allowed = true; let force_evacuation = gc_force_evacuate_enabled(); - let old_page_selection = if old_to_young_tracking_complete() { - select_old_page_defrag_pages(force_evacuation) - } else { - OldPageDefragSelection::default() - }; - let old_page_source_blocks = - crate::arena::old_arena_source_blocks_for_pages(&old_page_selection.pages); // MARK_SEEDS persists across GC cycles. Clear before any try_mark // call so trace sees only this cycle's freshly-marked headers. clear_mark_seeds(); @@ -333,6 +326,18 @@ fn gc_collect_minor_with_trigger_inner( }; } clear_mark_seeds(); + // Old-page defrag belongs to the non-copying fallback below. Snapshotting + // and sorting all old-page metadata before trying the copying fast path + // charged every ordinary minor an O(old pages) cost even though that path + // cannot consume the selection. Defer both selection and source-block + // expansion until the fast path has declined the collection. + let old_page_selection = if old_to_young_tracking_complete() { + select_old_page_defrag_pages(force_evacuation) + } else { + OldPageDefragSelection::default() + }; + let old_page_source_blocks = + crate::arena::old_arena_source_blocks_for_pages(&old_page_selection.pages); GcCycleState::new_minor_fallback( trigger, trace, diff --git a/crates/perry-runtime/src/gc/oldgen.rs b/crates/perry-runtime/src/gc/oldgen.rs index f3b35aff6a..6b4615bb6f 100644 --- a/crates/perry-runtime/src/gc/oldgen.rs +++ b/crates/perry-runtime/src/gc/oldgen.rs @@ -1340,10 +1340,10 @@ struct ArenaSweepObjectsState { /// Full traces DO visit every live parent, so mark-based reclaim stays /// sound there (and bounds the accumulation). minor_sweep: bool, - /// Old-gen blocks selected for page defrag this cycle. Their live contents - /// were evacuated out during this same cycle, so what is left really is - /// reclaimable even in a minor — and the block-level reclaim needs - /// `block_has_live` to stay false for them. + /// Old-gen blocks selected for page defrag this cycle. Every indexed + /// occupant was evacuated out during this same cycle, so what is left + /// really is reclaimable even in a minor — and the block-level reclaim + /// needs `block_has_live` to stay false for them. targeted_old_blocks: Option>, freed_bytes: u64, retained_forwarded_stub_objects: usize, @@ -1480,8 +1480,8 @@ impl ArenaSweepObjectsState { /// following minor stopped tracing the array's other pointer elements, /// sweeping objects that were still referenced. /// - /// The old-page defrag targets are exempt: this cycle evacuated their live - /// contents, so the remainder is genuinely reclaimable. + /// The old-page defrag targets are exempt: this cycle evacuated every + /// indexed occupant, so the remainder is genuinely reclaimable. #[inline] fn unmarked_is_provably_dead(&self, block_idx: usize) -> bool { if !self.minor_sweep || block_idx < self.old_block_start { @@ -1875,34 +1875,39 @@ pub(super) fn evacuate_selected_old_pages_collecting( &source_blocks.pages }; - crate::arena::old_arena_walk_objects_on_pages(selected_pages, |header_ptr| { - let header = header_ptr as *mut GcHeader; + // A minor trace deliberately does not establish old-generation liveness: + // an unmarked old object is normally live and merely unvisited. Reclaim is + // block-granular, so moving only the marked occupants of selected pages + // and then targeting their whole source block discards live unmarked + // neighbors (#7876). Snapshot every indexed occupant of the containing + // source blocks and evacuate the block all-or-nothing. Dead old objects + // remain indexed until a full trace proves them dead, so conservatively + // copying them here preserves the same minor-GC retention contract. + let mut source_headers = Vec::new(); + crate::arena::old_arena_walk_objects_on_pages(excluded_pages, |header_ptr| { + source_headers.push(header_ptr as *mut GcHeader); + }); + let source_block_is_movable = source_headers.iter().all(|&header| unsafe { + if header.is_null() { + return false; + } + let user_ptr = (header as *mut u8).add(GC_HEADER_SIZE); + let flags = (*header).gc_flags; + crate::arena::pointer_in_old_gen(user_ptr as usize) + && flags != 0 + && flags & (GC_FLAG_FORWARDED | GC_FLAG_PINNED) == 0 + && gc_type_is_movable((*header).obj_type) + && !is_conservatively_pinned(header) + }); + if source_headers.is_empty() || !source_block_is_movable { + return evacuated; + } + + for header in source_headers { unsafe { let user_ptr = (header as *mut u8).add(GC_HEADER_SIZE); - if !crate::arena::pointer_in_old_gen(user_ptr as usize) { - return; - } let flags = (*header).gc_flags; - if flags & GC_FLAG_FORWARDED != 0 { - return; - } - if flags & GC_FLAG_MARKED == 0 { - return; - } - if flags & GC_FLAG_PINNED != 0 { - return; - } - if !gc_type_is_movable((*header).obj_type) { - return; - } - if is_conservatively_pinned(header) { - return; - } - let total = (*header).size as usize; - if !old_object_pages_all_selected(header, total, selected_pages) { - return; - } let payload = total - GC_HEADER_SIZE; let new_user = crate::arena::arena_alloc_gc_old_excluding_pages( @@ -1936,7 +1941,7 @@ pub(super) fn evacuate_selected_old_pages_collecting( evacuated.old_page_moved_objects = evacuated.old_page_moved_objects.saturating_add(1); evacuated.old_page_moved_bytes = evacuated.old_page_moved_bytes.saturating_add(total); } - }); + } evacuated } diff --git a/crates/perry-runtime/src/gc/oldgen_defrag.rs b/crates/perry-runtime/src/gc/oldgen_defrag.rs index 169ce48fb2..52efd2f96f 100644 --- a/crates/perry-runtime/src/gc/oldgen_defrag.rs +++ b/crates/perry-runtime/src/gc/oldgen_defrag.rs @@ -79,17 +79,15 @@ pub(super) fn select_old_page_defrag_pages_from_snapshot( selection } -// gh #6206 test hook: the defrag machinery's unit tests exercise the -// selection/copy/re-remember mechanics directly and must bypass the -// production off-gate below. Thread-local so parallel tests don't race. +// Test override for selection-policy tests. Thread-local so parallel tests do +// not race with the production default or one another. #[cfg(test)] thread_local! { pub(crate) static OLD_DEFRAG_TEST_OVERRIDE: std::cell::Cell> = const { std::cell::Cell::new(None) }; } -/// RAII enable for the defrag unit tests: forces the off-gate open on this -/// thread for the guard's lifetime. +/// RAII enable for defrag unit tests on this thread for the guard's lifetime. #[cfg(test)] pub(crate) struct OldDefragTestEnable; @@ -108,36 +106,46 @@ impl Drop for OldDefragTestEnable { } } +fn old_page_defrag_enabled_from_value(value: Option<&str>) -> bool { + !matches!(value, Some("0") | Some("off") | Some("false")) +} + fn old_page_defrag_enabled() -> bool { #[cfg(test)] if let Some(v) = OLD_DEFRAG_TEST_OVERRIDE.with(|c| c.get()) { return v; } use std::sync::OnceLock; - static OPT_IN: OnceLock = OnceLock::new(); - *OPT_IN.get_or_init(|| { - matches!( - std::env::var("PERRY_GC_OLD_DEFRAG").as_deref(), - Ok("1") | Ok("on") | Ok("true") - ) + static ENABLED: OnceLock = OnceLock::new(); + *ENABLED.get_or_init(|| { + old_page_defrag_enabled_from_value(std::env::var("PERRY_GC_OLD_DEFRAG").ok().as_deref()) }) } pub(super) fn select_old_page_defrag_pages(force: bool) -> OldPageDefragSelection { - // gh #6206: old-page defrag evacuation is OFF pending a rewrite-contract - // fix. With defrag active, a reader can observe a pre-move address of a - // defrag-moved old object long after the cycle (wild-pointer crash / - // silently corrupt cached value); the reproducer corrupts 6/6 with defrag - // enabled and is clean 6/6 with it disabled, on the same binary, while - // every heap-payload slot (arrays in-length, object fields, Map entries) - // verifies as correctly rewritten — the stale reference lives on a - // non-heap path (address-keyed cache / IC / side table) the defrag - // rewrite doesn't reach. Nursery evacuation and tenured promotion (the - // reclaim-critical moving paths) are unaffected. Re-enable for - // debugging/bisection with PERRY_GC_OLD_DEFRAG=1. + // #7876 restored the mutable-root contract for old movable addresses and + // made defrag the production default. Keep an explicit kill switch for + // field diagnosis and rollback without shipping a second binary. if !old_page_defrag_enabled() { return OldPageDefragSelection::default(); } let snapshot = crate::arena::old_page_meta_snapshot(); select_old_page_defrag_pages_from_snapshot(&snapshot, force) } + +#[cfg(test)] +mod tests { + use super::old_page_defrag_enabled_from_value; + + #[test] + fn old_page_defrag_defaults_on_with_an_explicit_kill_switch() { + assert!(old_page_defrag_enabled_from_value(None)); + assert!(old_page_defrag_enabled_from_value(Some("1"))); + assert!(old_page_defrag_enabled_from_value(Some("on"))); + assert!(old_page_defrag_enabled_from_value(Some("true"))); + assert!(!old_page_defrag_enabled_from_value(Some("0"))); + assert!(!old_page_defrag_enabled_from_value(Some("off"))); + assert!(!old_page_defrag_enabled_from_value(Some("false"))); + assert!(old_page_defrag_enabled_from_value(Some("unexpected"))); + } +} diff --git a/crates/perry-runtime/src/gc/tests/copying/survival_and_malloc.rs b/crates/perry-runtime/src/gc/tests/copying/survival_and_malloc.rs index 5699c71a24..0be9461c6a 100644 --- a/crates/perry-runtime/src/gc/tests/copying/survival_and_malloc.rs +++ b/crates/perry-runtime/src/gc/tests/copying/survival_and_malloc.rs @@ -127,7 +127,13 @@ fn test_copying_minor_preserves_old_page_accounting_for_defrag_policy() { "seeded unpinned live/dead old page should be selected for defrag" ); + crate::arena::reset_old_page_meta_snapshot_calls_for_tests(); let trace = collect_minor_trace(GcTriggerKind::Direct); + assert_eq!( + crate::arena::old_page_meta_snapshot_calls_for_tests(), + 0, + "a copying minor cannot consume an old-page defrag selection" + ); let promoted = (js_shadow_slot_get(0) & POINTER_MASK) as usize; let promoted_header = unsafe { header_from_user_ptr(promoted as *const u8) }; let promoted_total = unsafe { (*promoted_header).size as usize }; diff --git a/crates/perry-runtime/src/gc/tests/oldgen.rs b/crates/perry-runtime/src/gc/tests/oldgen.rs index 1e34d5b173..33204d0e21 100644 --- a/crates/perry-runtime/src/gc/tests/oldgen.rs +++ b/crates/perry-runtime/src/gc/tests/oldgen.rs @@ -488,7 +488,7 @@ fn test_old_page_defrag_policy_selection_prefers_fragmented_unpinned_pages() { } #[test] -fn test_old_page_defrag_forced_moves_only_marked_old_objects_on_selected_pages() { +fn test_old_page_defrag_moves_every_source_block_occupant_during_a_minor() { let _isolation = copying_nursery_isolation_lock(); reset_remembered_set(); clear_marks(); @@ -517,28 +517,27 @@ fn test_old_page_defrag_forced_moves_only_marked_old_objects_on_selected_pages() &mut original_headers, ); - assert_eq!(moved.old_page_moved_objects, 1); - assert_eq!(moved.old_page_moved_bytes, movable_total); - assert_eq!(new_headers.len(), 1); - assert_eq!(original_headers, vec![movable_header]); - assert!( - old_object_pages_disjoint_from_selected(new_headers[0], movable_total, &selected_pages), - "old-page copy must not land in any selected source page" - ); - assert!( - old_object_pages_disjoint_from_selected( - new_headers[0], - movable_total, - &source_blocks.pages - ), - "old-page copy must not land in the selected source block" - ); + assert_eq!(moved.old_page_moved_objects, 2); + assert_eq!(new_headers.len(), 2); + assert!(original_headers.contains(&movable_header)); + assert!(original_headers.contains(&unmarked_header)); + for &new_header in &new_headers { + assert!( + old_object_pages_disjoint_from_selected( + new_header, + unsafe { (*new_header).size as usize }, + &source_blocks.pages, + ), + "old-page copies must not land in their source block" + ); + } unsafe { assert_ne!((*movable_header).gc_flags & GC_FLAG_FORWARDED, 0); - assert_eq!( + assert_ne!( (*unmarked_header).gc_flags & GC_FLAG_FORWARDED, 0, - "unmarked old object on the selected page must not move" + "a minor cannot call an unmarked old neighbor dead; source-block \ + evacuation must move it too" ); assert!(crate::arena::pointer_in_old_gen( forwarding_address(movable_header) as usize @@ -546,7 +545,7 @@ fn test_old_page_defrag_forced_moves_only_marked_old_objects_on_selected_pages() } let released = release_evacuated_original_forwarding_stubs(&original_headers); - assert_eq!(released.released_original_objects, 1); + assert_eq!(released.released_original_objects, 2); assert_eq!(released.released_original_reusable_bytes, 0); assert_eq!(released.released_original_returned_bytes, 0); clear_marks(); @@ -644,7 +643,9 @@ fn test_old_page_defrag_skips_pinned_old_objects() { clear_mark_seeds(); CONS_PINNED.with(|s| s.borrow_mut().clear()); + let neighbor = crate::arena::arena_alloc_gc_old(64, 8, GC_TYPE_OBJECT) as usize; let pinned = crate::arena::arena_alloc_gc_old(64, 8, GC_TYPE_OBJECT) as usize; + let (neighbor_header, _) = old_test_header_and_size(neighbor); let (pinned_header, pinned_total) = old_test_header_and_size(pinned); let mut selected_pages = crate::fast_hash::new_ptr_hash_set(); for (page, _) in crate::arena::old_object_page_overlaps(pinned_header as usize, pinned_total) { @@ -672,6 +673,12 @@ fn test_old_page_defrag_skips_pinned_old_objects() { 0, "pinned old object address must remain stable" ); + assert_eq!( + (*neighbor_header).gc_flags & GC_FLAG_FORWARDED, + 0, + "a pinned occupant must reject the whole source block before any \ + movable neighbor is forwarded" + ); (*pinned_header).gc_flags &= !GC_FLAG_MARKED; crate::gc::unpin_object(pinned_header); } @@ -934,6 +941,150 @@ fn test_old_page_defrag_target_gate_emits_trace() { } } +#[test] +fn test_old_page_defrag_mixed_size_fragmentation_converges_to_released_block() { + let _isolation = copying_nursery_isolation_lock(); + let _trigger_guard = GcTriggerThresholdTestGuard::suppress_automatic_triggers(); + let _defrag = OldDefragTestEnable::new(); + reset_remembered_set(); + clear_marks(); + clear_mark_seeds(); + old_free_reset_for_test(); + CONS_PINNED.with(|s| s.borrow_mut().clear()); + + // Finish the allocator's current block so this fixture owns the next + // mapped old block. Each following 4 KiB page contains a repeatable mix: + // twelve 64-byte live objects alternating with 256-byte dead objects, + // followed by one live and one dead 128-byte object. No live-only page can + // accidentally make the source block unreclaimable. + let _old_block_filler = + crate::arena::arena_alloc_gc_old(2 * 1024 * 1024 - GC_HEADER_SIZE, 8, GC_TYPE_STRING); + crate::arena::old_pages_begin_gc_cycle(); + let live_small_payload = 64usize.saturating_sub(GC_HEADER_SIZE); + let dead_large_payload = 256usize.saturating_sub(GC_HEADER_SIZE); + let mixed_payload = 128usize.saturating_sub(GC_HEADER_SIZE); + assert!(live_small_payload > 0 && mixed_payload > 0); + + let mut live_headers = Vec::new(); + let mut dead_large_users = Vec::new(); + for _ in 0..2 { + for _ in 0..12 { + let live = + crate::arena::arena_alloc_gc_old(live_small_payload, 8, GC_TYPE_STRING) as usize; + live_headers.push(unsafe { header_from_user_ptr(live as *const u8) }); + dead_large_users.push(crate::arena::arena_alloc_gc_old( + dead_large_payload, + 8, + GC_TYPE_STRING, + ) as usize); + } + let live = crate::arena::arena_alloc_gc_old(mixed_payload, 8, GC_TYPE_STRING) as usize; + live_headers.push(unsafe { header_from_user_ptr(live as *const u8) }); + let _dead_mixed = + crate::arena::arena_alloc_gc_old(mixed_payload, 8, GC_TYPE_STRING) as usize; + } + for &header in &live_headers { + unsafe { + (*header).gc_flags |= GC_FLAG_MARKED; + } + } + + let _fragmenting_sweep = sweep_with_age_bump_and_old_reclaim(false, true); + let selection = select_old_page_defrag_pages(true); + assert!( + selection.selected_pages >= 2, + "fixture must expose at least two fragmented pages (selected={})", + selection.selected_pages + ); + let exact_total = + unsafe { (*header_from_user_ptr(dead_large_users[0] as *const u8)).size as usize }; + assert_eq!(exact_total, 256, "fixture's exact-fit size drifted"); + assert!(old_free_bytes() >= dead_large_users.len() * exact_total); + + let mut found_fixture_hole = false; + while let Some(addr) = old_free_take_exact(exact_total, None) { + if dead_large_users.contains(&addr) { + found_fixture_hole = true; + break; + } + } + assert!( + found_fixture_hole, + "mixed-size sweep must publish an exact-fit hole from the fragmented source block" + ); + + // Sweep clears MARKED; arm the selected live objects for the direct + // evacuation step below. Keep the page accounting from that same sweep: + // invalidated hole headers are deliberately absent from later walks, so a + // second accounting sweep would no longer describe the fragmentation it + // just exposed. + for &header in &live_headers { + unsafe { + (*header).gc_flags |= GC_FLAG_MARKED; + } + } + for &header in &live_headers { + let total = unsafe { (*header).size as usize }; + assert!( + old_object_pages_all_selected(header, total, &selection.pages), + "every live fixture object must lie wholly on selected fragmented pages" + ); + unsafe { + (*header).gc_flags |= GC_FLAG_MARKED; + } + } + + let source_blocks = crate::arena::old_arena_source_blocks_for_pages(&selection.pages); + assert!(!source_blocks.block_indices.is_empty()); + assert!( + crate::arena::old_arena_walk_objects_on_pages(&source_blocks.pages, |_| {}) > 0, + "the selected source must still be mapped and indexed before evacuation" + ); + let before = crate::arena::arena_telemetry_snapshot(); + + let mut new_headers = Vec::new(); + let mut original_headers = Vec::new(); + let moved = evacuate_selected_old_pages_collecting( + &selection.pages, + &mut new_headers, + &mut original_headers, + ); + assert_eq!(moved.old_page_moved_objects, live_headers.len()); + assert_eq!(original_headers.len(), live_headers.len()); + assert_eq!(new_headers.len(), live_headers.len()); + let released = release_evacuated_original_forwarding_stubs(&original_headers); + assert_eq!(released.released_original_objects, live_headers.len()); + + let reclaim = sweep_with_age_bump_and_targeted_old_reclaim_and_malloc( + true, + &source_blocks.block_indices, + false, + ); + let after = crate::arena::arena_telemetry_snapshot(); + assert!( + reclaim.reusable_bytes > 0 || reclaim.removed_bytes > 0, + "an emptied fragmented source block must become reusable, pooled, or returned" + ); + assert_eq!( + crate::arena::old_arena_walk_objects_on_pages(&source_blocks.pages, |_| {}), + 0, + "released source pages must have no stale object-index entries" + ); + for page in &source_blocks.pages { + assert!(crate::arena::old_page_meta_for_tests(*page).is_none()); + } + assert!( + after.old.in_use_bytes < before.old.in_use_bytes, + "telemetry must distinguish the released block from its live copied bytes" + ); + + old_free_reset_for_test(); + clear_marks(); + clear_mark_seeds(); + reset_remembered_set(); + CONS_PINNED.with(|s| s.borrow_mut().clear()); +} + #[test] fn test_old_page_defrag_trace_json_distinguishes_moved_from_reclaimable() { let mut trace = GcCycleTrace::new( diff --git a/crates/perry-runtime/src/gc/tests/runtime_roots.rs b/crates/perry-runtime/src/gc/tests/runtime_roots.rs index 6fd8ee270f..2ad1858d53 100644 --- a/crates/perry-runtime/src/gc/tests/runtime_roots.rs +++ b/crates/perry-runtime/src/gc/tests/runtime_roots.rs @@ -8,6 +8,7 @@ mod hook_dispatch_handles; mod interned_string_caches; mod iter_result_keys; mod json_shape_template; +mod old_defrag_contract; mod prototype_addr_cache; mod side_table_scanners; mod string_slice; diff --git a/crates/perry-runtime/src/gc/tests/runtime_roots/old_defrag_contract.rs b/crates/perry-runtime/src/gc/tests/runtime_roots/old_defrag_contract.rs new file mode 100644 index 0000000000..db3a256f12 --- /dev/null +++ b/crates/perry-runtime/src/gc/tests/runtime_roots/old_defrag_contract.rs @@ -0,0 +1,83 @@ +//! #7876: old-page compaction is safe only when every non-heap address copy +//! participates in the registered mutable-root rewrite. These probes plant a +//! forwarding address directly, so they test the rewrite contract without +//! depending on heap pressure or page-selection policy. + +use super::*; + +fn forwarded_string() -> (usize, usize, ValidPointerSet) { + let from = crate::string::js_string_from_bytes_longlived(b"id".as_ptr(), 2) as usize; + let valid_ptrs = build_valid_pointer_set(); + let to = crate::string::js_string_from_bytes_longlived(b"id".as_ptr(), 2) as usize; + unsafe { + set_forwarding_address(header_from_user_ptr(from as *const u8), to as *mut u8); + } + (from, to, valid_ptrs) +} + +#[test] +fn json_hot_key_ring_rewrites_with_its_owning_cache() { + crate::json::test_clear_parse_roots(); + let (from, to, valid_ptrs) = forwarded_string(); + crate::json::test_seed_parse_roots( + f64::from_bits(crate::value::TAG_UNDEFINED), + from as *const _, + ); + crate::json::test_seed_parse_key_ring(from as *const _); + + crate::json::scan_parse_roots_mut(&mut RuntimeRootVisitor::for_rewrite(&valid_ptrs)); + + assert_eq!(crate::json::test_parse_roots_snapshot().1, to); + assert_eq!( + crate::json::test_parse_key_ring_snapshot(), + vec![to], + "the hot-key mirror must not retain the old address after its owning cache rewrites" + ); + crate::json::test_clear_parse_roots(); +} + +#[test] +fn performance_entry_shape_identity_rewrites_as_metadata() { + let from = crate::array::js_array_alloc_with_length_longlived(0) as usize; + let valid_ptrs = build_valid_pointer_set(); + let to = crate::array::js_array_alloc_with_length_longlived(0) as usize; + unsafe { + set_forwarding_address(header_from_user_ptr(from as *const u8), to as *mut u8); + } + crate::perf_hooks::test_seed_perf_entry_keys_array(from); + + crate::perf_hooks::scan_perf_entries_roots_mut(&mut RuntimeRootVisitor::for_rewrite( + &valid_ptrs, + )); + + assert_eq!( + crate::perf_hooks::test_perf_entry_keys_array(), + to, + "performance-entry identity must follow its structurally rooted keys array" + ); +} + +#[test] +fn diagnostics_symbol_lookup_key_rekeys_after_a_move() { + let from = unsafe { crate::value::js_nanbox_get_pointer(crate::symbol::js_symbol_new_empty()) } + as usize; + let valid_ptrs = build_valid_pointer_set(); + let to = unsafe { crate::value::js_nanbox_get_pointer(crate::symbol::js_symbol_new_empty()) } + as usize; + unsafe { + set_forwarding_address(header_from_user_ptr(from as *const u8), to as *mut u8); + } + crate::node_submodules::diagnostics::test_seed_diag_symbol_key( + POINTER_TAG | (from as u64 & POINTER_MASK), + ); + + crate::node_submodules::scan_node_submodule_singleton_roots_mut( + &mut RuntimeRootVisitor::for_rewrite(&valid_ptrs), + ); + + assert_eq!( + crate::node_submodules::diagnostics::test_diag_symbol_keys(), + vec![POINTER_TAG | (to as u64 & POINTER_MASK)], + "diagnostics_channel's address-keyed symbol lookup must be rekeyed" + ); +} diff --git a/crates/perry-runtime/src/json/mod.rs b/crates/perry-runtime/src/json/mod.rs index 7c58cc1802..b28c0e209a 100644 --- a/crates/perry-runtime/src/json/mod.rs +++ b/crates/perry-runtime/src/json/mod.rs @@ -556,6 +556,17 @@ pub fn scan_parse_roots_mut(visitor: &mut crate::gc::RuntimeRootVisitor<'_>) { visitor.visit_tagged_raw_const_ptr_slot(ptr, crate::value::STRING_TAG); } }); + // PARSE_KEY_RING mirrors the cache's hottest values. It does not own + // their liveness, but its duplicate addresses must follow an old-page + // move after PARSE_KEY_CACHE has kept the strings alive. + PARSE_KEY_RING.with(|ring| { + for ptr in ring.borrow_mut().iter_mut() { + let mut addr = *ptr as usize; + if visitor.visit_metadata_usize_slot(&mut addr) { + *ptr = addr as *const StringHeader; + } + } + }); PARSE_SHAPE_CACHE.with(|c| { for entry in c.borrow_mut().iter_mut() { visitor.visit_raw_mut_ptr_slot(&mut entry.keys_array); @@ -621,6 +632,20 @@ pub(crate) fn test_seed_parse_roots(value: f64, key_ptr: *const StringHeader) { }); } +#[cfg(test)] +pub(crate) fn test_seed_parse_key_ring(key_ptr: *const StringHeader) { + PARSE_KEY_RING.with(|ring| { + let mut ring = ring.borrow_mut(); + ring.clear(); + ring.push(key_ptr); + }); +} + +#[cfg(test)] +pub(crate) fn test_parse_key_ring_snapshot() -> Vec { + PARSE_KEY_RING.with(|ring| ring.borrow().iter().map(|&ptr| ptr as usize).collect()) +} + /// Seed the stringify-side shape cache with a template keyed on `keys_arr`, /// so #7268's scanner tests can exercise the mark/rewrite halves without /// having to drive a whole `JSON.stringify` to populate it. diff --git a/crates/perry-runtime/src/node_submodules/diagnostics.rs b/crates/perry-runtime/src/node_submodules/diagnostics.rs index 2282ecf6d8..87be84f73a 100644 --- a/crates/perry-runtime/src/node_submodules/diagnostics.rs +++ b/crates/perry-runtime/src/node_submodules/diagnostics.rs @@ -333,6 +333,61 @@ pub(crate) fn channel_key(name: f64) -> Option { None } +/// Rekey the symbol-address lookup after an evacuating collection. +/// +/// `DIAG_CHANNELS[*].name` owns and roots the symbol. This map is only its +/// identity index, so following an existing forwarding address here must not +/// make the map key an additional root. +pub(crate) fn scan_diagnostics_channel_key_roots_mut( + visitor: &mut crate::gc::RuntimeRootVisitor<'_>, +) { + if !visitor.is_metadata_rewrite_phase() { + return; + } + DIAG_CHANNEL_BY_KEY.with(|map| { + let mut map = map.borrow_mut(); + if !map + .keys() + .any(|key| matches!(key, DiagChannelKey::Symbol(_))) + { + return; + } + + let old = std::mem::take(&mut *map); + for (mut key, id) in old { + if let DiagChannelKey::Symbol(bits) = &mut key { + let mut addr = (*bits & crate::value::POINTER_MASK) as usize; + if visitor.visit_metadata_usize_slot(&mut addr) { + *bits = crate::value::POINTER_TAG | (addr as u64 & crate::value::POINTER_MASK); + } + } + map.insert(key, id); + } + }); +} + +#[cfg(test)] +pub(crate) fn test_seed_diag_symbol_key(bits: u64) { + DIAG_CHANNEL_BY_KEY.with(|map| { + let mut map = map.borrow_mut(); + map.clear(); + map.insert(DiagChannelKey::Symbol(bits), 1); + }); +} + +#[cfg(test)] +pub(crate) fn test_diag_symbol_keys() -> Vec { + DIAG_CHANNEL_BY_KEY.with(|map| { + map.borrow() + .keys() + .filter_map(|key| match key { + DiagChannelKey::Symbol(bits) => Some(*bits), + DiagChannelKey::String(_) => None, + }) + .collect() + }) +} + /// True when `value` is a JS Symbol. `channel(symbol)` accepts symbols, but /// `tracingChannel(nameOrChannels)` rejects them (#3084) — Node's validator /// only allows a string name or a channel-object map there. diff --git a/crates/perry-runtime/src/node_submodules/mod.rs b/crates/perry-runtime/src/node_submodules/mod.rs index 04afbb49bb..60e76d0ae0 100644 --- a/crates/perry-runtime/src/node_submodules/mod.rs +++ b/crates/perry-runtime/src/node_submodules/mod.rs @@ -1469,6 +1469,10 @@ pub fn scan_node_submodule_singleton_roots_mut(visitor: &mut crate::gc::RuntimeR visitor.visit_nanbox_f64_slot(err); } }); + // Symbol keys duplicate the structurally rooted DIAG_CHANNELS names. + // This is a rewrite-only identity index and can exist independently of + // the export/namespace singleton caches guarded below. + diagnostics::scan_diagnostics_channel_key_roots_mut(visitor); if ANY_SINGLETON_ALLOCATED.load(Ordering::Acquire) == 0 { return; } diff --git a/crates/perry-runtime/src/perf_hooks.rs b/crates/perry-runtime/src/perf_hooks.rs index b2a98bc30e..63de982478 100644 --- a/crates/perry-runtime/src/perf_hooks.rs +++ b/crates/perry-runtime/src/perf_hooks.rs @@ -1541,6 +1541,25 @@ pub fn scan_perf_entries_roots_mut(visitor: &mut crate::gc::RuntimeRootVisitor<' c.set(bits); } }); + // This is an identity index into structurally rooted entry objects, not + // an owner. Follow a forwarding address without keeping the keys array + // alive on its own. + PERF_ENTRY_KEYS_ARRAY.with(|c| { + let mut addr = c.get(); + if addr != 0 && visitor.visit_metadata_usize_slot(&mut addr) { + c.set(addr); + } + }); +} + +#[cfg(test)] +pub(crate) fn test_seed_perf_entry_keys_array(addr: usize) { + PERF_ENTRY_KEYS_ARRAY.with(|slot| slot.set(addr)); +} + +#[cfg(test)] +pub(crate) fn test_perf_entry_keys_array() -> usize { + PERF_ENTRY_KEYS_ARRAY.with(|slot| slot.get()) } // ── Histograms (perf_histogram namespace) ──────────────────────────────────── diff --git a/scripts/gc_root_dominance_check.py b/scripts/gc_root_dominance_check.py index 3e880d90ba..91b6abfcc2 100755 --- a/scripts/gc_root_dominance_check.py +++ b/scripts/gc_root_dominance_check.py @@ -2550,9 +2550,8 @@ def matches(self, ins): return bool(self.load_re and self.load_re.search(ins.text)) -# `js_build_class_keys_array`'s body, which the first probe reads. rustfmt puts -# a top-level fn's closing brace at column 0, so "from the signature to the -# next line that is exactly `}`" is the body and nothing else. +# rustfmt puts a top-level fn's closing brace at column 0, so "from the +# signature to the next line that is exactly `}`" is the body and nothing else. def rust_fn_body(path, fn_name): """Source text of a top-level Rust fn, or None if it is not there.""" try: @@ -2573,85 +2572,6 @@ def rust_fn_body(path, fn_name): return None -def _probe_class_keys_longlived(): - """Every array allocation in `js_build_class_keys_array` is `_longlived`.""" - body = rust_fn_body("crates/perry-runtime/src/object/alloc.rs", - "js_build_class_keys_array") - if body is None: - return (False, "js_build_class_keys_array not found in " - "crates/perry-runtime/src/object/alloc.rs") - allocs = re.findall(r"js_array_alloc\w*", body) - if not allocs: - return (False, "js_build_class_keys_array no longer allocates its " - "array with a js_array_alloc* call; the arena it uses " - "must be re-established by hand") - bad = [a for a in allocs if "longlived" not in a] - if bad: - return (False, "js_build_class_keys_array now calls %s — a NURSERY " - "allocator. The class-keys exemption is void; the 64 " - "#7210 reports are real." % ", ".join(sorted(set(bad)))) - return (True, "%d longlived allocation(s)" % len(allocs)) - - -# The defrag gate moved from `gc/oldgen.rs` to `gc/oldgen_defrag.rs` in #7443. -# Search both rather than hardcoding one: this probe FAILING does not merely -# turn the job red, it declares the reports it suppresses to be real again, so -# a file rename silently converts a passing audit into a wall of false -# positives. Listing candidates keeps the next split cheap, and the -# "not found in ANY of" message names them so the fix is obvious. (#7540) -_OLD_DEFRAG_SOURCES = ( - "crates/perry-runtime/src/gc/oldgen_defrag.rs", - "crates/perry-runtime/src/gc/oldgen.rs", -) - - -def _probe_old_defrag_off_by_default(): - """Old-page defrag still returns an empty selection unless opted in.""" - src = None - src_path = None - for cand in _OLD_DEFRAG_SOURCES: - try: - with open(cand, encoding="utf-8", errors="replace") as fh: - text = fh.read() - except OSError: - continue - if "PERRY_GC_OLD_DEFRAG" in text: - src, src_path = text, cand - break - if src is None: - return (False, "PERRY_GC_OLD_DEFRAG not mentioned in any of %s; " - "old-page defrag may now be unconditional (or the file " - "moved again -- add it to _OLD_DEFRAG_SOURCES)" - % ", ".join(_OLD_DEFRAG_SOURCES)) - body = rust_fn_body(src_path, "select_old_page_defrag_pages") - if body is None: - return (False, - "select_old_page_defrag_pages not found in %s" % src_path) - if not re.search(r"if\s+!old_page_defrag_enabled\(\)\s*\{\s*\n\s*return\s+" - r"OldPageDefragSelection::default\(\);", body): - return (False, "select_old_page_defrag_pages no longer short-circuits " - "to an empty selection when defrag is disabled — " - "old-arena objects may relocate on a default build") - return (True, "gated on PERRY_GC_OLD_DEFRAG in %s, empty selection when off" - % src_path) - - -def _probe_class_keys_global_is_a_root(): - """The keys global is registered, so the array is never unreachable.""" - try: - with open("crates/perry-codegen/src/codegen/string_pool.rs", - encoding="utf-8", errors="replace") as fh: - src = fh.read() - except OSError: - return (False, "perry-codegen/src/codegen/string_pool.rs not readable") - if "js_build_class_keys_array" not in src: - return (False, "string_pool.rs no longer emits js_build_class_keys_array") - if "js_gc_register_global_root" not in src: - return (False, "string_pool.rs no longer registers the class-keys " - "global as a GC root — the array can now be swept") - return (True, "@perry_class_keys_* registered via js_gc_register_global_root") - - def _probe_boxes_outside_the_gc_heap(): """Boxes are `std::alloc::alloc`, never freed, never arena-allocated.""" try: @@ -2677,39 +2597,6 @@ def _probe_boxes_outside_the_gc_heap(): IMMOVABLE_SOURCES = ( - ImmovableSource( - key="class-keys", - label="@perry_class_keys_* / js_build_class_keys_array (old arena)", - knob="assume_old_defrag", - callees={"js_build_class_keys_array"}, - load_re=re.compile(r"load\s+\S+,\s*ptr\s+@perry_class_keys_\w+"), - not_movable_because=( - "js_build_class_keys_array allocates through " - "js_array_alloc_with_length_longlived — the OLD arena. The nursery " - "copying minor relocates nursery objects only, and the one thing " - "that relocates an old-arena object is old-page defrag, which " - "select_old_page_defrag_pages short-circuits off (#6206). No " - "shipped configuration and no gc_repsel_matrix arm sets " - "PERRY_GC_OLD_DEFRAG=1."), - not_reclaimable_because=( - "the @perry_class_keys_* global is registered with " - "js_gc_register_global_root at module init, so the array is " - "reachable from a root for the life of the process and no sweep " - "can free it. (#5042 registered the global for the DEFRAG rewrite; " - "the reachability is the side effect that closes this half.)"), - becomes_real_when=( - "PERRY_GC_OLD_DEFRAG ships on by default, or " - "js_build_class_keys_array stops using a _longlived allocator. " - "Re-check with --assume-old-defrag; the fix is to bind " - "entry_init_load_global's cache slot (expr::root_entry_alloca " - "accepts a bare i64 heap word)."), - probes=( - ("class-keys array is old-arena", _probe_class_keys_longlived), - ("old-page defrag is off by default", _probe_old_defrag_off_by_default), - ("the keys global is a registered root", - _probe_class_keys_global_is_a_root), - ), - ), ImmovableSource( key="box", label="js_box_alloc* (outside the GC heap)", @@ -2776,8 +2663,7 @@ def audit_immovable_sources(): return 0 -def classify_heap_source(ins, exempt=True, assume_old_defrag=False, - assume_boxes_in_gc_heap=False): +def classify_heap_source(ins, exempt=True, assume_boxes_in_gc_heap=False): """`(is_heap_source, is_hazardous, exemption_or_None)` for one instruction. `is_hazardous` is the reportable property: the object can move, or can be @@ -2793,8 +2679,7 @@ def classify_heap_source(ins, exempt=True, assume_old_defrag=False, return (False, False, None) if not exempt: return (True, True, None) - assumed = {"assume_old_defrag": assume_old_defrag, - "assume_boxes_in_gc_heap": assume_boxes_in_gc_heap} + assumed = {"assume_boxes_in_gc_heap": assume_boxes_in_gc_heap} for src in IMMOVABLE_SOURCES: if src.matches(ins): if assumed.get(src.knob): @@ -3339,14 +3224,13 @@ def native_heap_source_kind(ins, token_callee): def native_immovable_exemption(ins, effective_callee, source_opts): """The `IMMOVABLE_SOURCES` entry that exempts this source, if any. - The #7210 adjudication is about the ALLOCATOR, not about the lowering: the - class-keys array is old-arena and the keys global is a registered root - under statepoints exactly as under the shadow stack, and a box is still - `std::alloc::alloc`. Re-deriving that judgement here would be the + The #7210 adjudication is about the ALLOCATOR, not about the lowering: a + box is still `std::alloc::alloc` under statepoints exactly as under the + shadow stack. Re-deriving that judgement here would be the `REWRITTEN_LOAD_RE` mistake again — two modes with two answers to one - question, of which the narrower was wrong (#7240). Measured before this - was wired: 40 of 137 `unrooted` hits on the 21-module corpus were - `@perry_class_keys_*`, i.e. the population #7210 already dismissed. + question, of which the narrower was wrong (#7240). Class-key arrays used + to share this exemption, but default old-page relocation makes them + movable; their cached copies are now ordinary mutable roots (#7876). The premises are gated by `--audit-immovable-sources`, which runs in CI ahead of both corpora, and each exemption's knob turns it back off. @@ -4482,24 +4366,14 @@ def statepoint_self_test(): "function reports clean.", file=sys.stderr) ok = False - # --- the exemptions carry over from #7210 ------------------------- - # An exemption that fires in one mode and not the other is the - # REWRITTEN_LOAD_RE divergence again, and #7240 is what that costs. + # Old-page relocation makes the class-keys source movable. A bare + # cached copy must therefore be reported in the native lowering too. ck = os.path.join(td, "sp_class_keys.ll") with open(ck, "w") as fh: fh.write(_SELFTEST_SP_CLASS_KEYS) - if _scan_statepoints([ck], moving_only=True): - print("self-test FAIL: the @perry_class_keys_* source is exempt " - "under #7210 (old-arena, registered root) and the statepoint " - "mode must honour the same adjudication the alloca mode " - "does. 1162 of this corpus's sources are that shape.", - file=sys.stderr) - ok = False - if len(_scan_statepoints([ck], moving_only=True, - assume_old_defrag=True)) != 1: - print("self-test FAIL: --assume-old-defrag must make the " - "class-keys fixture report again. An exemption with no " - "off-switch is a condition nobody can re-check.", + if len(_scan_statepoints([ck], moving_only=True)) != 1: + print("self-test FAIL: a bare @perry_class_keys_* copy must be " + "reported now that old-page relocation ships enabled.", file=sys.stderr) ok = False @@ -5071,14 +4945,13 @@ def self_test(): f"got {len(found)}", file=sys.stderr) ok = False - # --- movability vs rewritability (#7210), all four directions ------- + # --- movability vs rewritability (#7210) ----------------------------- # # An exemption is a suppression, so it gets the strictest treatment in - # the file: it must FIRE (or the 66 false positives come back), it must - # be exactly REVERSIBLE by its knob (or the counterfactual in #7210 is - # unrecheckable), it must not suppress a genuine nursery hazard of the - # SAME SHAPE (or it has become a code-shape rule instead of an - # allocator rule), and its premises must still hold in the tree. + # the file: it must FIRE, be exactly REVERSIBLE by its knob, avoid a + # genuine nursery hazard of the same shape, and retain its source-tree + # premises. The former class-keys fixture now asserts the opposite: + # default old-page relocation makes a bare copy reportable. ck = os.path.join(td, "exempt_class_keys.ll") bx = os.path.join(td, "exempt_box.ll") nz = os.path.join(td, "nursery_hazard.ll") @@ -5088,8 +4961,7 @@ def self_test(): with open(p, "w") as fh: fh.write(text) - for path, knob, label in ((ck, "assume_old_defrag", "class-keys"), - (bx, "assume_boxes_in_gc_heap", "box")): + for path, knob, label in ((bx, "assume_boxes_in_gc_heap", "box"),): found, n_allocas = _scan_unrooted([path], moving_only=True) if found: print(f"self-test FAIL: the {label} exemption fixture must " @@ -5120,6 +4992,14 @@ def self_test(): "a condition nobody can re-check.", file=sys.stderr) ok = False + found, n_allocas = _scan_unrooted([ck], moving_only=True) + if len(found) != 1 or n_allocas != 1: + print("self-test FAIL: a bare class-keys cache must be a moving " + f"hazard with old-page relocation enabled (got {len(found)} " + f"report(s), {n_allocas} alloca(s)).", + file=sys.stderr) + ok = False + # ★ The planted genuine hazard. Structurally identical to the # class-keys fixture — same slot type, same load below the same call, # same consumer — differing only in the ALLOCATOR. @@ -5136,11 +5016,10 @@ def self_test(): "as MOVING, or --moving-only would drop it and the " "acceptance proof would be vacuous", file=sys.stderr) ok = False - found, _ = _scan_unrooted([nz], assume_old_defrag=True, - assume_boxes_in_gc_heap=True) + found, _ = _scan_unrooted([nz], assume_boxes_in_gc_heap=True) if len(found) != 1: print("self-test FAIL: the planted nursery hazard must be reported " - "under BOTH knobs too — the knobs only ever widen", + "under the exemption knob too — knobs only ever widen", file=sys.stderr) ok = False found, _ = _scan_unrooted([nz], moving_only=True) @@ -5167,7 +5046,7 @@ def self_test(): # Every exemption must be reachable through its knob, or it is a dead # switch. Asserted structurally rather than per-entry so a new # exemption cannot be added without one. - knobs = {"assume_old_defrag", "assume_boxes_in_gc_heap"} + knobs = {"assume_boxes_in_gc_heap"} for src in IMMOVABLE_SOURCES: if src.knob not in knobs: print(f"self-test FAIL: exemption {src.key!r} declares knob " @@ -5477,11 +5356,9 @@ def main(): "came from. Takes no corpus.") ap.add_argument("--audit-immovable-sources", action="store_true", help="re-check the PREMISES of every --unrooted-allocas " - "exemption against the runtime source (#7210): the " - "class-keys array is still old-arena, old-page defrag " - "is still off by default, the keys global is still a " - "registered root, and boxes are still outside the GC " - "heap and never freed. An exemption whose premise has " + "exemption against the runtime source (#7210): boxes " + "are still outside the GC heap and never freed. An " + "exemption whose premise has " "quietly lapsed reads as a triaged false positive and " "is a live hazard. Takes no corpus.") ap.add_argument("--statepoints", action="store_true", @@ -5511,11 +5388,6 @@ def main(): "because the residual is a population under triage, " "not a list of adjudicated sites -- the same call " "--max-stale makes. It can only be lowered.") - ap.add_argument("--assume-old-defrag", action="store_true", - help="treat old-arena objects (the @perry_class_keys_* " - "cache) as MOVABLE, i.e. answer #7210's first " - "counterfactual: what --unrooted-allocas reports if " - "PERRY_GC_OLD_DEFRAG=1 ever ships on. Widens only.") ap.add_argument("--assume-boxes-in-gc-heap", action="store_true", help="treat js_box_alloc* results as GC-heap objects, i.e. " "#7210's second counterfactual: what " @@ -5579,10 +5451,9 @@ def main(): "--unrooted-allocas; it is honoured by the bind-anchored " "default and by --statepoints only") # Same disarmed-knob rule. The exemptions live in the unrooted-alloca pass - # only, so `--assume-old-defrag` on any other pass would read like a + # only, so the assumption knob on any other pass would read like a # widening and do nothing at all. - for flag, val in (("--assume-old-defrag", ns.assume_old_defrag), - ("--assume-boxes-in-gc-heap", ns.assume_boxes_in_gc_heap)): + for flag, val in (("--assume-boxes-in-gc-heap", ns.assume_boxes_in_gc_heap),): if val and not (ns.unrooted_allocas or ns.statepoints): ap.error(f"{flag} requires --unrooted-allocas or --statepoints " "(they are the passes that carry the exemptions)") @@ -5714,8 +5585,7 @@ def funcs_floor_violated(): rc = run_statepoints( parsed, poll_reaching, verbose, moving_only, allowlist, ns.max_stale, ns.max_unrooted, - source_opts={"assume_old_defrag": ns.assume_old_defrag, - "assume_boxes_in_gc_heap": ns.assume_boxes_in_gc_heap}) + source_opts={"assume_boxes_in_gc_heap": ns.assume_boxes_in_gc_heap}) # --- can this gate still fail? ----------------------------------- # # UNCONDITIONALLY, not after an early `return rc`. The bind-anchored @@ -5758,8 +5628,7 @@ def funcs_floor_violated(): per_fn = defaultdict(int) out = [] n_allocas = 0 - source_opts = {"assume_old_defrag": ns.assume_old_defrag, - "assume_boxes_in_gc_heap": ns.assume_boxes_in_gc_heap} + source_opts = {"assume_boxes_in_gc_heap": ns.assume_boxes_in_gc_heap} exempt_counts = defaultdict(int) for mod, fs in parsed: for f in fs: diff --git a/scripts/gc_runtime_root_holders.json b/scripts/gc_runtime_root_holders.json index fcc275bb3b..c4a4f510ee 100644 --- a/scripts/gc_runtime_root_holders.json +++ b/scripts/gc_runtime_root_holders.json @@ -16,10 +16,10 @@ " not_a_gc_pointer - the stored value is an id, a counter, an epoch, a code address, a", " .rodata object, or Rust-owned state. Nothing for the collector.", " test_only - #[cfg(test)] storage; never live in a shipped binary.", - " open_gap - a real unrooted GC pointer. `issue` says where it is tracked.", - " unverified - enumerated, verdict NOT established. A named, dated TODO \u2014 not an", - " exemption. Keep this list short; every entry here is a hole the", - " gate can see and nobody has looked into." + " open_gap - a real unrooted GC pointer. `issue` says where it is tracked; this", + " verdict FAILS while old-page relocation ships enabled.", + " unverified - enumerated, verdict NOT established. This also FAILS: an unknown", + " movable-address contract cannot be a production exemption." ], "holders": [ { @@ -40,13 +40,6 @@ "verdict": "not_a_gc_pointer", "why": "Monotonic id counter for CP_LIVE keys. CP_LIVE itself is covered by cp_reactor_scan_roots_mut." }, - { - "file": "crates/perry-runtime/src/json/mod.rs", - "name": "PARSE_KEY_RING", - "verdict": "open_gap", - "issue": "#7231", - "why": "A hot-key mirror of PARSE_KEY_CACHE holding the same *const StringHeader values. The CACHE is visited by scan_parse_roots_mut; the RING is not, so a move rewrites one copy and not the other and a ring hit hands out a pre-move address. Narrow: the keys are js_string_from_bytes_longlived, i.e. old-gen, so only old-gen defrag can move them \u2014 not the copying minor." - }, { "file": "crates/perry-runtime/src/node_submodules/blob.rs", "name": "FILE_BLOB_STREAMS", @@ -59,12 +52,6 @@ "verdict": "not_a_gc_pointer", "why": "Monotonic id counter." }, - { - "file": "crates/perry-runtime/src/node_submodules/diagnostics.rs", - "name": "DIAG_CHANNEL_BY_KEY", - "verdict": "unverified", - "why": "Values are channel handle ids (safe), but DiagChannelKey::Symbol carries a symbol ADDRESS. Whether that address can be a gc_malloc'd fresh symbol (alloc_symbol, GC_TYPE_STRING, movable) rather than one of the Box::leak'd registered/well-known symbols has not been established. Raised 2026-08-09 with #7231's sweep; needs the write sites read." - }, { "file": "crates/perry-runtime/src/node_submodules/diagnostics.rs", "name": "DIAG_CHANNELS", @@ -141,13 +128,6 @@ "scanner": "object::scan_object_cache_roots_mut (object/mod.rs:1141)", "why": "One of the six %IteratorPrototype%-style singletons visited by the same loop in object/mod.rs; see ITERATOR_PROTOTYPE_PTR for the full reasoning." }, - { - "file": "crates/perry-runtime/src/perf_hooks.rs", - "name": "PERF_ENTRY_KEYS_ARRAY", - "verdict": "open_gap", - "issue": "#7231", - "why": "Caches the shared keys_array address of performance entries (js_object_alloc_with_shape, nursery) and compares it by identity at perf_hooks.rs:119. Never rewritten, so after a move it misses (silent slow path) or \u2014 the sharper half \u2014 matches a newly-allocated array recycled into the address. The neighbouring PERF_ENTRIES table IS covered by scan_perf_entries_roots_mut; this one slot is not." - }, { "file": "crates/perry-runtime/src/process.rs", "name": "MODULE_LOADER_NEXT_RESOLVE", diff --git a/scripts/gc_runtime_root_holders.py b/scripts/gc_runtime_root_holders.py index 044935d631..87babc9e9a 100755 --- a/scripts/gc_runtime_root_holders.py +++ b/scripts/gc_runtime_root_holders.py @@ -47,6 +47,8 @@ * a new uncovered holder with no inventory entry -> exit 1 * an inventory entry that no longer matches a declaration -> exit 1 +* an `open_gap` or `unverified` verdict -> exit 1; old-page relocation ships + enabled, so a known or unevaluated movable-address holder cannot be exempted * fewer than MIN_HOLDERS declarations matched -> exit 2, because a regex that stopped matching would otherwise report a clean, empty, green run * fewer than MIN_REGISTERED registered scanners found -> exit 2, same reason: @@ -467,6 +469,12 @@ def inventory_problems(inventory: list[dict]) -> list[str]: ) if verdict == "open_gap" and not (entry.get("issue") or "").strip(): problems.append(f"{label}: open_gap must cite an `issue`") + if verdict in {"open_gap", "unverified"}: + problems.append( + f"{label}: `{verdict}` is not a shippable old-page relocation verdict. " + "Rewrite/invalidate the holder, prove it cannot hold a movable GC address, " + "or keep relocation disabled." + ) if verdict == "unverified": unverified += 1 if unverified > MAX_UNVERIFIED: From f9f6bf38636cd0d05be56b38314a53fbca4cf95a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Wed, 12 Aug 2026 07:27:54 +0200 Subject: [PATCH 2/2] docs(changelog): record old-page defrag fix --- changelog.d/7913-old-defrag-contract.md | 32 +++++++++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100644 changelog.d/7913-old-defrag-contract.md diff --git a/changelog.d/7913-old-defrag-contract.md b/changelog.d/7913-old-defrag-contract.md new file mode 100644 index 0000000000..acd111dfb5 --- /dev/null +++ b/changelog.d/7913-old-defrag-contract.md @@ -0,0 +1,32 @@ +### Fixed + +- Restored safe old-generation page defragmentation and enabled it by default. + A minor trace does not prove unmarked old objects dead, so evacuation now + snapshots every indexed occupant of a selected source block, rejects the + block before forwarding if any occupant cannot move, and otherwise relocates + the complete block before reclaiming it. `PERRY_GC_OLD_DEFRAG=0` remains an + explicit diagnosis and rollback switch. + +- Closed three runtime rewrite gaps: JSON's parse-key ring and the perf entry + keys cache now follow their structurally rooted owners, while diagnostics + symbol-keyed state is rekeyed after a move. Cached `@perry_class_keys_*` + copies are precise function-lifetime mutable roots rather than relying on the + old generation being immovable. The runtime-holder inventory now fails on + known or unevaluated movable-address gaps. + +The historical corruption workload fails on rebuilt main when old-page +relocation is enabled, but passed six consecutive verified runs after this +change with output identical to the clean control. A mixed-size regression +also demonstrates that a fragmented source block is actually released. + +### Performance + +Old-page metadata selection now runs only after the copying-minor fast path has +declined the collection. This removed an O(old pages) charge from ordinary +copying minors: the worst affected retention benchmark moved from +8.29% in the +initial implementation to +0.14% best / -0.09% median in the final quiet-M1 +best-of-15 comparison. + +Precise class-key roots cost 3.70-4.45% in four tight shape/class allocation +kernels. That is the remaining correctness tradeoff; `interp` improves 2.85%, +and the other 20 programs remain within +/-1.5%.