Skip to content

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

Merged
proggeramlug merged 2 commits into
mainfrom
gc/7965-credit-promotion-baseline
Aug 12, 2026
Merged

fix(gc): credit every promotion to the old-reclaim baseline, untraced or not (#7965)#7968
proggeramlug merged 2 commits into
mainfrom
gc/7965-credit-promotion-baseline

Conversation

@proggeramlug

@proggeramlug proggeramlug commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Closes #7965.

Bisect result — one commit, one line

A sibling agent bisected 8260a9e50..d78efca41 on the full count and I
confirmed the endpoint independently:

step verdict
0809ada92 → e886b56dd (#7900 + #7901) flat, ≤0.6% — both exonerated
e886b56dd → 1bd5eeb6b (#7902) retain +379%, deeplist +253%, retain1 +188%, retain_wide +190%, RSS +34–39%

cdb67021e (#7900 in, #7901/#7902 out) reproduces the pre-regression census
exactly — retain 5 cycles / 0 fulls / 2 841 M instr / 250.1 MB, matching
8260a9e50's recorded figures byte for byte.

★ The mechanism in the issue is refuted

#7965 proposed that old_reclaim_pressure_due's absolute arm is decided by
promotion step size. That is a real property of that arm — it is why #7960
exempted it — but it is not what happened here, and the disproof is clean:
deeplist's untraced-promotion trace is byte-identical across the boundary.
Step size did not change.

The actual cause

// #7902
if !untraced { credit_promoted_bytes_to_old_baseline(collector.stats.promoted_bytes); }

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. 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 / 2)
    collapses with it, leaving the constant floor.

So this is the fifth instance of CLAUDE.md's rule, reached from underneath: not
"paced on a quantity the collection does not move" but its corollary — a
constant band pacing a collector whose per-cycle cost is O(live)
, the
quadratic shape #7592 removed here and #7594 removed one generation down. The
pin is what re-created it.

#7902's other three changes are kept. They close a real defect and none of
them 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
that admitted the run. 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.

Census — main @ 54412cdc1 vs this branch

19/19 byte-exact against the expected output + exit 0 in both arms.
Cycle counts, full counts, instructions retired and peak RSS are all
load-independent; wall time is deliberately not quoted (dev box under load).

program cycles full instr M RSS MB
retain 5 → 4 1 → 0 8 237 → 2 176 (−73.6%) 311.3 → 249.3
retain1 2 → 2 0 → 0 963 → 956 84.1 → 84.1
retain_wide 7 → 6 1 → 0 5 741 → 2 859 (−50.2%) 409.7 → 402.7
retain_wide1 3 → 3 0 → 0 908 → 911 132.5 → 132.5
deeplist 2 → 2 0 → 0 961 → 961 72.9 → 72.8
shapes / asyncpipe / churn / churn_alloc / churn_read / push_cls / push_num / cycles / tree / tree_wide / interp / iso_miss / pipeline / fib40 unchanged 0 → 0 ±0.15% ±0.3%

No program gains a cycle or a full. Against the pre-regression base the fix
arm lands below it — retain 2 841 → 2 176 M (−23.4%), retain1 1 396 → 956
(−31.5%), retain_wide 3 305 → 2 859 (−13.5%), retain_wide1 1 204 → 911
(−24.3%), deeplist 1 165 → 961 (−17.5%) — which is exactly #7960's honest
−14%…−31%. That arithmetic is the check that the regression is gone rather
than masked by something else.

Correctness

  • 19/19 byte-exact + exit 0 under PERRY_GC_VERIFY_EVACUATION=1.
  • 14/14 under PERRY_GC_PROTECT_FROMSPACE=1 PERRY_GC_PROTECT_FROMSPACE_DEPTH=800,
    with the instrument confirmed to have a live subject where it still has one
    (churn 88 retired page-sets, cycles 22). retain retires no from-space at
    all since perf(gc): let the first copying minor decide its promotion from its own trace (#7937) #7960, so that arm is vacuous there by construction.
  • cargo test -p perry-runtime --lib (RUST_TEST_THREADS=1): 2228 passed, 0
    failed
    . cargo fmt --all -- --check, check_file_size.sh,
    gc_runtime_root_holders.py green. addr_class_inventory.py reports one
    PRE-EXISTING hit in object/tests.rs:646, not on this branch.

The gate — on the full count's cause, not on wall time

an_untraced_promotion_credits_the_old_reclaim_baseline. Two halves, because
either alone is a presence check:

  1. it drives a real untraced-promoting copying minor through the production
    entry point and asserts the baseline advanced by exactly
    trace.copying_nursery.promoted_bytes, with in_place_promotion and
    untraced_promotion_cycles() asserted non-zero so an evacuating or a traced
    cycle cannot satisfy it;
  2. it replays retain's measured promotion series and asserts
    old_reclaim_pressure_due stays false at every step and that the same
    series against an uncredited baseline does fire
    .

Half 2's second assertion is the discriminating one, and it earned its place:
it failed on its own first draft, because a 4-step replay does not cross the
band while the RETAINING latch is armed. It is now latch-independent, and it
also asserts that the band's proportional half is what a zero baseline
collapses — so a green run says the credit is load-bearing, not that nothing was
tried.

#7961 / #7929 re-measured on top of this fix

#7961 ("denominate the nursery constant band in objects") merged on top of
this regression, so it was measured against a baseline carrying the spurious
fulls. Re-measured with this fix in both arms (nursery_cap_object_scale_permille
forced to 1000 in the off arm), same corpus, 19/19 byte-exact + exit 0 in both:

program cycles w/ #7929 cycles w/o instr M w/ instr M w/o
retain / retain1 / retain_wide / retain_wide1 4 / 2 / 6 / 3 same 2 176 / 956 / 2 859 / 911 2 178 / 958 / 2 860 / 911
deeplist 2 2 961 968
cycles 22 12 1 836 1 808
interp 44 31 11 708 11 619
iso_miss 61 43 14 322 14 203
churn / churn_alloc / push_cls 88 86 +0.1%
pipeline 7 6 2 599 2 598

Two findings, and both are narrower than the warning that prompted the check:

  1. On its stated target — the retain cluster, where the 72 B → 56 B
    representation shrink was measured — the denomination is worth ±0.2% and
    zero cycle-count change
    once perf(gc): main regressed the retain cluster 2.2-4.8x — retain now runs 2 full collections where it ran none (suspect #7901/#7902) #7965 is fixed. fix(gc): denominate the nursery constant band in objects (#7929) #7961's reported retain
    −63.1% / retain1 −10.9% were the spurious fulls moving, not the band.
    It is not the large regression it was predicted to become either.
  2. Where it does respond is the churn-shaped cluster, and there it costs:
    cycles 12 → 22 collections (+83%), iso_miss 43 → 61 (+42%), interp
    31 → 44 (+42%). Those are cheap minors over a dying nursery so instructions
    move only +0.8…+1.5%, but the collection count is a load-independent 40–80%
    increase.

Deliberately not folded into this PR: #7965's fix does not depend on it, the
two are separable at the census level (every retain row is identical across the
#7929 arm), and re-tuning or reverting a merged band belongs to #7929's owner
with this table in hand. Posted to #7929/#7961.

Full working: gc-handoff/REGRESS-NOTES.md.

Summary by CodeRabbit

  • Bug Fixes

    • Improved garbage collection accounting so all promoted memory contributes to old-generation reclaim tracking.
    • Reduced unnecessary reclaim pressure caused by inaccurate promotion baselines.
    • Added regression coverage to verify baseline updates and reclaim behavior for untraced promotions.
  • Documentation

    • Expanded technical documentation explaining promotion accounting and its impact on memory-reclamation pacing.

… or not (#7965)

#7902 made a copying minor skip `credit_promoted_bytes_to_old_baseline` for an
untraced whole-block promotion, reasoning that "live by construction" is a
marked-liveness claim `PromotionLiveness::AssumeAllLive` does not make. The
premise is right; the conclusion does not follow, because the 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 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. Then `old_in_use - baseline` collapses into absolute
occupancy and `gc_old_reclaim_growth_band_bytes`'s proportional half
(`baseline / 2`) collapses with it, leaving the constant floor — a constant band
pacing a collector whose per-cycle cost is O(live), the quadratic shape #7592
removed here and #7594 removed one generation down.

Measured on the gc-handoff corpus, 19/19 byte-exact + exit 0 in both arms:
`retain` 1 full -> 0 and 8 237 M -> 2 176 M instructions retired, `retain_wide`
1 -> 0 and 5 741 M -> 2 859 M, peak RSS -62 MB and -7 MB. No program gains a
cycle or a full; the other 14 move by at most 0.15%.

#7902's other three changes are kept: they close a real defect and none of them
paces on this quantity.
@coderabbitai

coderabbitai Bot commented Aug 12, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: b7e6735a-2813-488b-8053-08bb59d77965

📥 Commits

Reviewing files that changed from the base of the PR and between 99418e7 and 390ede0.

📒 Files selected for processing (4)
  • changelog.d/7968-old-reclaim-baseline-credit.md
  • crates/perry-runtime/src/gc/copying.rs
  • crates/perry-runtime/src/gc/policy.rs
  • crates/perry-runtime/src/gc/tests/promote_in_place.rs

📝 Walkthrough

Walkthrough

Changes

GC baseline credit

Layer / File(s) Summary
Baseline accounting contract and implementation
crates/perry-runtime/src/gc/copying.rs, crates/perry-runtime/src/gc/policy.rs
All promoted bytes now advance the old-reclaim baseline, including untraced promotions. Policy comments document the accounting model and related safeguards.
Regression validation and release record
crates/perry-runtime/src/gc/tests/promote_in_place.rs, changelog.d/7968-old-reclaim-baseline-credit.md
The test verifies exact baseline advancement and reclaim behavior. The changelog records the fix, cause, benchmarks, and validation.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Suggested reviewers: jdalton, thehypnoo

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch gc/7965-credit-promotion-baseline

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

perf(gc): main regressed the retain cluster 2.2-4.8x — retain now runs 2 full collections where it ran none (suspect #7901/#7902)

1 participant