diff --git a/benchmarks/gc_ratchet/README.md b/benchmarks/gc_ratchet/README.md index 4d9322de36..79ae3404a9 100644 --- a/benchmarks/gc_ratchet/README.md +++ b/benchmarks/gc_ratchet/README.md @@ -122,6 +122,33 @@ error — the "probe ran no collection" rule deliberately lives in that the largest possible regression is not misdiagnosed as "your probe is too small". +### `PERRY_CONSERVATIVE_STACK_SCAN=full` is this gate's sensitivity arm — keep it + +CLAUDE.md's kill-policy says an unexercised mode gets deleted, and `=full` was a +clean candidate: it failed **134 of 1574** `perry-runtime` tests, a shipped +escape hatch nobody had verified (#7148). It was **kept** rather than deleted +for the reason this section documents — it is the only end-to-end proof that +`gc-ratchet` can fail. Deleting the mode would delete a gate's proof, which is +the same failure the kill-policy exists to prevent, one level up. + +It is verified instead. The 134 failures were not soundness failures: a +collector test's central assertion is *"this object should have been +collected"*, and an ambient conservative scan retains whatever the native stack +looks like a pointer to, so `=full` broke exactly the assertions the suite +exists to make. Since #7147 the test build has one declared scan mode and the +isolation guards are its authority, so as of #7148 a pinned per-thread override +beats an env request for `Full` **in the test build only** — the env var may +make the scan less aggressive than a test declared, never more. Production +binaries pin no override, so the arm above is unchanged and still forces the +scan. + +The `heap_used_bytes` column above is also the quantitative case behind #7148: +every `force_full_scan()` that fires at runtime is a collection that runs no +copying minor, so the four *automatic* fallback sites were each worth this much +RSS whenever they were reached. They are now counted +(`[gc-scan-fallback] site=… automatic=…` under `PERRY_GC_DIAG`), so a probe run +shows directly whether any of them fired. + ## Cross-host evidence (why the shared-CI profile gates what it gates) The baseline is captured on an 8-core M1 with 8 GB at load ~1.2. The first diff --git a/changelog.d/7166-gc-safepoint-deferral-scan-census.md b/changelog.d/7166-gc-safepoint-deferral-scan-census.md new file mode 100644 index 0000000000..00c0e751b2 --- /dev/null +++ b/changelog.d/7166-gc-safepoint-deferral-scan-census.md @@ -0,0 +1,19 @@ +fix(gc): run old-gen reclaim precisely at safepoints, stop scanning conservatively on OS memory pressure, and put a census on every conservative-scan fallback (#7148). + +`main` reached a forced conservative native-stack scan from **six** sites, **four of them without any user calling `gc()`**, and nothing measured whether any of them ever ran. A forced scan is not merely an imprecision: it makes the copying minor ineligible (`CopiedMinorFallbackReason::ConservativeStack`), so the cycle it covers runs **no copying minor at all**. `benchmarks/gc_ratchet/README.md` quantifies the end state — `heap_used_bytes` **+364% to +5371%** and `minor_cycles` → **0 on all eight probes**. + +#7148 asks for the fallback to become *unreachable*, not *imprecise*. Per site: + +- **Old-gen reclaim (`gc/policy.rs`) — kept, and given a competing precise path.** `gc_safepoint_moving_minor` used to bail on `OldReclaim` ("stays on its existing full mark-sweep path"); it now runs the same full mark-sweep there with `SkipDisabled` roots and no forced scan, so every program that reaches a safepoint gets its old-gen reclaim precisely and **no later than before**. The allocation-point arm is deliberately **not** deferred. An earlier revision of this change did defer it and the release suite caught it: #5476's regression test asserts that *a single* `gc_check_trigger` call — what every allocation does — completes the reclaim, because the workload it was filed for is a compute-only loop that reaches no host step. Two reasons that verdict is right rather than merely awkward: the headline RSS figure is the cost of the *copying minor* becoming ineligible, and this arm runs a full mark-sweep that is non-moving with or without the scan (the scan costs one cycle of conservative retention, which is much smaller and was not measured); and deferring trades a measured RSS guarantee — #5476's title is *RSS climbs unbounded* — for an unmeasured one. So the fallback is attacked by adding an earlier precise path, not by postponing the collection. +- **OS memory pressure (`gc/pressure.rs`) — the conservative arm is deleted outright.** The old rationale ("a host may deliver the callback with unspilled locals on the native frames above us") is now *tested* rather than assumed. With no active shadow frame the precise root set is complete, so the collection runs inline with no scan and the copying minor stays eligible — the run-loop-boundary case the module's own docs call typical, and the case where deferring would be worst, since a process idle enough to get an OS memory warning reaches no loop back-edge and pumps no microtasks. With a generated frame live a safepoint *is* reachable, so the request is deferred and the handler returns `1`, the code this API already documents as "trigger lowered, collection deferred". The lowered arena trigger is untouched either way, so no backstop is removed. `ConservativeScanSite` has no `HostPressure` variant as a result: an enum arm nothing can produce is a claim no test can check. +- **Nursery-churn valve, emergency reclaim, `gc()`, `perry/gc` `minor()` — kept, now counted.** Emergency reclaim provably cannot defer: it runs after an allocation has already failed and the caller's next act is to panic, so there is no next safepoint. The two explicit sites are user requests with synchronous semantics. + +New `gc/scan_fallback.rs` records every fallback (`[gc-scan-fallback] site=… automatic=… count=…` under `PERRY_GC_DIAG`) and every precise collection that replaced one (`[gc-safepoint-drain] kind=…`), separating automatic from explicit sites — all eight ratchet probes end with an explicit full `gc()`, so that site fires once per probe by construction and an undifferentiated census would misread. This is the reachability census for the invariant the statepoint experiment states and enforces (`docs/statepoint-gc-experiment.md` on `exp/stackmap-viability`): *a collection that skips the conservative scan consumes only precise roots, so it may only begin at a declared safepoint.* Census first, enforcement second. + +Every claim is tested under **both** `PERRY_GC_MOVING_LOOP_POLLS` defaults, since #7161 proposes flipping it OFF: the old-gen arm and the host-pressure precise collection never read that gate, and the host-pressure deferral keeps two gate-independent drains (the microtask-pump boundary, and the lowered arena trigger — the latter pinned by its own test because it is the one that holds for a program that never yields to the event loop). Site 2's dependence on the gate is pre-existing and unchanged; with polls off every nursery collection scans conservatively, which the census now makes visible rather than assumed. + +Also fixes `PERRY_CONSERVATIVE_STACK_SCAN=full`, which failed **134 of 1574** runtime tests on `main` — a shipped escape hatch nobody had verified. Fixed rather than deleted, against the kill-policy's default, because `=full` is `gc-ratchet`'s validated sensitivity arm: the run that produced the 60 regression rows proving that gate can fail. Deleting it would delete a gate's only end-to-end proof. The failures were never soundness failures — a collector test's central assertion is *"this object should have been collected"*, and an ambient conservative scan retains whatever the native stack looks like a pointer to. Since #7147 the test build has one declared scan mode and the isolation guards are its authority, so **in the test build only** a pinned override now beats an env request for `Full`; the env var may make the scan less aggressive than a test declared, never more. `=0` still beats a pinned `Full`, so the bisection hatch is intact. Production binaries pin no override and are unchanged. + +Every test asserts both that no conservative scan fired and that the precise collection which replaced it actually ran (a drain counter) — checking only the first half would pass on a tree where the trigger never armed at all, the largest possible regression reported as a pass. Verified with one-hunk sabotages in both directions; the sharpest is reintroducing the rejected old-gen deferral, which reddens #5476's own pre-existing test as well as the new one. + +`gc/roots.rs` crossed the 2,000-line file-size gate as a side effect, so the conservative-scan mode block moved to `gc/roots/scan_mode.rs` (1,992 → 1,851 lines). diff --git a/crates/perry-runtime/src/gc/mod.rs b/crates/perry-runtime/src/gc/mod.rs index edfa0e20b8..d639a8a18e 100644 --- a/crates/perry-runtime/src/gc/mod.rs +++ b/crates/perry-runtime/src/gc/mod.rs @@ -50,6 +50,11 @@ mod malloc; pub use malloc::*; mod roots; pub use roots::*; +/// #7148: the census of conservative-scan fallbacks and the precise-safepoint +/// drains that replace them. Declared next to `roots` because +/// `ManualGcScanGuard` is what records into it. +mod scan_fallback; +pub(crate) use scan_fallback::*; // The one decoder shared by the mark, rewrite and incremental-barrier paths // for words that may hold a heap reference (#6910). Declared before its // consumers for readability only — Rust module order is irrelevant. @@ -322,6 +327,24 @@ fn gc_collect_emergency_full() -> GcCollectOutcome { /// alloc-point direct arm forces it: this runs at an arbitrary allocation /// site where locals of the current call chain may not be spilled to /// shadow slots. +/// +/// ★ #7148 disposition: **keep, justified, observable.** This is the one site +/// that provably cannot defer to a precise safepoint. Deferral trades a +/// collection now for a collection at the next safepoint, and this path is +/// entered only after a heap allocation has *already failed*: the caller's +/// next act is to panic, so there is no "next safepoint" to defer to — the +/// program does not survive to reach one. The pressure-spike question the +/// other sites must answer is therefore vacuous here; the spike has already +/// happened and this is the response to it. +/// +/// It is instead made *measurable* (`ConservativeScanSite::EmergencyReclaim` +/// + a `PERRY_GC_DIAG` line), so "emergency reclaim never fires in practice" +/// stops being an assumption. The long-term plan for this site is the +/// statepoint work (`docs/statepoint-gc-experiment.md` on branch +/// `exp/stackmap-viability`, not on `main`): with native stack +/// maps a precise root set exists at *any* mapped PC, so an OOM-time +/// collection would not need the scan at all. That is the only mechanism that +/// removes this site, and it is not a deferral. pub(crate) fn gc_try_emergency_reclaim() -> bool { thread_local! { static IN_EMERGENCY: std::cell::Cell = const { std::cell::Cell::new(false) }; @@ -333,7 +356,7 @@ pub(crate) fn gc_try_emergency_reclaim() -> bool { return false; } IN_EMERGENCY.with(|c| c.set(true)); - let _scan = roots::ManualGcScanGuard::force_full_scan(); + let _scan = roots::ManualGcScanGuard::force_full_scan(ConservativeScanSite::EmergencyReclaim); let _ = gc_collect_emergency_full(); IN_EMERGENCY.with(|c| c.set(false)); true diff --git a/crates/perry-runtime/src/gc/policy.rs b/crates/perry-runtime/src/gc/policy.rs index 9656a817ee..f0af8ce5de 100644 --- a/crates/perry-runtime/src/gc/policy.rs +++ b/crates/perry-runtime/src/gc/policy.rs @@ -429,6 +429,22 @@ pub(super) fn force_legacy_gc_pacing() -> LegacyGcPacingGuard { LegacyGcPacingGuard { previous } } +/// Pin moving GC pacing (moving-loop polls ON) for the duration of the returned +/// guard — the shipped default, pinned explicitly so a #7148 deferral test +/// asserts against a *declared* pacing mode instead of inheriting whatever the +/// process-wide `PERRY_GC_MOVING_LOOP_POLLS` OnceLock resolved to. A test that +/// silently ran under legacy pacing would find the deferral branch dead and +/// pass for the wrong reason. +#[cfg(test)] +pub(super) fn force_moving_gc_pacing() -> LegacyGcPacingGuard { + let previous = GC_MOVING_LOOP_POLLS_TEST_OVERRIDE.with(|cell| { + let previous = cell.get(); + cell.set(Some(true)); + previous + }); + LegacyGcPacingGuard { previous } +} + pub(super) fn gc_trace_enabled() -> bool { #[cfg(test)] if GC_TRACE_TEST_FORCE.with(Cell::get) { @@ -1288,6 +1304,43 @@ pub fn gc_check_trigger() { // keeps it safe: anything still referenced from the stack/registers at this // allocation point (e.g. the temporary currently being built) is retained; // only genuinely unreachable old blocks are returned. + // + // ★ #7148 disposition: **keep, justified, observable — and add a precise + // path that beats it to the punch.** The first attempt at this site was a + // deferral like the nursery arm's. It was wrong, and the reasoning is kept + // because the shape recurs. + // + // 1. **The headline RSS argument does not apply here.** A conservative scan + // costs +364%..+5371% `heap_used_bytes` on the ratchet probes *because + // it makes the copying minor ineligible* — `minor_cycles` → 0. This arm + // runs a FULL mark-sweep, which is non-moving with or without the scan. + // What the scan costs here is conservative *retention* for one cycle, + // not the loss of evacuation. Much smaller, and unmeasured. + // 2. **Deferring it breaks a tested RSS guarantee.** #5476's regression test + // (`check_trigger_drives_old_reclaim_to_completion_without_host_stepping`) + // asserts that *a single* `gc_check_trigger` call — what every allocation + // does — drives the reclaim to completion, because the workload that + // motivated it is a compute-only loop that never reaches a host step. A + // deferral makes that "within one 32 MB growth quantum" instead, on the + // exact workload whose bug report was titled *RSS climbs unbounded*. + // Trading conservative retention for bounded-but-real extra old-gen + // residency is not obviously a win, and nothing here measured that it is. + // + // So this arm is unchanged, and `gc_safepoint_moving_minor` instead gained + // the SAME full mark-sweep with precise roots (#7148). Programs that reach a + // safepoint — every event-loop program — now get their old-gen reclaim + // precisely and *no later* than before; nothing is delayed for anyone. The + // fallback is attacked by adding a competing earlier precise path, not by + // postponing the collection. `ConservativeScanSite::OldReclaimAllocPoint` + // counts how often the alloc point still gets there first. + // + // Default-robustness (#7161 proposes flipping `PERRY_GC_MOVING_LOOP_POLLS` + // OFF): this arm does not consult that gate at all, so it behaves + // identically either way. The precise safepoint path is reached from the + // microtask pump under `gc_moving_safepoint_enabled` (a different knob, + // untouched by #7161) and from `js_gc_loop_safepoint` only while polls are + // on. Polls off ⇒ the precise path is reached less often ⇒ this arm fires + // more often ⇒ the census counter rises. Inert, not unsound. if !gc_budgeted_cycle_active() && matches!( gc_budgeted_due_trigger(), @@ -1297,7 +1350,9 @@ pub fn gc_check_trigger() { { let _reentry = OldReclaimReentryGuard::enter(); GC_OLD_RECLAIM_PENDING.with(|pending| pending.set(false)); - let _scan = super::roots::ManualGcScanGuard::force_full_scan(); + let _scan = super::roots::ManualGcScanGuard::force_full_scan( + super::ConservativeScanSite::OldReclaimAllocPoint, + ); gc_collect_full_mark_sweep_with_trigger(GcTriggerSnapshot::capture( GcTriggerKind::OldGenBytes, )) @@ -1400,8 +1455,28 @@ pub fn gc_check_trigger() { // arbitrary alloc point a value mid-construction may live only in // registers, which the conservative scan retains (and which makes // copied-minor ineligible, so the non-moving minor runs). - let _scan = (!super::gc_scavenge_enabled()) - .then(super::roots::ManualGcScanGuard::force_full_scan); + // + // ★ #7148 disposition: **keep as the bounded valve, now counted.** + // The deferral above is the primary path and is sound by + // construction; reaching here means the slack expired without the + // program touching a single loop back-edge poll or microtask-pump + // boundary — a mega-expression, or a synchronous recursion, that + // allocated `gc_moving_defer_slack_dyn_bytes()` past the deferral + // point. There is no safepoint to defer to in that state, so the + // answer to "what if pressure spikes before a safepoint is + // reached" is: this arm runs, and it is the reason RSS stays + // bounded. Making it *imprecise* instead (collecting without the + // scan) is the one thing #7148 rules out — it would trade a cost + // problem for a soundness problem. Making it **countable** is what + // turns "unreachable in practice" into a measurement: + // `ConservativeScanSite::NurseryChurnSlackValve` is 0 on all eight + // ratchet probes and across the stress matrix, and the drain + // counter proves the deferral ran instead. + let _scan = (!super::gc_scavenge_enabled()).then(|| { + super::roots::ManualGcScanGuard::force_full_scan( + super::ConservativeScanSite::NurseryChurnSlackValve, + ) + }); let outcome = super::gc_collect_minor_with_trigger(GcTriggerSnapshot::capture(kind)); // Re-baseline the arming trigger after the direct minor, mirroring // `gc_finish_budgeted_cycle`. This arm is taken whenever @@ -1602,11 +1677,30 @@ pub(crate) fn gc_safepoint_moving_minor() { // We are handling this safepoint (collect or find nothing due): clear the // deferral flag set by the alloc-point arm (Phase 2/3). GC_SAFEPOINT_PENDING.with(|p| p.set(false)); - // Only nursery-pressure triggers take the moving minor here; OldReclaim - // stays on its existing full mark-sweep path. let kind = match gc_budgeted_due_trigger() { Some(BudgetedGcTrigger::ArenaBytes) => GcTriggerKind::ArenaBytes, Some(BudgetedGcTrigger::MallocCount) => GcTriggerKind::MallocCount, + // ★ #7148: old-gen reclaim used to be the alloc-point arm's business + // exclusively — it ran a direct full mark-sweep behind a forced + // conservative scan, because at an allocation point that scan is what + // makes it sound. Here it is not needed: this is the same precise-root + // safepoint the nursery minor uses, so run the identical full + // mark-sweep with `SkipDisabled` roots. The alloc-point arm keeps only + // the bounded slack valve. + Some(BudgetedGcTrigger::OldReclaim) => { + if GC_OLD_RECLAIM_IN_PROGRESS.with(Cell::get) { + return; + } + let _reentry = OldReclaimReentryGuard::enter(); + GC_OLD_RECLAIM_PENDING.with(|pending| pending.set(false)); + // No `force_full_scan`: roots are precise at this safepoint. + gc_collect_full_mark_sweep_with_trigger(GcTriggerSnapshot::capture( + GcTriggerKind::OldGenBytes, + )) + .emit_after_current(); + super::record_safepoint_drain(super::SafepointDrainKind::OldReclaim); + return; + } _ => { // No nursery-pressure trigger is due — nothing to collect here. return; @@ -1624,6 +1718,11 @@ pub(crate) fn gc_safepoint_moving_minor() { gc_finish_arena_trigger_collection(pre_in_use, outcome); } } + // The live-subject counter for every deferral gate: a test that asserts + // "the conservative valve did not fire" is vacuous unless it can also show + // the precise collection that replaced it actually ran (CLAUDE.md, four + // ways a gate cannot fail — #4, the gate runs but its subject never did). + super::record_safepoint_drain(super::SafepointDrainKind::NurseryMinor); } /// Phase 2 of the moving-GC project: codegen emits a call to this at loop @@ -2160,8 +2259,28 @@ pub extern "C" fn js_gc_collect() { /// Run an explicit (`gc()`) full collection. The `gc()` callsite may hold live /// module-init/top-level locals only on the native stack, so the collection /// forces the conservative native-stack scan (#4977); see `ManualGcScanGuard`. +/// +/// ★ #7148 disposition: **keep, observable.** Unlike the four automatic sites +/// this is not a collection the program pays for without asking. `gc()` is a +/// user request with synchronous semantics — `gc(); assertFreed()` is the +/// shape every test and every ratchet probe uses — so deferring it to the next +/// safepoint would change the observable contract of the API, not just its +/// cost. It is counted (`ConservativeScanSite::ManualCollect`) so its share of +/// any census is attributable: all eight `gc_ratchet` probes end with an +/// explicit full `gc()`, so this site fires at least once per probe *by +/// construction*, and a census that did not separate it from the automatic +/// sites would look alarming for no reason. +/// +/// The known cost is #6942/#6946: forcing the scan makes this path non-moving, +/// which is why `PERRY_GC_FORCE_EVACUATE` was inert for every `gc()`-driven +/// test. Removing the scan here needs precise roots at the `gc()` callsite PC +/// — the safepoint contract in `docs/statepoint-gc-experiment.md` on branch +/// `exp/stackmap-viability` (not on `main`) — not a +/// deferral. fn manual_gc_collect_now() { - let _scan = super::roots::ManualGcScanGuard::force_full_scan(); + let _scan = super::roots::ManualGcScanGuard::force_full_scan( + super::ConservativeScanSite::ManualCollect, + ); // NOTE: pending finalization jobs from earlier AUTOMATIC cycles are NOT // cleared here — each record enqueues exactly once (its pending flag is // reset at enqueue time), so dropping the vec would lose those callbacks @@ -2191,12 +2310,18 @@ pub extern "C" fn js_gc_module_collect() -> f64 { /// freed byte count (0 when skipped: unsafe zone or deferred). Like `gc()`, /// the callsite may hold live locals only on the native stack, so force the /// conservative scan (#4977). +/// +/// ★ #7148 disposition: **keep, observable** — same reasoning as +/// `manual_gc_collect_now`. `minor()` returns the freed byte count, so its +/// result is only meaningful if the collection ran before it returned; +/// deferring would have to return 0 and silently mean something else. #[no_mangle] pub extern "C" fn js_gc_module_minor() -> f64 { if manual_gc_blocked_by_unsafe_zone() { return 0.0; } - let _scan = super::roots::ManualGcScanGuard::force_full_scan(); + let _scan = + super::roots::ManualGcScanGuard::force_full_scan(super::ConservativeScanSite::ManualMinor); super::gc_collect_minor() as f64 } diff --git a/crates/perry-runtime/src/gc/pressure.rs b/crates/perry-runtime/src/gc/pressure.rs index 879185588f..2161d04dd5 100644 --- a/crates/perry-runtime/src/gc/pressure.rs +++ b/crates/perry-runtime/src/gc/pressure.rs @@ -19,11 +19,43 @@ //! but the entry defends itself with the same guards the safepoint uses; //! when collecting here is unsafe, the lowered trigger still guarantees a //! prompt collection at the next allocation-side check. +//! +//! ★ #7148 disposition: **defer.** This site used to force the conservative +//! native-stack scan unconditionally, on the grounds that "a host may deliver +//! the callback with unspilled locals on the native frames above us". True — +//! but only when there *are* generated frames above us, and the paragraph +//! above says the common case is the opposite: the handler fires at a run-loop +//! boundary with the JS stack unwound. The rationale is now *tested* rather +//! than assumed: +//! +//! * **No active shadow frame** — no generated frame is live on this thread, +//! so the precise root set is complete (shadow stack empty; globals, module +//! vars and side tables scanned; runtime helpers hold their temporaries in +//! `RuntimeHandleScope`). Collect right here with `SkipDisabled` roots: +//! precise, and the copying minor stays eligible. This is the case hosts +//! actually deliver, and it is also the case where deferring would be +//! *worst* — memory pressure arrives when a process is idle, and an idle +//! process reaches no loop back-edge and pumps no microtasks, so a deferral +//! would shed nothing at all before the jetsam. +//! * **A generated frame is live above us** — do not scan. Arm the deferral +//! and return 1, the code this API already documents as "trigger lowered, +//! collection deferred". A live generated frame is precisely the condition +//! under which a safepoint *is* reachable: the loop back-edge poll or +//! microtask-pump boundary that frame will reach drains it precisely. +//! +//! **What if pressure spikes before that safepoint is reached?** The lowered +//! arena trigger is left in place either way, so the alloc-point arm in +//! `gc_check_trigger` stays armed and collects at the next allocation check — +//! identical to the pre-existing `blocked` path, which has always returned 1 +//! and relied on exactly that. The deferral adds a faster precise drain in +//! front of that fallback; it removes no backstop. use super::*; /// Return codes: 0 = ignored (level 0), 1 = trigger lowered but the -/// collection was deferred (unsafe point), 2 = collected synchronously. +/// collection was deferred (unsafe point, or a generated frame is live above +/// us and a precise safepoint is reachable — #7148), 2 = collected +/// synchronously. #[no_mangle] pub extern "C" fn js_gc_memory_pressure(level: u32) -> u32 { if level == 0 { @@ -48,10 +80,28 @@ pub extern "C" fn js_gc_memory_pressure(level: u32) -> u32 { if blocked { return 1; } - // Force the conservative scan for the same reason the alloc-point arm - // does: a host may deliver the callback with unspilled locals on the - // native frames above us. - let _scan = roots::ManualGcScanGuard::force_full_scan(); + + // #7148: a generated frame is live above us, so this is not a precise + // point — but it is a point from which a precise one is reachable. Arm the + // deferral instead of scanning conservatively. `GC_SAFEPOINT_PENDING` is + // the flag `js_gc_loop_safepoint` polls; for a *critical* level the owed + // collection must be a full cycle, and `GC_OLD_RECLAIM_PENDING` is exactly + // the sticky "a full old-gen reclaim is owed" flag `gc_budgeted_due_trigger` + // reads, so the safepoint drain runs a full mark-sweep rather than a minor. + if roots::shadow_stack_has_active_frame() { + if level >= 2 { + GC_OLD_RECLAIM_PENDING.with(|pending| pending.set(true)); + } + if !GC_SAFEPOINT_PENDING.with(std::cell::Cell::get) { + GC_SAFEPOINT_DEFER_ARENA_BASE.with(|base| base.set(total)); + GC_SAFEPOINT_PENDING.with(|p| p.set(true)); + } + return 1; + } + + // No generated frame is live on this thread: the precise root set is + // complete, so NO `force_full_scan`. `conservative_stack_scan_decision()` + // stays `SkipDisabled` and the copying minor remains eligible. if level >= 2 { let _ = gc_collect_full_mark_sweep_with_trigger(GcTriggerSnapshot::capture( GcTriggerKind::Emergency, @@ -59,5 +109,6 @@ pub extern "C" fn js_gc_memory_pressure(level: u32) -> u32 { } else { let _ = gc_collect_minor_with_trigger(GcTriggerSnapshot::capture(GcTriggerKind::Direct)); } + record_safepoint_drain(SafepointDrainKind::HostPressure); 2 } diff --git a/crates/perry-runtime/src/gc/roots.rs b/crates/perry-runtime/src/gc/roots.rs index 0db36e456a..91390de30e 100644 --- a/crates/perry-runtime/src/gc/roots.rs +++ b/crates/perry-runtime/src/gc/roots.rs @@ -2,6 +2,7 @@ use super::*; use std::any::Any; mod runtime_handles; +mod scan_mode; mod scanner_shims; mod shadow_stack; mod temp_roots; @@ -22,6 +23,18 @@ pub use scanner_shims::{ small_int_cache_mutable_root_scanner, small_int_cache_root_scanner, timer_mutable_root_scanner, timer_root_scanner, transition_cache_mutable_root_scanner, transition_cache_root_scanner, }; +// The conservative-scan mode lives in `roots/scan_mode.rs` (split out in #7148 +// when this file crossed the 2,000-line gate) but every consumer names it +// `gc::roots::…` or reaches it through `mod.rs`'s `pub use roots::*`, so the +// whole surface is re-exported here — same shape, and same `allow`, as the +// `shadow_stack` re-exports below. +#[allow(unused_imports)] +pub(crate) use scan_mode::{ + conservative_stack_scan_decision, conservative_stack_scan_decision_for, + conservative_stack_scan_mode, conservative_stack_scan_mode_from_value, + set_conservative_stack_scan_override, ConservativeStackScanDecision, ConservativeStackScanMode, + ManualGcScanGuard, CONSERVATIVE_STACK_SCAN_OVERRIDE, +}; pub(crate) use shadow_stack::shadow_stack_has_active_frame; pub(crate) use shadow_stack::SHADOW; #[allow(unused_imports)] @@ -147,155 +160,6 @@ pub(super) fn exit_gc_root_lock() { } } -#[derive(Clone, Copy, Debug, Eq, PartialEq)] -pub(crate) enum ConservativeStackScanMode { - Auto, - Disabled, - Full, -} - -#[derive(Clone, Copy, Debug, Eq, PartialEq, Default)] -pub(super) enum ConservativeStackScanDecision { - Scan, - #[default] - SkipDisabled, -} - -impl ConservativeStackScanDecision { - #[cfg(feature = "diagnostics")] - #[inline] - pub(super) const fn as_str(self) -> &'static str { - match self { - Self::Scan => "scan", - Self::SkipDisabled => "skip_disabled", - } - } -} - -pub(super) fn conservative_stack_scan_mode_from_value( - value: Option<&str>, -) -> ConservativeStackScanMode { - let Some(value) = value else { - return ConservativeStackScanMode::Auto; - }; - match value.trim().to_ascii_lowercase().as_str() { - "" | "auto" => ConservativeStackScanMode::Auto, - "0" | "off" | "false" => ConservativeStackScanMode::Disabled, - "1" | "on" | "true" | "full" | "debug" => ConservativeStackScanMode::Full, - _ => ConservativeStackScanMode::Auto, - } -} - -thread_local! { - /// Per-thread override for the conservative-scan mode, taking precedence over - /// the `#[cfg(test)]` default but not an explicit env var. The GC unit tests - /// set this to `Auto` (skip) inside their controlled-root scopes so a forced - /// collection still reclaims the objects they hold only as native-stack - /// locals; see `ScopedRootScannerRegistryGuard`. - pub(super) static CONSERVATIVE_STACK_SCAN_OVERRIDE: std::cell::Cell> = - const { std::cell::Cell::new(None) }; -} - -/// Set (or clear) this thread's conservative-scan mode override, returning the -/// previous value. -#[allow(dead_code)] // test-only scaffolding: GC unit tests (gc/tests) pin the scan mode via this override -pub(crate) fn set_conservative_stack_scan_override( - mode: Option, -) -> Option { - CONSERVATIVE_STACK_SCAN_OVERRIDE.with(|c| c.replace(mode)) -} - -/// Scoped guard forcing the conservative native-stack scan for an explicit -/// `gc()` collection (#4977). In the default `Auto` mode a full collection -/// skips the native scan, but at a `gc()` callsite live module-init/top-level -/// locals may be held only on the native stack — neither the precise -/// shadow-stack roots nor the module-var scanners cover them — so the -/// collector reclaimed live object graphs and later field reads returned -/// dangling-pointer garbage. An already-pinned per-thread override wins (the -/// GC unit tests pin `Auto` so a forced collection still reclaims objects they -/// hold only as native-stack locals), and an explicit -/// `PERRY_CONSERVATIVE_STACK_SCAN` env value beats any override either way, -/// so the bisection escape hatch keeps working. -pub(super) struct ManualGcScanGuard { - engaged: bool, -} - -impl ManualGcScanGuard { - pub(super) fn force_full_scan() -> Self { - let engaged = CONSERVATIVE_STACK_SCAN_OVERRIDE.with(|c| { - if c.get().is_some() { - return false; - } - c.set(Some(ConservativeStackScanMode::Full)); - true - }); - Self { engaged } - } -} - -impl Drop for ManualGcScanGuard { - fn drop(&mut self) { - if self.engaged { - CONSERVATIVE_STACK_SCAN_OVERRIDE.with(|c| c.set(None)); - } - } -} - -pub(super) fn conservative_stack_scan_mode() -> ConservativeStackScanMode { - // Explicit env var wins (so the dedicated mode tests and ops bisection keep - // working), then a per-thread override, then the build-time default. - if let Ok(value) = std::env::var("PERRY_CONSERVATIVE_STACK_SCAN") { - return conservative_stack_scan_mode_from_value(Some(&value)); - } - if let Some(mode) = CONSERVATIVE_STACK_SCAN_OVERRIDE.with(|c| c.get()) { - return mode; - } - // ONE default for every build, tests included. - // - // The test build used to default to `Full` on the grounds that unit tests - // root GC-managed pointers as raw locals on the *native* (Rust) stack - // rather than the shadow stack, so the conservative scan was what kept them - // alive. That is a description of how some tests were written, not a - // requirement of testing, and it had a cost that outweighed the - // convenience: it made the test build a DIFFERENT GC configuration from - // production. A missing precise root — the #7055 shape — was rescued by the - // native scan under test and was a live wrong answer in production, so the - // suite filtered out precisely the failure class it exists to catch. It - // also made the copying minor ineligible under test - // (`CopiedMinorFallbackReason::ConservativeStack`), so relocation was - // largely unexercised. - // - // The correct mechanism already existed and the tests already used it: - // `RuntimeHandleScope` / `gc_temp_root_*`, plus the isolation guards, which - // pin `Auto` themselves — 351 of the 531 gc tests were already running this - // way. Flipping the default cost exactly one test conversion - // (`test_minor_preserves_old_to_young_edge_across_minors`, which read a - // relocated child through a stale pre-collection local). - // - // A test that needs the scan *provably* off — not merely resolved off by - // today's `Auto` policy — should pin `ConservativeScanDisabledGuard`. - ConservativeStackScanMode::Auto -} - -#[inline] -pub(super) fn conservative_stack_scan_decision_for( - mode: ConservativeStackScanMode, - _shadow_frame_active: bool, -) -> ConservativeStackScanDecision { - match mode { - ConservativeStackScanMode::Disabled => ConservativeStackScanDecision::SkipDisabled, - ConservativeStackScanMode::Full => ConservativeStackScanDecision::Scan, - ConservativeStackScanMode::Auto => ConservativeStackScanDecision::SkipDisabled, - } -} - -pub(super) fn conservative_stack_scan_decision() -> ConservativeStackScanDecision { - conservative_stack_scan_decision_for( - conservative_stack_scan_mode(), - shadow_stack_has_active_frame(), - ) -} - /// Register a root scanner function. /// Each scanner is called during the mark phase to discover roots. /// This legacy API exposes copied values only. It remains supported for diff --git a/crates/perry-runtime/src/gc/roots/scan_mode.rs b/crates/perry-runtime/src/gc/roots/scan_mode.rs new file mode 100644 index 0000000000..12122b0fcb --- /dev/null +++ b/crates/perry-runtime/src/gc/roots/scan_mode.rs @@ -0,0 +1,210 @@ +//! Conservative native-stack scan mode: how a collection decides whether to +//! walk this thread's native (Rust) stack looking for anything that might be a +//! heap pointer. +//! +//! Split out of `roots.rs` in #7148 — that file crossed the 2,000-line gate, +//! and this is the topically self-contained piece: the mode enum, its env/ +//! override precedence, the `ManualGcScanGuard` that pins `Full` for one +//! collection, and the decision the mark phase consults. Nothing else in +//! `roots.rs` reads its internals. +//! +//! ★ The decision this module returns is the single most expensive bit in the +//! collector. `Scan` makes the copying minor ineligible +//! (`CopiedMinorFallbackReason::ConservativeStack`), so a cycle that takes it +//! runs no copying minor at all — measured at `heap_used_bytes` +364%..+5371% +//! and `minor_cycles` → 0 on all eight `gc_ratchet` probes. See +//! `super::super::scan_fallback` for the census of who asks for it. + +use super::super::*; + +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub(crate) enum ConservativeStackScanMode { + Auto, + Disabled, + Full, +} + +#[derive(Clone, Copy, Debug, Eq, PartialEq, Default)] +pub(crate) enum ConservativeStackScanDecision { + Scan, + #[default] + SkipDisabled, +} + +impl ConservativeStackScanDecision { + #[cfg(feature = "diagnostics")] + #[inline] + pub(crate) const fn as_str(self) -> &'static str { + match self { + Self::Scan => "scan", + Self::SkipDisabled => "skip_disabled", + } + } +} + +pub(crate) fn conservative_stack_scan_mode_from_value( + value: Option<&str>, +) -> ConservativeStackScanMode { + let Some(value) = value else { + return ConservativeStackScanMode::Auto; + }; + match value.trim().to_ascii_lowercase().as_str() { + "" | "auto" => ConservativeStackScanMode::Auto, + "0" | "off" | "false" => ConservativeStackScanMode::Disabled, + "1" | "on" | "true" | "full" | "debug" => ConservativeStackScanMode::Full, + _ => ConservativeStackScanMode::Auto, + } +} + +thread_local! { + /// Per-thread override for the conservative-scan mode, taking precedence over + /// the `#[cfg(test)]` default but not an explicit env var. The GC unit tests + /// set this to `Auto` (skip) inside their controlled-root scopes so a forced + /// collection still reclaims the objects they hold only as native-stack + /// locals; see `ScopedRootScannerRegistryGuard`. + pub(crate) static CONSERVATIVE_STACK_SCAN_OVERRIDE: std::cell::Cell> = + const { std::cell::Cell::new(None) }; +} + +/// Set (or clear) this thread's conservative-scan mode override, returning the +/// previous value. +#[allow(dead_code)] // test-only scaffolding: GC unit tests (gc/tests) pin the scan mode via this override +pub(crate) fn set_conservative_stack_scan_override( + mode: Option, +) -> Option { + CONSERVATIVE_STACK_SCAN_OVERRIDE.with(|c| c.replace(mode)) +} + +/// Scoped guard forcing the conservative native-stack scan for an explicit +/// `gc()` collection (#4977). In the default `Auto` mode a full collection +/// skips the native scan, but at a `gc()` callsite live module-init/top-level +/// locals may be held only on the native stack — neither the precise +/// shadow-stack roots nor the module-var scanners cover them — so the +/// collector reclaimed live object graphs and later field reads returned +/// dangling-pointer garbage. An already-pinned per-thread override wins (the +/// GC unit tests pin `Auto` so a forced collection still reclaims objects they +/// hold only as native-stack locals), and an explicit +/// `PERRY_CONSERVATIVE_STACK_SCAN` env value beats any override either way, +/// so the bisection escape hatch keeps working. +/// +/// ★ #7148: engaging this guard is *expensive*, not merely imprecise — the +/// conservative scan makes the copying minor ineligible +/// (`CopiedMinorFallbackReason::ConservativeStack`), so the cycle it covers +/// runs no copying minor at all (`benchmarks/gc_ratchet/README.md`: +/// `heap_used_bytes` +364%..+5371%, `minor_cycles` → 0 on all eight probes). +/// Every callsite therefore declares which of the six sites it is and is +/// counted in `super::scan_fallback`, so "this never fires in practice" is a +/// measurement instead of an assumption. +pub(crate) struct ManualGcScanGuard { + engaged: bool, +} + +impl ManualGcScanGuard { + pub(crate) fn force_full_scan(site: super::ConservativeScanSite) -> Self { + super::record_scan_fallback(site); + let engaged = CONSERVATIVE_STACK_SCAN_OVERRIDE.with(|c| { + if c.get().is_some() { + return false; + } + c.set(Some(ConservativeStackScanMode::Full)); + true + }); + Self { engaged } + } +} + +impl Drop for ManualGcScanGuard { + fn drop(&mut self) { + if self.engaged { + CONSERVATIVE_STACK_SCAN_OVERRIDE.with(|c| c.set(None)); + } + } +} + +pub(crate) fn conservative_stack_scan_mode() -> ConservativeStackScanMode { + // Explicit env var wins (so the dedicated mode tests and ops bisection keep + // working), then a per-thread override, then the build-time default. + let env_mode = std::env::var("PERRY_CONSERVATIVE_STACK_SCAN") + .ok() + .map(|value| conservative_stack_scan_mode_from_value(Some(&value))); + let pinned = CONSERVATIVE_STACK_SCAN_OVERRIDE.with(|c| c.get()); + + // ★ #7148: in the TEST build a pinned override beats an env request for + // `Full`. The env var may make the scan *less* aggressive than the mode a + // test declared, never more. + // + // Why: `PERRY_CONSERVATIVE_STACK_SCAN=full` failed **134 of 1574** + // perry-runtime tests — a shipped escape hatch in a state nobody had + // verified. The failures were not soundness failures. A collector test's + // central assertion is *"this object should have been collected"*, and a + // conservative scan retains whatever the native stack happens to look like + // a pointer to, so an ambient `=full` broke exactly the assertions the + // suite exists to make. Since #7147 the test build has ONE declared scan + // mode (`Auto`) and the isolation guards are its authority; letting an + // ambient env var silently re-impose `Full` on a guarded scope reintroduces + // the test-build-is-a-different-GC-configuration problem #7147 removed. + // + // Kept rather than deleted (CLAUDE.md's kill-policy default) because + // `=full` is load-bearing *outside* the test build: it is the ratchet's + // validated sensitivity arm — the run that produced the 60 regression rows + // proving `gc-ratchet` can actually fail (`benchmarks/gc_ratchet/README.md`). + // Deleting the mode would delete a gate's only end-to-end proof. This makes + // it verified instead: the suite is green under `=full`. + #[cfg(test)] + if matches!(env_mode, Some(ConservativeStackScanMode::Full)) { + if let Some(pinned) = pinned { + return pinned; + } + } + + if let Some(mode) = env_mode { + return mode; + } + if let Some(mode) = pinned { + return mode; + } + // ONE default for every build, tests included. + // + // The test build used to default to `Full` on the grounds that unit tests + // root GC-managed pointers as raw locals on the *native* (Rust) stack + // rather than the shadow stack, so the conservative scan was what kept them + // alive. That is a description of how some tests were written, not a + // requirement of testing, and it had a cost that outweighed the + // convenience: it made the test build a DIFFERENT GC configuration from + // production. A missing precise root — the #7055 shape — was rescued by the + // native scan under test and was a live wrong answer in production, so the + // suite filtered out precisely the failure class it exists to catch. It + // also made the copying minor ineligible under test + // (`CopiedMinorFallbackReason::ConservativeStack`), so relocation was + // largely unexercised. + // + // The correct mechanism already existed and the tests already used it: + // `RuntimeHandleScope` / `gc_temp_root_*`, plus the isolation guards, which + // pin `Auto` themselves — 351 of the 531 gc tests were already running this + // way. Flipping the default cost exactly one test conversion + // (`test_minor_preserves_old_to_young_edge_across_minors`, which read a + // relocated child through a stale pre-collection local). + // + // A test that needs the scan *provably* off — not merely resolved off by + // today's `Auto` policy — should pin `ConservativeScanDisabledGuard`. + ConservativeStackScanMode::Auto +} + +#[inline] +pub(crate) fn conservative_stack_scan_decision_for( + mode: ConservativeStackScanMode, + _shadow_frame_active: bool, +) -> ConservativeStackScanDecision { + match mode { + ConservativeStackScanMode::Disabled => ConservativeStackScanDecision::SkipDisabled, + ConservativeStackScanMode::Full => ConservativeStackScanDecision::Scan, + ConservativeStackScanMode::Auto => ConservativeStackScanDecision::SkipDisabled, + } +} + +pub(crate) fn conservative_stack_scan_decision() -> ConservativeStackScanDecision { + conservative_stack_scan_decision_for( + conservative_stack_scan_mode(), + shadow_stack_has_active_frame(), + ) +} diff --git a/crates/perry-runtime/src/gc/scan_fallback.rs b/crates/perry-runtime/src/gc/scan_fallback.rs new file mode 100644 index 0000000000..d83347be7a --- /dev/null +++ b/crates/perry-runtime/src/gc/scan_fallback.rs @@ -0,0 +1,254 @@ +//! Conservative-scan fallback accounting and the safepoint-deferral counters +//! that make "the fallback never fires" a *measured* statement (#7148). +//! +//! # Why this module exists +//! +//! `ManualGcScanGuard::force_full_scan()` pins this thread's conservative +//! native-stack scan on for the duration of one collection. That is sound — +//! it retains anything reachable from a register or an unspilled native local +//! — but it is *catastrophically* expensive, and the cost is not a slowdown: +//! a conservative scan makes the copying minor ineligible +//! (`CopiedMinorFallbackReason::ConservativeStack`), so a cycle that takes it +//! runs **no copying minor at all**. `benchmarks/gc_ratchet/README.md` +//! measures the end-to-end effect of scanning on the eight ratchet probes: +//! `heap_used_bytes` +364% to +5371%, and `minor_cycles` → **0 on all eight**. +//! +//! So every `force_full_scan()` that fires is a collection that reclaims by +//! sweeping instead of by evacuating, on a heap that has retained everything +//! the native stack happened to look like a pointer to. +//! +//! # The rule +//! +//! Before #7148 there were six `force_full_scan()` sites and no way to tell, +//! for any given program, whether any of them ever ran. "It never fires in +//! practice" was an assumption. Per CLAUDE.md's four-ways-a-gate-cannot-fail +//! rule — *a gate must assert its subject was live* — an unobservable fallback +//! is indistinguishable from a fallback that fires on every cycle. +//! +//! Every site therefore records itself here, and the two *deferral* paths that +//! replace a scan with a precise safepoint collection record themselves too. +//! A test or a benchmark can now assert both halves of the claim: +//! +//! * the conservative fallback did **not** fire (`scan_fallback_count`), and +//! * the precise safepoint collection that replaced it **did** +//! (`safepoint_drain_count`) — not merely that nothing crashed. +//! +//! # Relationship to the explicit-safepoint contract +//! +//! The statepoint experiment (`exp/stackmap-viability`, +//! `docs/statepoint-gc-experiment.md`) states the invariant this instrument +//! measures compliance with: *a collection that skips the conservative stack +//! scan consumes only precise roots, so it may only begin at a declared +//! safepoint (a loop back-edge poll or the outermost microtask-pump +//! boundary); anywhere else it must scan conservatively.* That experiment +//! enforces the invariant with a thread-local declared-safepoint flag plus a +//! check at the root-scan subphase, in two levels (heal / strict panic). +//! +//! This module is the *reachability census* for the same contract on `main`: +//! it counts the sites where the contract's "anywhere else" arm is taken. +//! Driving those counts to zero is what makes the conservative scanner +//! deletable; the contract's enforcement check is what makes zero *provable* +//! rather than observed. The two compose — census first, enforcement second. + +use std::cell::Cell; + +/// A `force_full_scan()` callsite. Ordering matters only for the counter array. +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub(crate) enum ConservativeScanSite { + /// `gc_check_trigger` old-gen reclaim, at an allocation point. Automatic. + /// Deliberately NOT deferred — #5476 requires a single `gc_check_trigger` + /// call to complete the reclaim, and delaying it would regress the RSS bug + /// that arm exists for. What #7148 added is a competing PRECISE path at + /// safepoints (`SafepointDrainKind::OldReclaim`); this counter is how often + /// the allocation point still got there first. + OldReclaimAllocPoint, + /// `gc_check_trigger` nursery-churn direct minor, at an allocation point, + /// after the safepoint deferral ran out of slack. Automatic. + NurseryChurnSlackValve, + /// `gc_try_emergency_reclaim` — a heap allocation already failed and the + /// caller is about to panic. Automatic; cannot defer (see `mod.rs`). + EmergencyReclaim, + /// `manual_gc_collect_now` — explicit `gc()`. Explicit. + ManualCollect, + /// `js_gc_module_minor` — explicit `perry/gc` `minor()`. Explicit. + ManualMinor, + // ★ There is deliberately no `HostPressure` variant. `js_gc_memory_pressure` + // used to force the scan unconditionally; after #7148 it either collects + // with precise roots (empty shadow stack) or defers to a safepoint (a + // generated frame is live), so it has no conservative arm left to count. + // The site is not "unreached", it is **deleted** — which is the strongest + // form of what #7148 asks for, and why an unconstructed variant kept here + // "for completeness" would be the wrong kind of tidy: an enum arm nothing + // can produce is a claim no test can check. +} + +impl ConservativeScanSite { + pub(crate) const COUNT: usize = 5; + + const fn index(self) -> usize { + match self { + Self::OldReclaimAllocPoint => 0, + Self::NurseryChurnSlackValve => 1, + Self::EmergencyReclaim => 2, + Self::ManualCollect => 3, + Self::ManualMinor => 4, + } + } + + pub(crate) const fn as_str(self) -> &'static str { + match self { + Self::OldReclaimAllocPoint => "old_reclaim_alloc_point", + Self::NurseryChurnSlackValve => "nursery_churn_slack_valve", + Self::EmergencyReclaim => "emergency_reclaim", + Self::ManualCollect => "manual_collect", + Self::ManualMinor => "manual_minor", + } + } + + /// Whether this site is reached without any user code calling `gc()`. + /// The automatic sites are the ones #7148 is about: they are the + /// collections a program pays for without asking for them. + pub(crate) const fn is_automatic(self) -> bool { + match self { + Self::OldReclaimAllocPoint | Self::NurseryChurnSlackValve | Self::EmergencyReclaim => { + true + } + Self::ManualCollect | Self::ManualMinor => false, + } + } + + #[cfg(test)] + pub(crate) const ALL: [Self; Self::COUNT] = [ + Self::OldReclaimAllocPoint, + Self::NurseryChurnSlackValve, + Self::EmergencyReclaim, + Self::ManualCollect, + Self::ManualMinor, + ]; +} + +/// A precise-root collection that ran *instead of* a conservative one. These +/// are the "the precise path actually ran" counters — the live-subject +/// assertion for every gate in this family, without which "nothing scanned" +/// would also be satisfied by "nothing collected". +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub(crate) enum SafepointDrainKind { + /// A deferred nursery-pressure trigger collected at a safepoint. + NurseryMinor, + /// A deferred old-gen reclaim collected at a safepoint (#7148). + OldReclaim, + /// A host memory-pressure request collected **synchronously, inline in + /// `js_gc_memory_pressure`**, with precise roots because no generated frame + /// was live (#7148). Unlike the other two kinds this is not a deferred + /// drain through `js_gc_loop_safepoint` → `gc_safepoint_moving_minor`: the + /// handler is already at a precise point, and deferring there would shed + /// nothing, since a process idle enough to get an OS memory warning reaches + /// no loop back-edge and pumps no microtasks. (When a frame *is* live the + /// handler defers, and the drain is counted as `OldReclaim` or + /// `NurseryMinor` by whichever trigger it armed.) + HostPressure, +} + +impl SafepointDrainKind { + pub(crate) const COUNT: usize = 3; + + const fn index(self) -> usize { + match self { + Self::NurseryMinor => 0, + Self::OldReclaim => 1, + Self::HostPressure => 2, + } + } + + pub(crate) const fn as_str(self) -> &'static str { + match self { + Self::NurseryMinor => "nursery_minor", + Self::OldReclaim => "old_reclaim", + Self::HostPressure => "host_pressure", + } + } +} + +thread_local! { + static SCAN_FALLBACKS: Cell<[u64; ConservativeScanSite::COUNT]> = + const { Cell::new([0; ConservativeScanSite::COUNT]) }; + static SAFEPOINT_DRAINS: Cell<[u64; SafepointDrainKind::COUNT]> = + const { Cell::new([0; SafepointDrainKind::COUNT]) }; +} + +/// Record that `site` is about to force the conservative native-stack scan. +/// +/// Called from `ManualGcScanGuard::force_full_scan`, i.e. exactly once per +/// engagement, *including* engagements that turn out to be no-ops because an +/// override was already pinned — the census is of the callsite's intent, not +/// of the guard's internal bookkeeping. Under `PERRY_GC_DIAG` each occurrence +/// prints a line so an ops/benchmark run shows which sites a program reaches +/// and how often. +pub(crate) fn record_scan_fallback(site: ConservativeScanSite) { + let count = SCAN_FALLBACKS.with(|c| { + let mut counts = c.get(); + counts[site.index()] = counts[site.index()].saturating_add(1); + c.set(counts); + counts[site.index()] + }); + if std::env::var_os("PERRY_GC_DIAG").is_some() { + eprintln!( + "[gc-scan-fallback] site={} automatic={} count={}", + site.as_str(), + site.is_automatic(), + count + ); + } +} + +/// Record that a deferred collection drained at a precise-root safepoint — +/// the collection that a `force_full_scan()` site would otherwise have run +/// conservatively at an allocation point. +pub(crate) fn record_safepoint_drain(kind: SafepointDrainKind) { + let count = SAFEPOINT_DRAINS.with(|c| { + let mut counts = c.get(); + counts[kind.index()] = counts[kind.index()].saturating_add(1); + c.set(counts); + counts[kind.index()] + }); + if std::env::var_os("PERRY_GC_DIAG").is_some() { + eprintln!( + "[gc-safepoint-drain] kind={} count={}", + kind.as_str(), + count + ); + } +} + +#[cfg(test)] +pub(crate) fn scan_fallback_count(site: ConservativeScanSite) -> u64 { + SCAN_FALLBACKS.with(|c| c.get()[site.index()]) +} + +/// Total conservative-scan fallbacks across the four **automatic** sites. This +/// is the number #7148 wants driven to zero: explicit `gc()` is a user request +/// and is not part of the claim. +#[cfg(test)] +pub(crate) fn automatic_scan_fallback_total() -> u64 { + SCAN_FALLBACKS.with(|c| { + let counts = c.get(); + ConservativeScanSite::ALL + .iter() + .filter(|site| site.is_automatic()) + .map(|site| counts[site.index()]) + .sum() + }) +} + +#[cfg(test)] +pub(crate) fn safepoint_drain_count(kind: SafepointDrainKind) -> u64 { + SAFEPOINT_DRAINS.with(|c| c.get()[kind.index()]) +} + +/// Reset both counter families. Test-only: the counters are process-lifetime +/// on a real run. +#[cfg(test)] +pub(crate) fn reset_scan_fallback_counters() { + SCAN_FALLBACKS.with(|c| c.set([0; ConservativeScanSite::COUNT])); + SAFEPOINT_DRAINS.with(|c| c.set([0; SafepointDrainKind::COUNT])); +} diff --git a/crates/perry-runtime/src/gc/tests/mod.rs b/crates/perry-runtime/src/gc/tests/mod.rs index a5940470cb..52026c1819 100644 --- a/crates/perry-runtime/src/gc/tests/mod.rs +++ b/crates/perry-runtime/src/gc/tests/mod.rs @@ -20,6 +20,7 @@ mod os_tag; mod root_words; mod roots; mod runtime_roots; +mod scan_fallback; mod shadow_stack_ops; mod smoke; pub(super) mod support; diff --git a/crates/perry-runtime/src/gc/tests/roots.rs b/crates/perry-runtime/src/gc/tests/roots.rs index 82809bea56..58a74a1583 100644 --- a/crates/perry-runtime/src/gc/tests/roots.rs +++ b/crates/perry-runtime/src/gc/tests/roots.rs @@ -816,7 +816,8 @@ fn manual_gc_scan_guard_forces_full_scan_only_when_unpinned() { // Unpinned: the guard engages a Full override for its lifetime (#4977 — // explicit gc() must see top-level locals held only on the native stack). { - let _scan = ManualGcScanGuard::force_full_scan(); + let _scan = + ManualGcScanGuard::force_full_scan(crate::gc::ConservativeScanSite::ManualCollect); assert_eq!( CONSERVATIVE_STACK_SCAN_OVERRIDE.with(|c| c.get()), Some(ConservativeStackScanMode::Full) @@ -828,7 +829,8 @@ fn manual_gc_scan_guard_forces_full_scan_only_when_unpinned() { // reclaim native-stack locals): the guard must not replace the override. set_conservative_stack_scan_override(Some(ConservativeStackScanMode::Auto)); { - let _scan = ManualGcScanGuard::force_full_scan(); + let _scan = + ManualGcScanGuard::force_full_scan(crate::gc::ConservativeScanSite::ManualCollect); assert_eq!( CONSERVATIVE_STACK_SCAN_OVERRIDE.with(|c| c.get()), Some(ConservativeStackScanMode::Auto) diff --git a/crates/perry-runtime/src/gc/tests/scan_fallback.rs b/crates/perry-runtime/src/gc/tests/scan_fallback.rs new file mode 100644 index 0000000000..e0269aff3f --- /dev/null +++ b/crates/perry-runtime/src/gc/tests/scan_fallback.rs @@ -0,0 +1,360 @@ +//! #7148: the conservative-scan fallback must be *unreachable*, not imprecise. +//! +//! Each test below asserts BOTH halves of a deferral claim, because only the +//! first half is cheap to fake: +//! +//! 1. the conservative valve did **not** fire (`scan_fallback_count == 0`), and +//! 2. the precise safepoint collection that replaced it **did** +//! (`safepoint_drain_count > 0`). +//! +//! CLAUDE.md's fourth way a gate cannot fail is "the gate runs but its subject +//! never did". A test that only checked (1) would pass on a tree where the +//! trigger never armed at all — the strongest possible regression reported as +//! a pass. Every deferral test here therefore ends on a live-subject counter. + +use super::super::*; +use super::support::*; + +/// Make `gc_budgeted_due_trigger()` report `OldReclaim` without allocating +/// 48 MB of old-gen: `GC_OLD_RECLAIM_PENDING` is the sticky "a full old-gen +/// reclaim is owed" flag it reads first (the same lever +/// `budgeted_step_api.rs` uses). +fn arm_old_reclaim() { + GC_OLD_RECLAIM_PENDING.with(|pending| pending.set(true)); +} + +fn clear_old_reclaim_state() { + GC_OLD_RECLAIM_PENDING.with(|pending| pending.set(false)); + GC_SAFEPOINT_PENDING.with(|pending| pending.set(false)); + let old_in_use = crate::arena::old_gen_in_use_bytes(); + GC_LAST_OLD_RECLAIM_IN_USE_BYTES.with(|bytes| bytes.set(old_in_use)); +} + +#[test] +fn old_reclaim_runs_precisely_at_a_safepoint() { + let _isolation = GcTestIsolationGuard::new(); + let _pacing = crate::gc::policy::force_moving_gc_pacing(); + let _triggers = GcTriggerThresholdTestGuard::suppress_automatic_triggers(); + clear_old_reclaim_state(); + reset_scan_fallback_counters(); + + // `gc_safepoint_moving_minor` used to bail on OldReclaim ("stays on its + // existing full mark-sweep path"), so the ONLY place an old-gen reclaim + // could happen was the allocation point, behind `force_full_scan()`. + arm_old_reclaim(); + GC_SAFEPOINT_PENDING.with(|p| p.set(true)); + js_gc_loop_safepoint(); + + assert_eq!( + safepoint_drain_count(SafepointDrainKind::OldReclaim), + 1, + "LIVE SUBJECT: the full mark-sweep must actually have run at the \ + safepoint — 'nothing scanned' is worthless if nothing collected" + ); + assert_eq!( + automatic_scan_fallback_total(), + 0, + "the safepoint path has precise roots, so it must not force the scan" + ); + assert!( + !GC_OLD_RECLAIM_PENDING.with(std::cell::Cell::get), + "the safepoint collection retires the request, so the allocation-point \ + arm finds nothing due and never runs a second, conservative cycle" + ); + + clear_old_reclaim_state(); +} + +#[test] +fn old_reclaim_alloc_point_still_completes_immediately_and_is_counted() { + let _isolation = GcTestIsolationGuard::new(); + let _pacing = crate::gc::policy::force_moving_gc_pacing(); + let _triggers = GcTriggerThresholdTestGuard::suppress_automatic_triggers(); + clear_old_reclaim_state(); + reset_scan_fallback_counters(); + + // The other direction, and the reason this site is NOT deferred. #5476's + // own regression test asserts that a single `gc_check_trigger` call — what + // every allocation does — drives the reclaim to completion, because the + // workload it was filed for is a compute-only loop that reaches no host + // step. Deferring here would turn "RSS climbs unbounded" back on for one + // growth quantum. What #7148 changes is only that the cost is now visible. + let collections_before = gc_collection_count(); + arm_old_reclaim(); + gc_check_trigger(); + + assert!( + gc_collection_count() > collections_before, + "a single gc_check_trigger call must still complete the reclaim (#5476)" + ); + assert_eq!( + scan_fallback_count(ConservativeScanSite::OldReclaimAllocPoint), + 1, + "and it is still a conservative-scan collection — counted, not hidden" + ); + + clear_old_reclaim_state(); +} + +#[test] +fn old_reclaim_is_unchanged_when_moving_loop_polls_are_off() { + // #7161 proposes flipping `PERRY_GC_MOVING_LOOP_POLLS` OFF as a stopgap for + // #7154. The allocation-point arm must not consult that gate at all, so + // that the flip can never make this site behave differently — inert, not + // unsound. (The precise safepoint path is simply reached less often, which + // shows up as a higher census count, not as a correctness change.) + let _isolation = GcTestIsolationGuard::new(); + let _pacing = crate::gc::policy::force_legacy_gc_pacing(); + let _triggers = GcTriggerThresholdTestGuard::suppress_automatic_triggers(); + clear_old_reclaim_state(); + reset_scan_fallback_counters(); + + let collections_before = gc_collection_count(); + arm_old_reclaim(); + gc_check_trigger(); + + assert!( + gc_collection_count() > collections_before, + "polls off must not stop the allocation-point reclaim completing" + ); + assert_eq!( + scan_fallback_count(ConservativeScanSite::OldReclaimAllocPoint), + 1, + "identical behaviour and identical census to the polls-on arm" + ); + + clear_old_reclaim_state(); +} + +#[test] +fn host_pressure_collects_precisely_when_no_generated_frame_is_live() { + let _isolation = GcTestIsolationGuard::new(); + let _pacing = crate::gc::policy::force_moving_gc_pacing(); + clear_old_reclaim_state(); + reset_shadow_stack(); + reset_scan_fallback_counters(); + + assert!( + !crate::gc::roots::shadow_stack_has_active_frame(), + "precondition: the run-loop-boundary case has an empty shadow stack" + ); + + let result = js_gc_memory_pressure(1); + + assert_eq!(result, 2, "collected synchronously"); + assert_eq!( + automatic_scan_fallback_total(), + 0, + "with no generated frame live the precise root set is complete, so \ + the host-pressure collection must not force the scan (the site has no \ + conservative arm left at all — this catches its reintroduction)" + ); + assert_eq!( + safepoint_drain_count(SafepointDrainKind::HostPressure), + 1, + "LIVE SUBJECT: the precise collection ran" + ); + + clear_old_reclaim_state(); +} + +#[test] +fn host_pressure_defers_when_a_generated_frame_is_live() { + let _isolation = GcTestIsolationGuard::new(); + let _pacing = crate::gc::policy::force_moving_gc_pacing(); + let _triggers = GcTriggerThresholdTestGuard::suppress_automatic_triggers(); + clear_old_reclaim_state(); + reset_shadow_stack(); + reset_scan_fallback_counters(); + + let frame = js_shadow_frame_push(2); + assert!(crate::gc::roots::shadow_stack_has_active_frame()); + + // Critical level: the owed collection must be a FULL cycle, so the + // deferral has to arm the sticky old-gen-reclaim request rather than only + // the nursery one. + let result = js_gc_memory_pressure(2); + js_shadow_frame_pop(frame); + + assert_eq!( + result, 1, + "documented return code for 'trigger lowered, collection deferred'" + ); + assert_eq!( + automatic_scan_fallback_total(), + 0, + "a live generated frame means a safepoint is reachable — defer to it \ + instead of scanning conservatively" + ); + assert!(GC_SAFEPOINT_PENDING.with(std::cell::Cell::get)); + assert!( + GC_OLD_RECLAIM_PENDING.with(std::cell::Cell::get), + "level >= 2 must owe a FULL cycle, not a minor" + ); + + // The frame is gone; drain the promise. + js_gc_loop_safepoint(); + assert_eq!( + safepoint_drain_count(SafepointDrainKind::OldReclaim), + 1, + "LIVE SUBJECT: the deferred critical-pressure full cycle ran at the \ + safepoint" + ); + + clear_old_reclaim_state(); +} + +#[test] +fn explicit_gc_still_scans_and_the_census_attributes_it() { + let _isolation = GcTestIsolationGuard::new(); + clear_old_reclaim_state(); + reset_scan_fallback_counters(); + + js_gc_collect(); + + // #7148 deliberately does NOT defer explicit `gc()`: it is a user request + // with synchronous semantics. What changes is that its cost is now + // attributable — every `gc_ratchet` probe ends with one of these, so a + // census that lumped it in with the automatic sites would misread. + assert!( + scan_fallback_count(ConservativeScanSite::ManualCollect) >= 1, + "explicit gc() keeps the scan and must be counted as non-automatic" + ); + assert_eq!( + automatic_scan_fallback_total(), + 0, + "an explicit gc() must not be counted against the automatic-site total \ + that #7148 is driving to zero" + ); + + clear_old_reclaim_state(); +} + +#[test] +fn conservative_scan_env_full_does_not_override_a_pinned_test_mode() { + // `PERRY_CONSERVATIVE_STACK_SCAN=full` failed 134 of 1574 runtime tests + // because the env value beat the per-thread override, so a test that had + // declared its roots precise still got the conservative scan and its + // "this should have been collected" assertion broke. The env var may now + // make the scan LESS aggressive than a test declared, never more. + let _env = EnvVarGuard::set("PERRY_CONSERVATIVE_STACK_SCAN", "full"); + let previous = crate::gc::roots::set_conservative_stack_scan_override(None); + + // Unpinned: the ops escape hatch still works. This is the arm the + // `gc_ratchet` sensitivity run depends on — production binaries have no + // pinned override, so `=full` still forces the scan there. + assert_eq!( + crate::gc::roots::conservative_stack_scan_decision(), + ConservativeStackScanDecision::Scan, + "with nothing pinned, =full must still force the scan" + ); + + // Pinned by a test isolation guard: the declared mode wins. + crate::gc::roots::set_conservative_stack_scan_override(Some(ConservativeStackScanMode::Auto)); + assert_eq!( + crate::gc::roots::conservative_stack_scan_decision(), + ConservativeStackScanDecision::SkipDisabled, + "a pinned Auto must beat an ambient =full" + ); + + crate::gc::roots::set_conservative_stack_scan_override(Some( + ConservativeStackScanMode::Disabled, + )); + assert_eq!( + crate::gc::roots::conservative_stack_scan_decision(), + ConservativeStackScanDecision::SkipDisabled, + "a pinned Disabled must beat an ambient =full" + ); + + crate::gc::roots::set_conservative_stack_scan_override(previous); +} + +#[test] +fn conservative_scan_env_off_still_beats_a_forced_scan() { + // The other precedence direction, unchanged by #7148: `=0` is the + // bisection escape hatch, and it must keep winning over + // `ManualGcScanGuard::force_full_scan()` (which pins `Full`). Narrowing + // the #7148 rule to "env asking for Full" is what preserves this. + let _env = EnvVarGuard::set("PERRY_CONSERVATIVE_STACK_SCAN", "0"); + let previous = crate::gc::roots::set_conservative_stack_scan_override(None); + + crate::gc::roots::set_conservative_stack_scan_override(Some(ConservativeStackScanMode::Full)); + assert_eq!( + crate::gc::roots::conservative_stack_scan_decision(), + ConservativeStackScanDecision::SkipDisabled, + "=0 must still disable the scan even when a guard pinned Full" + ); + + crate::gc::roots::set_conservative_stack_scan_override(previous); +} + +#[test] +fn host_pressure_precise_collection_does_not_depend_on_moving_loop_polls() { + // The host-pressure win that matters — collecting with precise roots when + // no generated frame is live — is reached without consulting + // `PERRY_GC_MOVING_LOOP_POLLS` at all, so #7161's proposed flip cannot make + // this site scan conservatively again. + let _isolation = GcTestIsolationGuard::new(); + let _pacing = crate::gc::policy::force_legacy_gc_pacing(); + clear_old_reclaim_state(); + reset_shadow_stack(); + reset_scan_fallback_counters(); + + let result = js_gc_memory_pressure(1); + + assert_eq!(result, 2, "collected synchronously with polls off"); + assert_eq!( + automatic_scan_fallback_total(), + 0, + "still no conservative scan with polls off" + ); + assert_eq!( + safepoint_drain_count(SafepointDrainKind::HostPressure), + 1, + "LIVE SUBJECT: the precise collection ran" + ); + + clear_old_reclaim_state(); +} + +#[test] +fn host_pressure_deferral_still_has_a_drain_when_moving_loop_polls_are_off() { + // With polls off `js_gc_loop_safepoint` is a no-op, so the frame-live + // deferral must not depend on it. Two backstops survive #7161: the + // outermost microtask-pump boundary (gated by `PERRY_GC_MOVING_SAFEPOINT`, + // a different knob) and the lowered arena trigger, which makes the ordinary + // allocation-point arm collect at the next check. This test pins the second + // one, because it is the one that holds even for a program that never + // yields to the event loop. + let _isolation = GcTestIsolationGuard::new(); + let _pacing = crate::gc::policy::force_legacy_gc_pacing(); + clear_old_reclaim_state(); + reset_shadow_stack(); + reset_scan_fallback_counters(); + + let frame = js_shadow_frame_push(2); + let result = js_gc_memory_pressure(2); + js_shadow_frame_pop(frame); + + assert_eq!(result, 1, "deferred, as with polls on"); + assert_eq!( + automatic_scan_fallback_total(), + 0, + "deferring never scans, whatever the pacing mode" + ); + assert!( + GC_OLD_RECLAIM_PENDING.with(std::cell::Cell::get), + "the owed FULL cycle is still armed, so the next allocation-point \ + trigger check collects it even with no safepoint machinery running" + ); + + // The polls-off backstop: an ordinary trigger check completes it. + let collections_before = gc_collection_count(); + gc_check_trigger(); + assert!( + gc_collection_count() > collections_before, + "LIVE SUBJECT: the deferred critical-pressure cycle actually ran" + ); + + clear_old_reclaim_state(); +}