diff --git a/changelog.d/7250-gc-lazy-barrier-arming.md b/changelog.d/7250-gc-lazy-barrier-arming.md new file mode 100644 index 0000000000..dda188d508 --- /dev/null +++ b/changelog.d/7250-gc-lazy-barrier-arming.md @@ -0,0 +1,94 @@ +**perf(gc): the write barrier stops maintaining a remembered set nothing has read yet (#7187 Phase A).** +`classify_heap_generation` is the single largest symbol in `batch.ts` — 19.03% of +the program, 657M instructions, 2.3× the next symbol — and #7170 measured it +**with zero collections running**. It is reached from the write barrier, on the +sort's own element stores, maintaining a remembered set whose only consumer is a +collection that never happens. The remembered-set half of the barrier now starts +**unarmed** and returns immediately, before the parent decode; it arms on the +first *read*. + +Measured on `batch.ts` (macOS arm64, release, existing `PERRY_GC_TRACE` counters, +base `8b024958f`): the program makes 2,265,777 barrier calls; 280,536 exit at the +#6011 non-pointer-child fast path, which is untouched; the remaining **1,985,241 +(87.6%)** reached the remembered-set half and now do not. That deletes +**5,532,729 `classify_heap_generation` calls** (211,497 × 1 on the +parent-not-old exit, 1,773,744 × 3 — parent, child, slot — on the slow path) and +**1,773,744 `mark_dirty_old_page` calls**, of which only 517 (0.029%) ever +produced a new dirty page. Collections on `batch.ts`: zero, so all of it was +skippable. + +**The invariant, and why the obvious version of it is wrong.** "Nothing has +collected, so there is no old generation and no edge to record" is false, and +believing it reproduces a bug this repo has already had: `arena_alloc_gc` routes +any allocation over `LARGE_OBJECT_THRESHOLD_BYTES` straight to +`arena_alloc_gc_old`, so old-gen exists from the first large allocation. +`batch.ts` is exactly that case — its 40 000-element arrays are born old while +every record they hold is nursery, making every element move a genuine old→young +edge. `note_array_slot_layout_only`'s comment in `array/header.rs` is the +postmortem of the last time this was got wrong ("155 edges, all born-old +array→young object; the store never hit any barrier"). So the contract is stated +on the *reading* side: **while unarmed the barrier records nothing, and in +exchange the first read of the remembered set on a thread does not trust the log +at all — it reconstructs the complete old→young edge set from the heap and arms +the barrier on the way past.** That is enforceable because +`remembered_dirty_snapshot()` is the sole read path — the budgeted/full cycle's +`RememberedSetRootMarkState`, the copying nursery fast path and its preflight, +the cycle's pre-clear coverage snapshot and the evacuation verifier all obtain +the dirty set there, and the only other touches of `DIRTY_OLD_PAGES` / +`EXTERNAL_DIRTY_SLOT_PAGES` / `REMEMBERED_SET` are the barrier's own writes, the +clear, and diagnostics (the arena's per-page `dirty` bit feeds `old_page_summary` +telemetry only). Arming inside that one function means no collector can observe +an unarmed, and therefore empty, log, and the #5029 verifier's predicate is +unchanged. Two orderings are load-bearing: **arm first, walk second** (a store +landing mid-walk must be logged, because the walk may already have passed its +parent), and the check sits **after** the child prologue (so the #6011 numeric +fast path pays nothing new and SATB shading is never skipped) and **before** the +parent decode (so the unarmed window also skips `decode_heap_addr`'s +raw-pointer arm, itself a `classify_heap_generation`). The flag is a global +`AtomicBool` rather than a thread-local — arming is monotone and conservative in +the early direction, while `thread_local!` costs a `_tlv_get_addr` call on macOS +on a per-heap-store path — while "has *this* thread reconstructed" stays +thread-local, so a worker armed early by another thread's collection still +reconstructs on its own first read. + +**The gate asserts its subject was live** (`gc/tests/barrier_arming.rs`), over a +born-old parent holding a nursery child: (1) the unarmed barrier logged nothing +for a real old→young edge — that skip *is* the lever; (2) the first read +reconstructed exactly once, recovered the exact page the barrier skipped, and the +collector marked the child through it; (3) a **sabotage arm** proves that with +the reconstruct suppressed the same edge is uncovered and the child ends up +unmarked. Both mutations were verified to turn the suite red. No new env knob. +Census under the existing `PERRY_GC_TRACE` surface: +`write_barrier.unarmed_skips`, `armed`, `reconstructs`, +`reconstruct_recovered_{old,external}_pages`. + +**End-to-end A/B on the built binaries** (same target dir, artifacts snapshotted +per arm, runtime archive md5s confirmed different). The branch's own census on +`batch.ts` reports `unarmed_skips = 1,985,241` — the base's +`calls − non_pointer_child_skips` to the unit — with `old_to_young_slow_hits`, +`parent_not_old_skips` and `dirty_page_mark_attempts` all at **0**, and +`reconstructs = 1` recovering 393 old pages (fewer than the 517 the barrier had +logged: the reconstruct records only edges live at collection time, so it is +strictly more precise). A probe that collects *before* the sort and again at the +end reports the armed steady state **counter-for-counter identical** across arms +(2,265,777 / 280,536 / 211,497 / 1,773,744 / 1,773,744 / 517), confirming the +change is a pure pre-collection skip. Compiled-binary size **−32,936 bytes +(−0.25%)**, so no size trade to state. Max RSS unchanged (24.3 MB both arms) — +the retained remembered set is ~517 page numbers, invisible at this scale, so +the RSS argument for lazy arming does not show up on this workload. 29 GC/repsel +gap tests byte-identical across arms; `cargo test -p perry-runtime --lib` +1639 passed / 0 failed single-threaded. + +Also recorded, because the campaign brief expected otherwise: the +`Object.defineProperty(require, 'name', …)` preamble in +`compile/cjs_wrap/wrap.rs` contributes **0%** of this cost. The "barrier" it arms +is the compile-time `Ptr` promotion barrier (`ptr_shape_report.rs`'s +`MODULE_BARRIER` / rule 5, which `collectors/cjs_scaffolding.rs` exempts), not +the GC write barrier; `wrap.rs` contains no write-barrier references, and +`batch.ts` is never CJS-wrapped. Rank 1 is 100% intrinsic runtime barrier. + +Phase A only: it removes the pre-collection cost, which on a program that never +collects — most of what Perry compiles — is the whole barrier. Capping the armed +steady state is #7187's B3/B4, unstarted; the 12.91% layout side-table (#5094, +#6759) is untouched by design, since `layout_note_slot` sits outside the barrier +at every call site. diff --git a/crates/perry-runtime/src/gc/barrier.rs b/crates/perry-runtime/src/gc/barrier.rs index 55bd79dde8..9f561fa6e6 100644 --- a/crates/perry-runtime/src/gc/barrier.rs +++ b/crates/perry-runtime/src/gc/barrier.rs @@ -8,7 +8,23 @@ pub(super) struct RememberedDirtySnapshot { pub(super) fallback_headers: Vec, } +/// The **sole read path** for the remembered set. +/// +/// Every collector obtains the dirty set here: the budgeted/full cycle's +/// `RememberedSetRootMarkState::new`, the copying nursery fast path and its +/// preflight, the cycle's pre-clear coverage snapshot, and the evacuation +/// verifier. The barrier *writes* `DIRTY_OLD_PAGES` / +/// `EXTERNAL_DIRTY_SLOT_PAGES` / `REMEMBERED_SET`; `remembered_set_clear` +/// empties them; nothing else reads them for collection decisions. That is +/// what lets #7187's lazy arming be sound by construction rather than by +/// audit: arming the barrier here means no collector can observe an unarmed, +/// and therefore empty, log. +/// +/// If a future collector reads those thread-locals directly instead of coming +/// through here, it must call +/// [`arm_and_reconstruct_remembered_set_if_unarmed`] itself. pub(super) fn remembered_dirty_snapshot() -> RememberedDirtySnapshot { + arm_and_reconstruct_remembered_set_if_unarmed(); let dirty_old_pages: crate::fast_hash::PtrHashSet = DIRTY_OLD_PAGES.with(|s| s.borrow().iter().copied().collect()); let external_dirty_entries: Vec<(usize, usize)> = EXTERNAL_DIRTY_SLOT_PAGES.with(|s| { @@ -899,6 +915,7 @@ pub(super) fn bump_write_barrier_trace_counter(counter: BarrierTraceCounter) { BarrierTraceCounter::ConservativeParentSpanMarks => { counters.conservative_parent_span_marks += 1; } + BarrierTraceCounter::UnarmedSkips => counters.unarmed_skips += 1, } cell.set(counters); }); @@ -967,6 +984,9 @@ pub(super) fn write_barrier_slot_inner( let Some(child_addr) = barrier_child_prologue(child) else { return; }; + if !barrier_remembering_active() { + return; + } // Decode the parent — must be a NaN-boxed heap pointer. let parent_addr = decode_heap_addr(parent); if parent_addr == 0 { @@ -992,6 +1012,33 @@ fn barrier_child_prologue(child: u64) -> Option { Some(child_addr) } +/// #7187: should this barrier call do remembered-set work at all? +/// +/// Placed **after** [`barrier_child_prologue`] and **before** the parent +/// decode, in every entry point. Both halves of that placement are +/// load-bearing: +/// +/// * After the prologue, so the #6011 fast path (any number stored into any +/// slot — the overwhelmingly common store) pays literally nothing new, and +/// so SATB/insertion shading for an in-progress incremental cycle is never +/// skipped. An incremental cycle implies a collection has run implies +/// armed, so this could not bite today; writing the order down keeps a +/// later refactor from hoisting the check above the shading. +/// * Before the parent decode, so the unarmed window also skips +/// `decode_heap_addr`'s raw-pointer arm — itself a +/// `classify_heap_generation` on the bare-`u64` entry point. +/// +/// Cost once armed: one relaxed load of a `static` (`adrp`/`ldr`) plus a +/// perfectly-predicted, permanently-taken branch. +#[inline] +fn barrier_remembering_active() -> bool { + if barrier_remembering_armed() { + return true; + } + bump_write_barrier_trace_counter(BarrierTraceCounter::UnarmedSkips); + false +} + /// [`write_barrier_slot_inner`] for a caller that already holds the parent as /// a plain GC user pointer — see [`write_barrier_decoded_parent`] for why the /// `u64` round-trip is worth avoiding (#7187). @@ -1004,6 +1051,9 @@ pub(super) fn write_barrier_slot_decoded( let Some(child_addr) = barrier_child_prologue(child) else { return; }; + if !barrier_remembering_active() { + return; + } // The NaN-box round-trip this replaces was also FILTERING, not just // decoding, and dropping the filter is a segfault rather than a wrong // answer: `barrier_parent_needs_remembering` reaches diff --git a/crates/perry-runtime/src/gc/barrier_arming.rs b/crates/perry-runtime/src/gc/barrier_arming.rs new file mode 100644 index 0000000000..8904f28c2e --- /dev/null +++ b/crates/perry-runtime/src/gc/barrier_arming.rs @@ -0,0 +1,193 @@ +//! #7187 Phase A — lazy write-barrier arming. +//! +//! Split out of `barrier.rs`, which is at the 2 000-line cap +//! `scripts/check_file_size.sh` enforces. + +use super::*; + +/// #7187 Phase A — is the remembered-set half of the write barrier armed? +/// +/// The remembered set has exactly one consumer: a collection, and every +/// collector reaches it through [`remembered_dirty_snapshot`] (see that +/// function for why that is the *sole* read path). Until the first such read, +/// maintaining the log buys nothing — and on `batch.ts` it is the program's +/// single largest cost: #7170 measured `classify_heap_generation` at 19.03% +/// (657M instructions) with **zero collections running**, reached from this +/// barrier on the sort's own element stores. +/// +/// So the log starts unarmed and records nothing. The exchange, and the whole +/// soundness argument, lives on the *reading* side: the first snapshot on a +/// thread does not trust the log at all — it reconstructs the complete +/// old→young edge set from the heap +/// ([`arm_and_reconstruct_remembered_set_if_unarmed`]) and arms the barrier on +/// the way past. From then on the barrier maintains it incrementally exactly +/// as before. +/// +/// **The naive premise "nothing has collected, so there is no old generation" +/// is false and is deliberately not used here.** `arena_alloc_gc` routes any +/// allocation over `LARGE_OBJECT_THRESHOLD_BYTES` straight to +/// `arena_alloc_gc_old`, so old-gen exists from the first large allocation — +/// `batch.ts`'s own 40 000-element arrays are born old while every record they +/// hold is nursery. `note_array_slot_layout_only`'s comment in +/// `array/header.rs` is the postmortem of the last time that was got wrong +/// ("155 edges, all born-old array→young object"). The reconstruct, not the +/// flag, is what makes this sound. +/// +/// A process-global `AtomicBool` rather than a thread-local: arming is +/// monotone and one-way, and conservative in the early direction (a thread +/// that arms early merely stops being free), while a `thread_local!` costs a +/// `_tlv_get_addr` *call* on macOS on a path that runs on every heap store. +/// Per-thread state — "has *this* thread reconstructed yet" — stays in +/// [`REMEMBERED_SET_RECONSTRUCTED`], where a wrong answer would matter. +/// +/// **In `cfg(test)` builds this starts ARMED**, and so does +/// [`REMEMBERED_SET_RECONSTRUCTED`]. The runtime's existing barrier unit tests +/// assert the armed steady state — they hand-build an old parent and a young +/// child and expect the store to be logged — and they are right to: that is +/// the state every program is in after its first collection. Letting them +/// observe the unarmed window instead would have deleted their subject +/// wholesale, and made the suite order-dependent besides (the first test that +/// collects arms the process for every later test). The unarmed window gets +/// dedicated tests that opt back out via [`reset_barrier_arming_for_tests`], +/// plus end-to-end coverage from every compiled binary the parity/gap suites +/// run — those start unarmed for real. +static BARRIER_REMEMBERING_ARMED: std::sync::atomic::AtomicBool = + std::sync::atomic::AtomicBool::new(cfg!(test)); + +#[inline] +pub(super) fn barrier_remembering_armed() -> bool { + // The unarmed-window tests must not disarm the barrier for every OTHER test + // running concurrently — libtest gives each test its own thread but shares + // the process, and a global disarm made two `gc::tests::teardown` cases + // fail under the default parallel run. The override is thread-local so the + // window is opened only for the thread that asked for it. Production is a + // single relaxed load of the `static`; this arm compiles away entirely. + #[cfg(test)] + if let Some(forced) = TEST_ARMED_OVERRIDE.with(Cell::get) { + return forced; + } + BARRIER_REMEMBERING_ARMED.load(Ordering::Relaxed) +} + +/// What the unarmed-window reconstruct recovered. The #7187 gate asserts on +/// these rather than on "nothing crashed": a reconstruct that recovers no +/// edges on a workload built from born-old parents means the walk never ran or +/// never looked, which is precisely the `PERRY_GC_FORCE_EVACUATE` failure mode +/// (#6942/#6946) — a green test whose subject was inert. +#[derive(Clone, Copy, Default, Debug, Eq, PartialEq)] +pub(super) struct RememberedReconstructCensus { + /// Reconstructs run on this thread: 0 (never collected) or 1. + pub(super) reconstructs: u64, + /// Old-gen pages re-derived from the heap — the old→young edges the + /// unarmed barrier deliberately did not log. + pub(super) recovered_old_pages: u64, + /// External (malloc-backed) slot-page entries re-derived from the heap. + pub(super) recovered_external_pages: u64, +} + +impl RememberedReconstructCensus { + pub(super) const fn zero() -> Self { + Self { + reconstructs: 0, + recovered_old_pages: 0, + recovered_external_pages: 0, + } + } +} + +thread_local! { + /// Has this thread already reconstructed its remembered set from the heap? + /// Set by the first [`remembered_dirty_snapshot`] on the thread. Threads + /// are independent here: [`BARRIER_REMEMBERING_ARMED`] going true because + /// some *other* thread collected does not excuse this thread from its own + /// reconstruct, because this thread's stores before that moment were never + /// logged. The two flags are a superset and a no-op respectively, so there + /// is no window in which an edge is neither logged nor reconstructed. + static REMEMBERED_SET_RECONSTRUCTED: Cell = const { Cell::new(cfg!(test)) }; + + static RECONSTRUCT_CENSUS: Cell = + const { Cell::new(RememberedReconstructCensus::zero()) }; + + /// Test-only per-thread override of [`BARRIER_REMEMBERING_ARMED`]. See + /// [`barrier_remembering_armed`] for why the unarmed-window tests must not + /// reach for the global. + #[cfg(test)] + static TEST_ARMED_OVERRIDE: Cell> = const { Cell::new(None) }; +} + +pub(super) fn remembered_reconstruct_census() -> RememberedReconstructCensus { + RECONSTRUCT_CENSUS.with(Cell::get) +} + +/// Arm the barrier and rebuild this thread's remembered set from the heap, if +/// that has not happened yet. Called by [`remembered_dirty_snapshot`] — i.e. +/// on the read side, before any collector observes the log. +/// +/// Ordering is load-bearing: **arm first, walk second.** A store that lands +/// while the walk is in flight must be logged, because the walk may already +/// have passed its parent. Arming first makes the two coverages overlap; the +/// reverse order leaves a hole. +pub(super) fn arm_and_reconstruct_remembered_set_if_unarmed() { + if REMEMBERED_SET_RECONSTRUCTED.with(Cell::get) { + return; + } + // Set before the walk: the walk touches layout side tables and GC rewrite + // hooks, and must not be able to recurse into a second reconstruct. + REMEMBERED_SET_RECONSTRUCTED.with(|cell| cell.set(true)); + BARRIER_REMEMBERING_ARMED.store(true, Ordering::Relaxed); + // A thread that opened the unarmed window for itself has now closed it: + // drop the override so `barrier_remembering_armed` reads the real flag. + #[cfg(test)] + TEST_ARMED_OVERRIDE.with(|cell| cell.set(None)); + + // `require_marked = false`: nothing is marked yet when a collector first + // asks for the log, so the walk must consider every retained old parent. + // That over-approximates (a dead-but-unswept old parent's young children + // are kept one extra cycle) in the same direction the barrier itself + // over-approximates — an edge logged while the parent was alive also + // outlives the parent's death. + let sticky = super::verify::rebuild_minor_old_to_young_remembered_set(); + let recovered_old_pages = sticky.old_pages.len() as u64; + let recovered_external_pages = sticky.external_pages.len() as u64; + sticky.restore(); + + RECONSTRUCT_CENSUS.with(|cell| { + let mut census = cell.get(); + census.reconstructs = census.reconstructs.saturating_add(1); + census.recovered_old_pages = census + .recovered_old_pages + .saturating_add(recovered_old_pages); + census.recovered_external_pages = census + .recovered_external_pages + .saturating_add(recovered_external_pages); + cell.set(census); + }); +} + +/// Test-only: reopen the process-start unarmed window **for this thread only**, +/// so a test can exercise it deliberately. Production has no way back — arming +/// is one-way and global. Pair with [`close_barrier_arming_window_for_tests`], +/// or the override outlives the test if libtest reuses the thread. +#[cfg(test)] +pub(super) fn reset_barrier_arming_for_tests() { + TEST_ARMED_OVERRIDE.with(|cell| cell.set(Some(false))); + REMEMBERED_SET_RECONSTRUCTED.with(|cell| cell.set(false)); + RECONSTRUCT_CENSUS.with(|cell| cell.set(RememberedReconstructCensus::zero())); +} + +/// Test-only: arm this thread's barrier WITHOUT a reconstruct. Only a test can +/// reach this state, and reaching it is the point — it is the sabotage arm that +/// proves the reconstruct is what recovers the unlogged edge. +#[cfg(test)] +pub(super) fn arm_barrier_for_tests() { + TEST_ARMED_OVERRIDE.with(|cell| cell.set(Some(true))); + REMEMBERED_SET_RECONSTRUCTED.with(|cell| cell.set(true)); +} + +/// Test-only: drop this thread's override and restore the suite-wide armed +/// default. Runs from the unarmed-window tests' RAII guard. +#[cfg(test)] +pub(super) fn close_barrier_arming_window_for_tests() { + TEST_ARMED_OVERRIDE.with(|cell| cell.set(None)); + REMEMBERED_SET_RECONSTRUCTED.with(|cell| cell.set(true)); +} diff --git a/crates/perry-runtime/src/gc/mod.rs b/crates/perry-runtime/src/gc/mod.rs index c92f85918d..17b9f68c4d 100644 --- a/crates/perry-runtime/src/gc/mod.rs +++ b/crates/perry-runtime/src/gc/mod.rs @@ -66,6 +66,8 @@ mod trace; pub(crate) use trace::*; mod barrier; pub use barrier::*; +mod barrier_arming; +pub(crate) use barrier_arming::*; mod copying; use copying::*; // The copied-minor pointer classifier is consumed by the weak-holder registry diff --git a/crates/perry-runtime/src/gc/telemetry.rs b/crates/perry-runtime/src/gc/telemetry.rs index ec41683930..c8fe66bb5c 100644 --- a/crates/perry-runtime/src/gc/telemetry.rs +++ b/crates/perry-runtime/src/gc/telemetry.rs @@ -482,6 +482,7 @@ pub(super) struct BarrierTraceCounters { pub(super) dirty_page_mark_attempts: u64, pub(super) new_dirty_pages: u64, pub(super) conservative_parent_span_marks: u64, + pub(super) unarmed_skips: u64, } impl BarrierTraceCounters { @@ -498,6 +499,7 @@ impl BarrierTraceCounters { dirty_page_mark_attempts: 0, new_dirty_pages: 0, conservative_parent_span_marks: 0, + unarmed_skips: 0, } } } @@ -515,6 +517,11 @@ pub(super) enum BarrierTraceCounter { DirtyPageMarkAttempts, NewDirtyPages, ConservativeParentSpanMarks, + /// #7187: a barrier call whose child WAS a heap pointer but which exited + /// before any remembered-set work because the barrier is not armed yet. + /// This is the count the lazy-arming lever removes; on a program that + /// never collects it equals `calls - non_pointer_child_skips`. + UnarmedSkips, } #[derive(Clone, Copy, Debug, Default, Eq, PartialEq)] @@ -984,6 +991,11 @@ impl GcCycleTrace { "retained_forwarded_stub_objects": self.sweep.retained_forwarded_stub_objects, "retained_forwarded_stub_bytes": self.sweep.retained_forwarded_stub_bytes, }); + // #7187 census. `armed` / `reconstructs` are what let the lazy-arming + // gate observe its own subject: a cycle reporting `unarmed_skips > 0` + // with `reconstructs == 0` would mean the reconstruct never ran and + // the collection is reading an incomplete log. + let reconstruct_census = crate::gc::remembered_reconstruct_census(); let write_barrier_json = serde_json::json!({ "calls": self.write_barrier.calls, "non_pointer_parent_skips": self.write_barrier.non_pointer_parent_skips, @@ -996,6 +1008,11 @@ impl GcCycleTrace { "dirty_page_mark_attempts": self.write_barrier.dirty_page_mark_attempts, "new_dirty_pages": self.write_barrier.new_dirty_pages, "conservative_parent_span_marks": self.write_barrier.conservative_parent_span_marks, + "unarmed_skips": self.write_barrier.unarmed_skips, + "armed": crate::gc::barrier_remembering_armed(), + "reconstructs": reconstruct_census.reconstructs, + "reconstruct_recovered_old_pages": reconstruct_census.recovered_old_pages, + "reconstruct_recovered_external_pages": reconstruct_census.recovered_external_pages, }); let trigger_json = serde_json::json!({ "kind": self.trigger_kind.as_str(), diff --git a/crates/perry-runtime/src/gc/tests/barrier_arming.rs b/crates/perry-runtime/src/gc/tests/barrier_arming.rs new file mode 100644 index 0000000000..52c948f156 --- /dev/null +++ b/crates/perry-runtime/src/gc/tests/barrier_arming.rs @@ -0,0 +1,225 @@ +//! #7187 Phase A — lazy write-barrier arming. +//! +//! Kept in its own file rather than appended to `barrier.rs`, which is already +//! at the 2 000-line cap `scripts/check_file_size.sh` enforces. + +use super::super::*; +use super::barrier::assert_heap_child_marked; +use super::support::*; + +// ── #7187 Phase A: lazy barrier arming ───────────────────────────────────── +// +// Three tests, deliberately shaped as a liveness triad rather than as "nothing +// threw". CLAUDE.md's failure mode #4 — the gate runs but its subject never did +// — is the one that has cost this repo the most (`PERRY_GC_FORCE_EVACUATE` was +// inert for every `gc()`-driven test for months, #6942/#6946). So each test +// asserts that the mechanism it covers was actually exercised: +// +// 1. the unarmed barrier really skipped (the log is empty for a real edge), +// 2. the reconstruct really ran and really recovered that edge (census +// counters, plus the child ends up marked through it), +// 3. suppressing the reconstruct really loses the edge — the sabotage arm, +// without which (1) and (2) would both pass on a fixture that never +// produced an old→young edge at all. + +/// Opens the unarmed window for THIS TEST'S THREAD ONLY and closes it on drop. +/// A global disarm would reach every concurrently-running test — measured: two +/// `gc::tests::teardown` cases went red under the default parallel run before +/// the override became thread-local. And closing on drop matters even so, +/// because libtest may reuse the thread for a later test. +struct UnarmedWindowGuard; + +impl UnarmedWindowGuard { + fn open() -> Self { + reset_barrier_arming_for_tests(); + Self + } +} + +impl Drop for UnarmedWindowGuard { + fn drop(&mut self) { + close_barrier_arming_window_for_tests(); + } +} + +/// The fixture the whole lever is about, and the shape of the bug it must not +/// reintroduce: a **born-old** parent (`arena_alloc_gc_old`, exactly what +/// `arena_alloc_gc` does for anything over `LARGE_OBJECT_THRESHOLD_BYTES` — +/// `batch.ts`'s 40 000-element arrays) holding a **nursery** child, stored +/// while the barrier has never been armed. `note_array_slot_layout_only`'s +/// comment records the last time this went wrong: "155 edges, all born-old +/// array→young object; the store never hit any barrier". +unsafe fn born_old_parent_with_young_child() -> (usize, *mut u64, usize) { + let young = crate::arena::arena_alloc_gc(40, 8, GC_TYPE_OBJECT) as usize; + let (old_obj, fields) = alloc_old_test_object(1); + *fields = ptr_bits(young); + js_write_barrier_slot(ptr_bits(old_obj as usize), fields as u64, ptr_bits(young)); + (old_obj as usize, fields, young) +} + +#[test] +fn test_7187_unarmed_barrier_logs_nothing_and_first_snapshot_reconstructs() { + let _guard = GcTestIsolationGuard::new(); + reset_remembered_set(); + clear_marks(); + let _window = UnarmedWindowGuard::open(); + + let (_old, fields, young) = unsafe { born_old_parent_with_young_child() }; + let slot_page = crate::arena::generation_page_for_addr(fields as usize); + + // (1) The subject was live: this is a genuine old→young edge, it went + // through the real barrier entry point, and the barrier logged NOTHING. + assert!( + !barrier_remembering_armed(), + "fixture must run inside the unarmed window" + ); + assert_eq!( + remembered_dirty_page_count(), + 0, + "an unarmed barrier must not maintain the remembered set — that skip \ + IS the lever, and a non-zero count here means it never engaged" + ); + assert!( + !old_page_dirty_for(slot_page), + "the written slot's old page must be undirtied while unarmed" + ); + assert_eq!(remembered_reconstruct_census().reconstructs, 0); + + // (2) The first read of the remembered set reconstructs what the barrier + // did not log, and the collector finds the child through it. + let valid_ptrs = build_valid_pointer_set(); + let stats = mark_remembered_set_roots(&valid_ptrs); + + let census = remembered_reconstruct_census(); + assert_eq!( + census.reconstructs, 1, + "the first remembered-set read must reconstruct exactly once" + ); + assert!( + census.recovered_old_pages >= 1, + "the reconstruct recovered no old→young pages at all — it walked \ + nothing, or looked for the wrong thing" + ); + assert!( + old_page_dirty_for(slot_page), + "the reconstruct must cover the exact page the unarmed store skipped" + ); + assert!( + stats.newly_marked >= 1, + "remembered-set root marking found no young children" + ); + assert_heap_child_marked(young as *const u8, "young child of born-old parent"); + + // (3) Arming went live: the next store is logged by the barrier itself, + // with no reconstruct involved. + assert!( + barrier_remembering_armed(), + "the first remembered-set read must arm the barrier" + ); + reset_remembered_set(); + clear_marks(); + let young2 = crate::arena::arena_alloc_gc(40, 8, GC_TYPE_OBJECT) as usize; + unsafe { + *fields = ptr_bits(young2); + } + js_write_barrier_slot(ptr_bits(_old), fields as u64, ptr_bits(young2)); + assert!( + old_page_dirty_for(slot_page), + "once armed, the barrier must log the edge without a reconstruct" + ); + assert_eq!( + remembered_reconstruct_census().reconstructs, + 1, + "arming must not re-run the reconstruct" + ); + + reset_remembered_set(); + clear_marks(); +} + +/// Sabotage arm. Same fixture, reconstruct suppressed — the edge must be LOST. +/// A version of the test above that passes with and without the reconstruct +/// would be testing nothing; this is what makes it testing something. +#[test] +fn test_7187_without_the_reconstruct_the_unarmed_edge_is_uncovered() { + let _guard = GcTestIsolationGuard::new(); + reset_remembered_set(); + clear_marks(); + let _window = UnarmedWindowGuard::open(); + + let (_old, fields, young) = unsafe { born_old_parent_with_young_child() }; + let slot_page = crate::arena::generation_page_for_addr(fields as usize); + assert_eq!(remembered_dirty_page_count(), 0); + + // Suppress the reconstruct by marking this thread as already-reconstructed + // WITHOUT having reconstructed — the one thing the production path can + // never do, and precisely the mistake a future refactor could make. + arm_barrier_for_tests(); + + let valid_ptrs = build_valid_pointer_set(); + let stats = mark_remembered_set_roots(&valid_ptrs); + + assert_eq!( + remembered_reconstruct_census().reconstructs, + 0, + "sabotage arm must actually suppress the reconstruct" + ); + assert!( + !old_page_dirty_for(slot_page), + "with the reconstruct suppressed the unarmed edge must stay uncovered \ + — if this page is dirty, something else is covering it and the \ + positive test above proves nothing" + ); + assert_eq!( + stats.newly_marked, 0, + "no young child should be reachable through an empty remembered set" + ); + unsafe { + let child_header = header_from_user_ptr(young as *const u8); + assert_eq!( + (*child_header).gc_flags & GC_FLAG_MARKED, + 0, + "the young child must be UNMARKED without the reconstruct — this \ + is the sweep-a-live-object bug the reconstruct prevents" + ); + } + + reset_remembered_set(); + clear_marks(); +} + +/// End-to-end through the real collection entry point rather than +/// `mark_remembered_set_roots` directly: a minor that begins in the unarmed +/// window must still see complete old→young coverage. +#[test] +fn test_7187_minor_in_the_unarmed_window_has_complete_old_young_coverage() { + let _guard = GcTestIsolationGuard::new(); + reset_remembered_set(); + clear_marks(); + let _window = UnarmedWindowGuard::open(); + + let (_old, _fields, _young) = unsafe { born_old_parent_with_young_child() }; + assert_eq!( + remembered_dirty_page_count(), + 0, + "the minor must start from an EMPTY log for this to be a real test" + ); + + let _freed = gc_collect_minor(); + + let census = remembered_reconstruct_census(); + assert_eq!(census.reconstructs, 1, "the minor must have reconstructed"); + assert!(census.recovered_old_pages >= 1); + assert!( + barrier_remembering_armed(), + "a collection must leave the barrier armed" + ); + let stats = verify_old_to_young_edges_covered(); + assert_eq!( + stats.missing_edges, 0, + "a minor started in the unarmed window left old→young edges uncovered" + ); + + reset_remembered_set(); + clear_marks(); +} diff --git a/crates/perry-runtime/src/gc/tests/mod.rs b/crates/perry-runtime/src/gc/tests/mod.rs index 6da4238116..cdc09d9acb 100644 --- a/crates/perry-runtime/src/gc/tests/mod.rs +++ b/crates/perry-runtime/src/gc/tests/mod.rs @@ -1,5 +1,6 @@ mod alloc; mod barrier; +mod barrier_arming; mod barrier_decoded_parent; mod budgeted_step_api; mod buffer_side_tables;