From 26c9f5995376a6d0234b35853062667654494078 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Tue, 11 Aug 2026 18:51:56 +0200 Subject: [PATCH] perf(gc): pace the full-mark-sweep escalation on what a collection failed to reclaim --- .../7869-gc-pacing-escalation-reading.md | 51 +++++++++++++++ crates/perry-runtime/src/gc/cycle.rs | 4 ++ crates/perry-runtime/src/gc/policy.rs | 65 ++++++++++++++++++- crates/perry-runtime/src/gc/telemetry.rs | 7 ++ crates/perry-runtime/src/gc/tests/triggers.rs | 64 ++++++++++++++++++ 5 files changed, 190 insertions(+), 1 deletion(-) create mode 100644 changelog.d/7869-gc-pacing-escalation-reading.md diff --git a/changelog.d/7869-gc-pacing-escalation-reading.md b/changelog.d/7869-gc-pacing-escalation-reading.md new file mode 100644 index 0000000000..9ce80169ac --- /dev/null +++ b/changelog.d/7869-gc-pacing-escalation-reading.md @@ -0,0 +1,51 @@ +### Fixed: arena-growth pacing escalated on allocation volume, not on unreclaimed bytes + +`gc-handoff/bench/tree.ts` and `tree_wide.ts` — the two slowest programs in the GC +benchmark corpus — spent **more than half their runtime in full mark-sweeps**, and the +copying minor that would have reclaimed the same bytes was never attempted. All 40 of +each program's collections were fulls (`PERRY_GC_TRACE=1`: +`{'full': 40}`, `copying_nursery.eligible: false`, `fallback_reason: "not_attempted"`). + +**Root cause.** `arena_growth_full_escalation_due` escalates a minor to a full once the +arena reading passes `max(PERRY_GC_MAJOR_PACING_FLOOR_MB, K × baseline)`. The baseline +(`GC_LAST_FULL_ARENA_IN_USE_BYTES`) is measured *after* a full, so it is LIVE bytes; the +reading was `arena_in_use_bytes()` sampled when the trigger fires, which is ALLOCATED +bytes — the entire un-collected nursery, nearly all of it garbage a minor is about to +reclaim for free. The two sides of the comparison were different kinds of quantity. +`tree`'s nursery high-water is 37.7 MB against the 32 MB floor, so every cycle escalated, +and the escalation perpetuated itself: `note_copying_minor_young_survival` is the only +thing that can widen the band, and it runs only when a copying minor runs. + +**Fix.** The escalation now tests the arena occupancy recorded at the **end of the last +collection** — the same kind of quantity as its baseline, and precisely what the +escalation exists to detect: bytes a collection could not reclaim. Array-growth +forwarding stubs, the hazard the escalation was written for, pin their blocks through a +non-moving minor and so remain in that reading and still escalate; nursery garbage does +not. Recorded once per cycle in `GcCycle::publish_reclaim_outcome`, the one site both +collection kinds pass through. + +**Measured** (absolute seconds, quiet M1 mini, best-of-5, exit-checked, window verified +quiet at both ends; 22 programs byte-identical to `node --experimental-strip-types` and +exit 0 on both arms before timing): + +| bench | main | after | delta | node | +|---|--:|--:|--:|--:| +| `tree` | 1.1671 | **0.5947** | **−49.0%** | 0.450 | +| `tree_wide` | 1.6468 | **1.0747** | **−34.7%** | 0.896 | +| every other of the 22 | | | within ±0.7% | | + +`ns` per GC-managed allocation on `tree`: **55.65 → 28.36**. `churn_alloc` (12.03 → 12.02) +and `retain` (89.17 → 89.37) are unchanged *by construction* — identical cycle counts, +identical collection kinds, and identical `promoted_objects + copied_objects` +(47,707 and 2,358,760) on both arms. + +**Peak RSS falls**: `tree` 57.0 → 45.0 MB (−21.0%), `tree_wide` 82.1 → 70.3 MB (−14.4%), +every other program within ±0.2%. A full mark-sweep's non-moving reclaim leaves the arena +fragmented; the copying minor returns whole blocks. + +The GC trace now emits `major_pacing.escalation_reading_bytes` — the left-hand side of the +comparison, previously invisible, which is how a post-full live baseline could be tested +against a pre-collection allocated reading with nothing in the trace saying so. The new +unit test asserts **both** directions: an arena the last collection emptied must not +escalate, and an arena still at the floor after a collection must still escalate — only +the pair distinguishes this from switching the escalation off. diff --git a/crates/perry-runtime/src/gc/cycle.rs b/crates/perry-runtime/src/gc/cycle.rs index 456459af96..e5a49db648 100644 --- a/crates/perry-runtime/src/gc/cycle.rs +++ b/crates/perry-runtime/src/gc/cycle.rs @@ -1898,6 +1898,10 @@ impl GcCycleState { if self.minor.is_none() { finish_full_old_reclaim_baseline(); } + // #7865: arena-growth pacing tests a POST-collection occupancy, which + // is the same kind of quantity as its post-full baseline. Recorded here + // rather than per-kind because this is the one site both kinds reach. + super::policy::note_collection_finished_arena_occupancy(); let malloc_swept = self .minor diff --git a/crates/perry-runtime/src/gc/policy.rs b/crates/perry-runtime/src/gc/policy.rs index 2e6889344f..057831a9cb 100644 --- a/crates/perry-runtime/src/gc/policy.rs +++ b/crates/perry-runtime/src/gc/policy.rs @@ -890,6 +890,30 @@ thread_local! { /// Paired with `GC_LAST_FULL_ARENA_IN_USE_BYTES` to price what that full /// actually reclaimed — see `GC_MAJOR_PACING_BACKOFF_SHIFT`. pub(super) static GC_FULL_CYCLE_PRE_IN_USE_BYTES: Cell = const { Cell::new(0) }; + /// Total arena in-use bytes measured at the END of the most recent + /// collection of ANY kind — the reading arena-growth pacing tests against + /// its boundary (#7865). + /// + /// The pacing baseline (`GC_LAST_FULL_ARENA_IN_USE_BYTES`) is a *post*-full + /// reading, i.e. LIVE bytes. Testing it against `arena_in_use_bytes()` at + /// the moment a trigger fires compared it against *allocated* bytes — the + /// entire un-collected nursery, most of which is garbage a minor is about + /// to reclaim for free. On `gc-handoff/bench/tree.ts` that reading is + /// 37.7 MB against a 32 MB floor on **every** cycle, so all 40 collections + /// escalated to a whole-heap mark-sweep and the copying minor was never + /// even attempted (`copying_nursery.eligible: false`, + /// `fallback_reason: "not_attempted"`). The escalation then perpetuates + /// itself: `note_copying_minor_young_survival` is the only thing that can + /// widen the band, and it only runs when a copying minor runs. + /// + /// A post-collection reading is the same *kind* of quantity as the + /// baseline, and it says exactly what the escalation exists to detect: + /// **bytes the last minor could not reclaim.** Array-growth forwarding + /// stubs — the hazard `arena_growth_full_escalation_due` was written for — + /// pin their blocks through a non-moving minor, so they are still in this + /// reading and still escalate. Nursery garbage is not. + pub(super) static GC_LAST_COLLECTION_POST_IN_USE_BYTES: Cell = + const { Cell::new(0) }; /// Yield-adaptive backoff for major-GC pacing (#7726). /// /// `arena_growth_full_escalation_due` escalates a minor to a full once the @@ -1718,6 +1742,34 @@ fn pacing_arena_in_use_bytes() -> usize { crate::arena::arena_in_use_bytes() } +/// Record the post-collection arena occupancy arena-growth pacing tests +/// against. Called once at the end of every cycle, minor and full alike, from +/// `GcCycle::publish_reclaim_outcome` — the single site both kinds pass +/// through, so a future collection kind cannot forget it. +pub(super) fn note_collection_finished_arena_occupancy() { + let bytes = pacing_arena_in_use_bytes(); + GC_LAST_COLLECTION_POST_IN_USE_BYTES.with(|cell| cell.set(bytes)); +} + +/// The arena reading [`arena_growth_full_escalation_due`] tests — see +/// [`GC_LAST_COLLECTION_POST_IN_USE_BYTES`]. +/// +/// Zero before any collection has finished, so the very first collection of a +/// process is never escalated: there is no evidence yet that a minor would +/// fail to reclaim, and the whole point of the pacing is to fire on that +/// evidence rather than on allocation volume. +/// +/// Keeps the `#[cfg(test)]` seam in front, so the existing positive-direction +/// tests that force a `true` verdict out of the real predicate keep working +/// without a 32 MB live heap. +pub(super) fn pacing_escalation_reading_bytes() -> usize { + #[cfg(test)] + if let Some(bytes) = TEST_PACING_ARENA_IN_USE.with(|cell| cell.get()) { + return bytes; + } + GC_LAST_COLLECTION_POST_IN_USE_BYTES.with(|cell| cell.get()) +} + #[cfg(test)] thread_local! { /// Test-only override for [`pacing_arena_in_use_bytes`]. Thread-local, so @@ -1769,6 +1821,17 @@ pub(super) fn test_major_pacing_pre_in_use_bytes() -> usize { GC_FULL_CYCLE_PRE_IN_USE_BYTES.with(|bytes| bytes.get()) } +/// Override the post-collection occupancy the escalation predicate reads. +/// Returns the previous value so a test can restore it. +#[cfg(test)] +pub(super) fn test_set_collection_post_in_use_bytes(bytes: usize) -> usize { + GC_LAST_COLLECTION_POST_IN_USE_BYTES.with(|cell| { + let previous = cell.get(); + cell.set(bytes); + previous + }) +} + #[cfg(test)] pub(super) fn test_set_major_pacing_baseline(bytes: usize) -> usize { GC_LAST_FULL_ARENA_IN_USE_BYTES.with(|cell| { @@ -2889,7 +2952,7 @@ fn arena_growth_full_escalation_due_inner() -> bool { // `PERRY_GC_MAJOR_PACING_FLOOR_MB=0` disables the pacing: no reading // escalates, so there is no boundary to compare against. None => false, - Some(threshold) => pacing_arena_in_use_bytes() >= threshold, + Some(threshold) => pacing_escalation_reading_bytes() >= threshold, } } diff --git a/crates/perry-runtime/src/gc/telemetry.rs b/crates/perry-runtime/src/gc/telemetry.rs index 06e36e76c2..bd95343532 100644 --- a/crates/perry-runtime/src/gc/telemetry.rs +++ b/crates/perry-runtime/src/gc/telemetry.rs @@ -1186,6 +1186,13 @@ impl GcCycleTrace { // survival-adaptive band is indistinguishable from one that did and // simply had nothing to skip. "retaining": super::policy::major_pacing_retaining(), + // #7865: the reading actually compared against + // `escalate_at_or_above_bytes`. Emitted because the two used to be + // different KINDS of quantity — a post-full live baseline against a + // pre-collection allocated reading — and nothing in the trace said + // so. A gate that cannot see the left-hand side cannot prove which + // way the comparison went. + "escalation_reading_bytes": super::policy::pacing_escalation_reading_bytes(), }); serde_json::json!({ "event": "gc_cycle", diff --git a/crates/perry-runtime/src/gc/tests/triggers.rs b/crates/perry-runtime/src/gc/tests/triggers.rs index 2319f3bba3..d4444075ca 100644 --- a/crates/perry-runtime/src/gc/tests/triggers.rs +++ b/crates/perry-runtime/src/gc/tests/triggers.rs @@ -1363,3 +1363,67 @@ fn retaining_rebaseline_never_lowers_the_pacing_baseline() { "a larger post-minor occupancy must raise it" ); } + +/// #7865 — arena-growth pacing must escalate on **bytes a collection could not +/// reclaim**, not on allocation volume. +/// +/// The baseline (`GC_LAST_FULL_ARENA_IN_USE_BYTES`) is a post-full reading, so +/// it is LIVE bytes. Testing it against `arena_in_use_bytes()` at the moment a +/// trigger fires compared it against ALLOCATED bytes — the whole un-collected +/// nursery. `gc-handoff/bench/tree.ts` reads 37.7 MB against the 32 MB floor on +/// every cycle, so all 40 of its collections escalated to a whole-heap +/// mark-sweep (1.76 s of pause on the dev host) and the copying minor that +/// would have reclaimed the same bytes was never attempted. Worse, the +/// escalation perpetuates itself: `note_copying_minor_young_survival` is the +/// only thing that can widen the band, and it runs only when a copying minor +/// runs. +/// +/// Both directions are asserted, because only the pair distinguishes the fix +/// from "escalation switched off": a heap the last collection LEFT full still +/// escalates — that is the array-growth-forwarding-stub case the escalation was +/// written for, and stubs survive a non-moving minor precisely by staying in +/// this reading. +#[test] +fn escalation_reads_what_the_last_collection_failed_to_reclaim() { + use super::super::policy::{ + arena_growth_full_escalation_due, major_pacing_config, test_reset_major_pacing_backoff, + test_set_collection_post_in_use_bytes, test_set_major_pacing_baseline, + test_set_pacing_arena_in_use, + }; + + let (floor_bytes, _growth_num) = major_pacing_config(); + if floor_bytes == 0 { + return; // pacing disabled outright: no boundary to test + } + + // The `#[cfg(test)]` injection seam short-circuits the real reading, so it + // has to be OFF for this test to exercise the path it is about. + let previous_seam = test_set_pacing_arena_in_use(None); + test_reset_major_pacing_backoff(); + let previous_baseline = test_set_major_pacing_baseline(0); // boundary == floor + + // A nursery-churn workload: the last collection emptied the arena. The + // program may have allocated gigabytes since; none of it is evidence that a + // full is needed. + let previous_post = test_set_collection_post_in_use_bytes(0); + let emptied_due = arena_growth_full_escalation_due(); + + // A stub-pinned workload: the last collection ran and the arena is STILL at + // the floor. That is the escalation's subject and it must still fire. + test_set_collection_post_in_use_bytes(floor_bytes); + let retained_due = arena_growth_full_escalation_due(); + + test_set_collection_post_in_use_bytes(previous_post); + test_set_major_pacing_baseline(previous_baseline); + test_set_pacing_arena_in_use(previous_seam); + test_reset_major_pacing_backoff(); + + assert!( + !emptied_due, + "a collection that emptied the arena must not escalate the next one" + ); + assert!( + retained_due, + "an arena still at the floor after a collection must still escalate" + ); +}