fix(gc): credit every promotion to the old-reclaim baseline, untraced or not (#7965)

changelog.d/7968-old-reclaim-baseline-credit.md

@@ -0,0 +1,53 @@
+### Fixed
+
+- **GC: credit every promotion to the old-reclaim baseline, untraced or not (#7965).**
+  #7902 made a copying minor skip `credit_promoted_bytes_to_old_baseline` for an
+  untraced whole-block promotion, on the grounds that "live by construction" is a
+  marked-liveness claim `PromotionLiveness::AssumeAllLive` does not make. The
+  premise is right and the conclusion does not follow, because
+  `GC_LAST_OLD_RECLAIM_IN_USE_BYTES` is **not a liveness claim** — it is the base
+  of a growth measurement. `old_in_use - baseline` is meant to read "how much has
+  old-gen grown since the last reclaim decision", and bytes a minor has just
+  relocated there are growth that decision has already seen.
+
+  A fully-live young generation promotes untraced on *every* cycle, so on exactly
+  the workloads that reach this path nothing else credits the baseline and it
+  stays pinned at 0. `old_in_use - baseline` then collapses into absolute
+  occupancy, and `gc_old_reclaim_growth_band_bytes`'s proportional half
+  (`baseline / OLD_RECLAIM_GROWTH_DIVISOR`) collapses with it, leaving the
+  constant floor — **a constant band pacing a collector whose per-cycle cost is
+  O(live)**, which is the quadratic shape #7592 removed at this trigger and #7594
+  removed one generation down. The pin re-created it.
+
+  Bisected on the **full count** over `8260a9e50..d78efca41`: `0809ada92 →
+  e886b56dd` (#7900 + #7901) is flat at ≤0.6% and both are exonerated;
+  `e886b56dd → 1bd5eeb6b` (#7902) carries the whole regression. The
+  granularity hypothesis originally attached to #7965 — that the absolute arm is
+  decided by promotion *step size* — is refuted here by `deeplist`'s
+  untraced-promotion trace being byte-identical across the boundary.
+
+  Measured on the `gc-handoff` corpus, 19/19 byte-exact against the expected
+  output and exit 0 in both arms: `retain` **1 full → 0** and 8 237 M → 2 176 M
+  instructions retired (−73.6%) with peak RSS 311 → 249 MB; `retain_wide`
+  **1 full → 0** and 5 741 M → 2 859 M (−50.2%). No program gains a cycle or a
+  full, and the other 14 move by at most 0.15%. Against the pre-regression base
+  the arm lands *below* it (`retain` −23.4%, `retain1` −31.5%, `deeplist`
+  −17.5%), which is #7960's independently-reported figure and the arithmetic
+  check that the regression is gone rather than masked.
+
+  #7902's three other changes are kept — the bounded, heap-scaled untraced
+  budget, the clamped `implied_dead_bytes` charge, and
+  `request_old_reclaim_for_untraced_promotions`. They close a real defect and
+  none of them paces on this quantity: each acts on *evidence about the
+  promoted cohort*, whereas a pinned pacing base acts on every program that
+  retains.
+
+  Gated by `an_untraced_promotion_credits_the_old_reclaim_baseline`, which
+  drives a real untraced-promoting copying minor through the production entry
+  point and asserts the baseline advanced by exactly the bytes it relocated
+  (with the untraced counters asserted non-zero, so an evacuating or traced
+  cycle cannot satisfy it), then replays `retain`'s measured promotion series
+  and asserts both that the credited baseline keeps `old_reclaim_pressure_due`
+  false **and** that the same series against an uncredited baseline fires. That
+  second assertion caught its own first draft, which did not cross the band
+  while the `RETAINING` latch was armed.

crates/perry-runtime/src/gc/copying.rs

@@ -1854,20 +1854,17 @@ pub(super) fn run_copied_minor_attempt(
         collector.stats.copied_objects,
         collector.stats.promoted_objects,
     );
-    // #7592: promoted bytes are live by construction — credit them to the
-    // old-reclaim baseline BEFORE the pressure check below, or the check reads
-    // the stale baseline and schedules a full that is guaranteed to free
-    // nothing (see `credit_promoted_bytes_to_old_baseline`).
+    // #7592: credit the bytes this minor moved into old-gen to the old-reclaim
+    // baseline BEFORE the pressure check below, or the check reads a stale
+    // baseline and schedules a full that is guaranteed to free nothing (see
+    // `credit_promoted_bytes_to_old_baseline`).
     //
-    // #7902: "live by construction" is a MARKED-liveness claim, and an untraced
-    // promotion makes none — it uses `PromotionLiveness::AssumeAllLive`. Those
-    // bytes are the uncertain class the untraced budget bounds, so crediting
-    // them here would tell old-reclaim pacing that a cohort nobody has looked
-    // at is clean, and defer the very collection that could decide it. Charge
-    // only what a traced cycle actually marked.
-    if !untraced {
-        credit_promoted_bytes_to_old_baseline(collector.stats.promoted_bytes);
-    }
+    // #7965: UNCONDITIONAL, including for an untraced promotion — see
+    // `credit_promoted_bytes_to_old_baseline`, which carries the argument. In
+    // one line: the baseline is the base of a GROWTH measurement, not a
+    // liveness claim, and withholding it pins that base at 0 on exactly the
+    // workloads that reach this path.
+    credit_promoted_bytes_to_old_baseline(collector.stats.promoted_bytes);
     // Everything outside from-space retains its pre-minor accounting. Remove
     // the from-space share of that accounting, then add back exactly the
     // objects that survived by copy or promotion. This also preserves objects

crates/perry-runtime/src/gc/policy.rs

@@ -1577,6 +1577,39 @@ pub(super) fn copied_minor_promotion_handoff_due(trigger_kind: GcTriggerKind) ->
 /// quickly now wait for the growth band instead of the next threshold
 /// crossing. That is deliberate — a promoted-then-dead cohort big enough to
 /// matter moves the band by its own size.
+///
+/// # ★ Every promotion is credited, including an UNTRACED one (#7965)
+///
+/// #7902 made the call site skip this for a `PromotionLiveness::AssumeAllLive`
+/// promotion, reasoning that "live by construction" is a marked-liveness claim
+/// an untraced cycle does not make. The premise is right and the conclusion
+/// does not follow, because **this baseline is not a liveness claim**. It is
+/// the base of a growth measurement: `old_in_use - baseline` is meant to read
+/// "how much has old-gen grown since the last reclaim decision", and bytes a
+/// minor has just relocated there are growth that decision has already seen.
+///
+/// Withholding it does not defer a little reclamation, it degenerates the
+/// predicate. A fully-live young generation promotes untraced on *every*
+/// cycle, so on exactly the workloads that reach the untraced path nothing
+/// else credits the baseline and it stays pinned at 0. Then
+/// `old_in_use - baseline` collapses into `old_in_use` — absolute occupancy,
+/// not growth — and [`gc_old_reclaim_growth_band_bytes`]'s proportional half
+/// (`baseline / OLD_RECLAIM_GROWTH_DIVISOR`) collapses with it, leaving the
+/// constant floor. **A constant band pacing a collector whose per-cycle cost is
+/// O(live)** is the quadratic shape #7592 removed here and #7594 removed one
+/// generation down. Measured on `retain` (#7965): 0 fulls → 1–2 fulls,
+/// 2 841 M → 8 237 M instructions retired, +25% peak RSS, and the same on
+/// `retain1` / `retain_wide` / `retain_wide1` / `deeplist`.
+///
+/// The uncertain cohort #7902 is right to worry about — assumed-live bytes
+/// parked in old-gen by a predictor that has since been contradicted — is
+/// bounded by the three instruments #7902 itself added, none of which paces on
+/// this quantity: `untraced_promotion_budget_bytes` forces a measuring cycle,
+/// `implied_dead_bytes` charges that run against `PROMOTED_DEAD_BUDGET_BYTES`,
+/// and [`request_old_reclaim_for_untraced_promotions`] schedules the reclaim
+/// outright when the measurement contradicts the predictor. Those act on
+/// evidence about the cohort; a pinned pacing base acts on every program that
+/// retains, whether or not anything about it is uncertain.
 pub(super) fn credit_promoted_bytes_to_old_baseline(promoted_bytes: usize) {
     if promoted_bytes == 0 {
         return;

crates/perry-runtime/src/gc/tests/promote_in_place.rs

@@ -502,6 +502,112 @@ fn an_untraced_promotion_indexes_the_objects_it_could_not_prove_live() {
     }
 }
 
+/// #7965: an UNTRACED promotion must credit the old-reclaim baseline.
+///
+/// The baseline is the base of a GROWTH measurement, not a liveness claim, so
+/// "an untraced cycle proved nothing" is not a reason to withhold it — see
+/// `credit_promoted_bytes_to_old_baseline`. #7902 withheld it and pinned the
+/// baseline at 0 on every fully-live workload, which cost `retain` two full
+/// mark-sweeps and 2 841 M → 8 237 M instructions retired.
+///
+/// Two halves, because either alone is a presence check:
+///
+/// 1. the real collector, driven through the same entry point as production,
+///    must move the baseline by exactly the bytes it moved into old-gen — and
+///    the counters must show the UNTRACED path is what ran;
+/// 2. the CONSEQUENCE, replayed on `retain`'s measured promotion schedule: the
+///    credited baseline keeps `old_reclaim_pressure_due` false at every step,
+///    and the same schedule against an uncredited baseline fires. Without that
+///    second half a green run would not distinguish "the credit works" from
+///    "this schedule never approached a trigger".
+#[test]
+fn an_untraced_promotion_credits_the_old_reclaim_baseline() {
+    {
+        let _guard = CopyingNurseryTestGuard::new(4);
+        let _trigger_guard = GcTriggerThresholdTestGuard::suppress_automatic_triggers();
+        let _promote = InPlacePromotionTestGuard::untraced();
+
+        let child = young_leaf();
+        js_shadow_slot_set(0, ptr_bits(child));
+
+        let before = GC_LAST_OLD_RECLAIM_IN_USE_BYTES.with(|b| b.get());
+        let trace = collect_minor_trace(GcTriggerKind::Direct);
+        assert_copied_minor_trace(&trace, true, CopiedMinorFallbackReason::None, false);
+
+        // Live subject: the UNTRACED promotion path ran and moved something.
+        // A cycle that evacuated, or promoted with a trace, would exercise the
+        // arm this test is not about.
+        assert!(
+            trace.copying_nursery.in_place_promotion
+                && untraced_promotion_cycles() > 0
+                && trace.copying_nursery.promoted_bytes > 0,
+            "the untraced promotion must have run and promoted something \
+             (in_place={}, untraced_cycles={}, promoted_bytes={}); declined because: {}",
+            trace.copying_nursery.in_place_promotion,
+            untraced_promotion_cycles(),
+            trace.copying_nursery.promoted_bytes,
+            crate::gc::copying::last_untraced_decline_reason()
+        );
+
+        let after = GC_LAST_OLD_RECLAIM_IN_USE_BYTES.with(|b| b.get());
+        assert_eq!(
+            after.saturating_sub(before),
+            trace.copying_nursery.promoted_bytes,
+            "#7965: the baseline must advance by exactly the bytes this cycle \
+             relocated into old-gen, whether or not it traced them"
+        );
+    }
+
+    let _iso = GcTestIsolationGuard::new();
+
+    // The mechanism as arithmetic, independent of which arm happens to fire
+    // and of the `GC_MAJOR_PACING_RETAINING` latch: a baseline pinned at zero
+    // collapses the band's proportional half, so an adaptive band degenerates
+    // into the constant floor however large the live old generation grows.
+    let floor_band = gc_old_reclaim_growth_band_bytes(0);
+    assert!(
+        gc_old_reclaim_growth_band_bytes(512 * 1024 * 1024) > floor_band,
+        "the proportional half is what a zero baseline collapses; without it \
+         this test is not about the same quantity"
+    );
+
+    // `retain`'s measured promotion steps (#7965 trace, `PERRY_GC_DIAG=1`).
+    // Cycle 0 evacuates and every following cycle promotes the whole young
+    // generation untraced, so on a workload like this NOTHING else credits the
+    // baseline — which is why withholding the credit pins it at zero forever.
+    const RETAIN_UNTRACED_PROMOTION_BYTES: [usize; 4] =
+        [18_742_816, 26_213_656, 35_650_552, 37_747_640];
+
+    let previous = GC_LAST_OLD_RECLAIM_IN_USE_BYTES.with(|b| b.get());
+    GC_LAST_OLD_RECLAIM_IN_USE_BYTES.with(|b| b.set(0));
+    let mut old_in_use = 0usize;
+    let mut uncredited_fired = false;
+    for step in RETAIN_UNTRACED_PROMOTION_BYTES.iter().cycle().take(16) {
+        old_in_use += step;
+        credit_promoted_bytes_to_old_baseline(*step);
+        let baseline = GC_LAST_OLD_RECLAIM_IN_USE_BYTES.with(|b| b.get());
+        assert!(
+            !old_reclaim_pressure_due(old_in_use, baseline),
+            "a full scheduled purely because promotion moved bytes into old-gen \
+             is guaranteed to free nothing (old_in_use={old_in_use}, baseline={baseline})"
+        );
+        // The same occupancy read against a baseline nobody credits — the
+        // #7902 state.
+        uncredited_fired |= old_reclaim_pressure_due(old_in_use, 0);
+    }
+    GC_LAST_OLD_RECLAIM_IN_USE_BYTES.with(|b| b.set(previous));
+
+    // The subject was genuinely at risk: a heap made ENTIRELY of objects a
+    // minor just relocated there does schedule a full once the baseline stops
+    // tracking it. Without this the loop above would not distinguish "the
+    // credit works" from "this schedule never approached a trigger".
+    assert!(
+        uncredited_fired,
+        "#7965: an uncredited baseline must make this schedule due — if it does \
+         not, the assertions above prove nothing about the credit"
+    );
+}
+
 #[test]
 fn a_low_survival_cycle_still_evacuates_and_moves_the_object() {
     // NOTE: `CopyingNurseryTestGuard::new` takes the copying-nursery isolation