Skip to content
27 changes: 27 additions & 0 deletions benchmarks/gc_ratchet/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
19 changes: 19 additions & 0 deletions changelog.d/7166-gc-safepoint-deferral-scan-census.md
Original file line number Diff line number Diff line change
@@ -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:

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Fix the Markdown issue reference.

Line 5 starts with #7148 without a space, which triggers markdownlint MD018. Prefix the reference with Issue or escape the hash.

Proposed fix
-#7148 asks for the fallback to become **unreachable**, not *imprecise*.
+Issue `#7148` asks for the fallback to become **unreachable**, not *imprecise*.
🧰 Tools
🪛 markdownlint-cli2 (0.23.1)

[warning] 5-5: No space after hash on atx style heading

(MD018, no-missing-space-atx)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@changelog.d/7166-gc-safepoint-deferral-scan-census.md` at line 5, Update the
Markdown text in the changelog entry so the issue reference no longer begins
with an unescaped hash; prefix `#7148` with descriptive text such as “Issue” while
preserving the existing meaning.

Source: Linters/SAST tools


- **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).
25 changes: 24 additions & 1 deletion crates/perry-runtime/src/gc/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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<bool> = const { std::cell::Cell::new(false) };
Expand All @@ -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
Expand Down
Loading
Loading