fix(gc): do not start a budgeted cycle for a nursery-cap trigger it cannot discharge (#7909) - #7972
Merged
Merged
Conversation
…annot discharge (#7909) `gc_budgeted_due_trigger()` reported the young-generation scavenge cap as `ArenaBytes`, so a host safepoint started a budgeted (low-pause NON-MOVING) cycle for it. Such a cycle sweeps in place and cannot lower `copying_from_space_in_use_bytes()` — the exact quantity the cap tests — while `gc_safepoint_moving_minor` rejects every precise safepoint at its `budgeted` entry guard for the cycle's whole life. When the host's step cadence cannot finish the cycle the composition is permanent and silent: cap due -> cycle started -> moving minor locked out -> nothing reclaims -> cap still due, with the SATB mark barrier armed for the rest of the process and an empty `[gc]` trace because the trace is written by the completion path. Split the cap out as `BudgetedGcTrigger::YoungScavengeCap` (every collection site treats it identically to `ArenaBytes`) and decline to START a budgeted cycle for it, deferring to the precise safepoint exactly as the alloc-point arm already does. Counted as `nursery_cap_deferred` in the `PERRY_GC_DIAG=1` `[gc-incremental]` line. The regression test drives the real host-safepoint path with a per-thread cap override and pairs the refusal with a control phase on the same fixture: only the due trigger differs, so the pair discriminates "declines this trigger" from "declines everything".
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughChangesNursery-cap GC handling
Sequence Diagram(s)sequenceDiagram
participant HostSafepoint
participant GcPolicy
participant Instrumentation
participant PreciseSafepoint
HostSafepoint->>GcPolicy: Check nursery-cap pressure
GcPolicy->>Instrumentation: Record nursery-cap deferral
GcPolicy->>PreciseSafepoint: Arm precise collection
PreciseSafepoint->>GcPolicy: Run evacuating minor collection
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
proggeramlug
marked this pull request as ready for review
August 12, 2026 16:05
proggeramlug
pushed a commit
that referenced
this pull request
Aug 12, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #7909.
The defect
gc_budgeted_due_trigger()reported the young-generation scavenge cap asBudgetedGcTrigger::ArenaBytes, so a host safepoint (and a mutator assist)started a budgeted cycle for it. A budgeted cycle is
low_pause_non_movingby construction: it sweeps in place and cannot lower
copying_from_space_in_use_bytes(), which is the exact quantityyoung_scavenge_cap_due()tests. Meanwhilegc_safepoint_moving_minorrejectsevery precise safepoint at its
budgetedentry guard for the cycle's wholelife.
The two compose into a stall that sustains itself and reports nothing:
The SATB mark barrier stays armed for the duration (every heap store, every
shadow-slot root store, allocate-black on every birth) and the
[gc]tracestays empty, because the trace is written by the completion path.
The fix
Split the cap out as
BudgetedGcTrigger::YoungScavengeCap. Every collectionsite treats it exactly as
ArenaBytes— the split exists only so the budgetedstepper can tell them apart at the moment it decides whether to start a
cycle. When the only due trigger is the cap and the cycle would be budgeted, no
cycle is started; the pressure is deferred to the precise safepoint (arena
baseline included, so the
moving_defer_within_slackvalve is not left readinga stale, already-exceeded baseline — the #7024 shape) exactly as
gc_check_trigger's alloc-point arm has always done. That asymmetry betweenthe two paths was the bug.
No new knob.
PERRY_GC_DIAG=1's[gc-incremental]line gainsnursery_cap_deferred=N, so the refusal is distinguishable from "nothing wasdue".
Unreachable by construction in the configuration where it would be wrong:
young_scavenge_cap_due()is false unlessnursery_cap_active(), which isgc_moving_loop_polls_enabled()— so the cap can only be the due trigger inexactly the configuration where the precise route exists.
The test, and why it is a unit test
a_nursery_cap_only_trigger_is_deferred_to_the_collector_that_can_discharge_itdrives the real host-safepoint path with a per-thread cap override (the shape
support.rsmandates — never a process-wide environment write, #7946) and pairsthe refusal with a control phase on the same fixture: same thread, same
heap, only the due trigger differs, and the control must still start a cycle. So
the pair discriminates "declines this trigger" from "declines everything", which
a bare
cycle_starts == 0cannot.★ The issue's named end-to-end reproducer is stale and this is why the test
does not use one.
PERRY_GC_SCAVENGE_NURSERY_MB=4onapps/asyncpipe.tsreproduced the filed signature at
ac52a5c38(0.5.1490). Swept across the wholedial on
a769fafc6, it does not:The stall half is no longer reachable on that program at any cap; the
defect half is fully intact one notch lower, at cap 2 — a budgeted non-moving
cycle started for a trigger it cannot discharge, arming the mark barrier for
87.9 ms of a ~127 ms program. A fixture pinned to one knob value on one
benchmark moved twice in five patch versions, so it is not a durable gate.
Validation
Own build, own worktree (
wt-hdr), ownCARGO_TARGET_DIR, basea769fafc6(0.5.1495). Both arms built with the identical
-pset andPERRY_RUNTIME_DIRpinned; the twolibperry_runtime.aarchives differ, so theA/B is not vacuous.
Correctness. 19/19 corpus programs byte-exact vs
m0810/expected/(node 26.5.1), exit 0 — and again under
PERRY_GC_PROTECT_FROMSPACE=1 PERRY_GC_PROTECT_FROMSPACE_DEPTH=800 PERRY_GC_VERIFY_EVACUATION=1.iso_misscanary:checksum 437840 misses 0.cargo test --release -p perry-runtime(RUST_TEST_THREADS=1):2229 passed, 0 failed, 4 ignored.
Sabotage-proved. With the new branch disabled (
if false && …, the fixcommitted first), the test fails on the intended assertion —
a cap-only host safepoint must report idle, not an active cycle— andgrep -c "Compiling perry-runtime v"on the sabotage log reads 1, so the redverdict is about the sabotaged code and not a stale unit. (The pre-existing
an_active_budgeted_cycle_locks_out_…also goes red under the sabotage: the newtest panics before completing the cycle it opened, and these run single-threaded
in one process. It is green in both non-sabotaged runs.)
Cost on the shipped corpus: zero, for a stated reason. Every GC counter is
bit-identical across all 19 programs — same
minors, samefulls, samecopied_objects, samepromoted_objects— and RSS matches to 0.06 %.Instructions span −0.48 % … +0.63 %, inside the base arm's own best-of-3 spread.
nursery_cap_deferred=0on all 19: at default settings no budgeted cycle isstarted anywhere in the corpus (
cycle_starts=0,no_trigger=6), so there isnothing for the new branch to decline.
In the regime where it IS live it is a large win — which refutes the recorded
blocker.
GC7909-NOTES.md§4.2 measured the equivalent unblocking atc109b089das +51 % instructions / +57 % RSS and concluded "#7909 cannot beclosed before #7915". Re-measured on
a769fafc6atPERRY_GC_SCAVENGE_NURSERY_MB=2(the value that still reaches the defect),ab_instr.pybest of 5, output byte-exact and exit 0 on both arms:cycle_startsstepsmark_barrier_armed_usnursery_cap_deferredcopying_minors−16 % instructions and −13.5 MB RSS. The state removed (88 ms of armed SATB
barrier plus a locked-out collector) is now more expensive than the state created
(six more copying minors). The blocker was a price, and the price has moved.
Full write-up and the harnesses:
gc-handoff/HDR-NOTES.md.Summary by CodeRabbit
Bug Fixes
Diagnostics
nursery_cap_deferredreporting to incremental garbage collection diagnostics.