perf(gc): pace the full mark-sweep on what a collection failed to reclaim, not on allocation volume (tree 1.167 -> 0.595, tree_wide 1.647 -> 1.075)

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.

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

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<usize> = 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<usize> =
+        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,
     }
 }
 

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",

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"
+    );
+}