-
-
Notifications
You must be signed in to change notification settings - Fork 163
perf(gc): one proportional old-reclaim rule, and price the fulls it schedules (#10928) #10960
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
617fa93
1c85d9c
ae60a96
42f24fb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| Pace old-gen reclamation on one proportional rule, and price the full | ||
| collections it schedules. | ||
|
|
||
| The trigger had four pacing inputs and only one could fire. Measured per | ||
| trigger on a TypeScript transpile, the absolute first-crossing arm was true in | ||
| 96% of observations, the proportional arm alone in ~1%, and both together in | ||
| none. The absolute arm is not a property of the heap: every reclaim resets the | ||
| baseline below the 48 MiB threshold, so the "first crossing" re-arms and fires | ||
| again on the next ~18 MiB of growth — 723 full collections in one transpile. | ||
| The other three inputs could not bind: the proportional band sat on its 32 MiB | ||
| floor in every observation because `baseline/2` needs a 64 MiB baseline; the | ||
| productivity backoff returned early on every old-reclaim full because the | ||
| predicate never recorded a pre-collection reading; and the retaining latch | ||
| needs 900‰ young survival against a measured 893‰ ceiling. | ||
|
|
||
| Delete the absolute arm, leaving one proportional rule. Record the pre-full | ||
| live reading inside the predicate so the productivity backoff finally sees | ||
| old-reclaim fulls, following the structural fix `arena_growth_full_escalation_due` | ||
| already carries. Give that backoff its own cell and a tighter cap than the | ||
| arena-growth one, because each doubling of this band is peak RSS. | ||
|
|
||
| On the transpile: instructions fall 57.0% (mean of five interleaved rounds, | ||
| non-overlapping ranges), full collections 723 → 236, and peak RSS falls 22.0%. | ||
| The memory result is not a pacing effect — 723 non-moving in-place sweeps | ||
| generate fragmentation faster than they reclaim it, so fewer collections also | ||
| consolidate the heap (old-gen holes 238 → 200 MB, heap-to-live 5.07× → 3.21×). | ||
|
|
||
| Validated on that workload plus a retaining-heap control, where the change is | ||
| bit-identical by construction because the deleted arm was already exempted | ||
| while retaining. The campaign's light fixtures run below the 48 MiB threshold | ||
| and never reach this trigger at all. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1305,6 +1305,20 @@ crate::perry_thread_local! { | |
| /// `OldReclaim` escalation — old-gen garbage still forces a full through | ||
| /// `old_reclaim_pressure_due` regardless of this backoff. | ||
| pub(super) static GC_MAJOR_PACING_BACKOFF_SHIFT: Cell<u32> = const { Cell::new(0) }; | ||
| /// #10928: pre-full LIVE reading for the old-reclaim full that is about to | ||
| /// run, recorded by `old_reclaim_pressure_due` itself so a future call site | ||
| /// cannot forget to. Zero means "no start reading was taken", exactly as | ||
| /// `GC_FULL_CYCLE_PRE_IN_USE_BYTES` uses it. | ||
| pub(super) static GC_OLD_RECLAIM_PRE_IN_USE_BYTES: Cell<usize> = const { Cell::new(0) }; | ||
| /// #10928: how far left to shift the old-reclaim growth band because the | ||
| /// preceding old-reclaim fulls were unproductive. Reset to 0 by the first | ||
| /// productive one. Capped at `OLD_RECLAIM_BACKOFF_SHIFT_MAX`. | ||
| pub(super) static GC_OLD_RECLAIM_BACKOFF_SHIFT: Cell<u32> = const { Cell::new(0) }; | ||
| /// #10960: live bytes the PREVIOUS priced full left behind, so | ||
| /// `update_old_reclaim_backoff` can tell a futile full (it re-traced the | ||
| /// same live set) from one that found the live set GROWING. Zero before | ||
| /// the first full, which reads as "everything this full found is new". | ||
| pub(super) static GC_OLD_RECLAIM_LAST_POST_IN_USE_BYTES: Cell<usize> = const { Cell::new(0) }; | ||
| /// Survival-adaptive arm of major-GC pacing: `true` once a copying minor | ||
| /// has measured a young-survival ratio at or above | ||
| /// `MAJOR_PACING_RETAINING_SURVIVAL_PERMILLE`, cleared by any minor that | ||
|
|
@@ -1515,6 +1529,17 @@ impl Drop for OldReclaimReentryGuard { | |
| } | ||
| } | ||
|
|
||
| /// #10928: DIAGNOSTIC ONLY - this no longer gates any collection. | ||
| /// | ||
| /// It was the #7937 absolute first-crossing threshold. That arm is deleted | ||
| /// (see `old_reclaim_pressure_due`), so the sole live old-reclaim input is | ||
| /// `gc_old_reclaim_growth_band_bytes`. What remains here is the value the | ||
| /// `[gc-trigger]` line prints as `old_threshold=`, kept so the parsers three | ||
| /// lanes point at this trace keep working; `old_threshold_governs=0` is | ||
| /// printed next to it. Delete both after the campaign. | ||
| /// | ||
| /// Do not reach for this constant in new code: an orphaned tunable that | ||
| /// still prints is worse than an inert arm, because it looks live. | ||
| pub(super) const GC_OLD_GEN_RECLAIM_THRESHOLD_BYTES: usize = 48 * 1024 * 1024; | ||
| pub(super) const GC_OLD_GEN_RECLAIM_GROWTH_BYTES: usize = 32 * 1024 * 1024; | ||
| pub(super) const GC_COPY_PROMOTION_HANDOFF_MIN_BYTES: usize = 24 * 1024 * 1024; | ||
|
|
@@ -1873,42 +1898,167 @@ pub(super) fn gc_old_reclaim_growth_band_bytes(baseline: usize) -> usize { | |
| // full that reclaimed 7.6% — the same futile-full shape one trigger over, | ||
| // reached by the same route. While the young generation is not dying, old | ||
| // growth is priced as live here too. | ||
| if GC_MAJOR_PACING_RETAINING.with(|c| c.get()) { | ||
| return band.saturating_mul(MAJOR_PACING_RETAINING_GROWTH_MULTIPLIER); | ||
| } | ||
| band | ||
| let band = if GC_MAJOR_PACING_RETAINING.with(|c| c.get()) { | ||
| band.saturating_mul(MAJOR_PACING_RETAINING_GROWTH_MULTIPLIER) | ||
| } else { | ||
| band | ||
| }; | ||
| // #10928: the adaptive input. `update_old_reclaim_backoff` shifts this left | ||
| // once per old-reclaim full that freed less than | ||
| // `MAJOR_PACING_PRODUCTIVE_YIELD_PCT` of the live set it traced, and resets | ||
| // it to zero on the first productive one. | ||
| // | ||
| // The signal already existed (`GC_MAJOR_PACING_BACKOFF_SHIFT`) and could | ||
| // never see an old-reclaim full: `note_full_cycle_started` has exactly one | ||
| // production caller, inside `arena_growth_full_escalation_due`, so | ||
| // `update_major_pacing_backoff` returned at `pre_in_use == 0` on every one | ||
| // of the 412 fulls a tsc transpile runs. Recording the pre-full reading | ||
| // inside the PREDICATE is the same structural fix | ||
| // `arena_growth_full_escalation_due` already carries, for the same reason | ||
| // its comment gives: #7726 wired call sites by hand and missed the one the | ||
| // shipped path actually takes. | ||
| // | ||
| // Quiescent on tsc by measurement - its fulls free a median 21.5 MB against | ||
| // a 65.1 MB arena, comfortably past the 20% yield test - so the shift stays | ||
| // 0 there and the band is unchanged. It engages on the futile-full shape | ||
| // the yield test was written for. | ||
| let shift = GC_OLD_RECLAIM_BACKOFF_SHIFT.with(Cell::get); | ||
| band.checked_shl(shift).unwrap_or(usize::MAX) | ||
| } | ||
|
|
||
| /// #10928: the single live old-reclaim pacing rule. | ||
| /// | ||
| /// The #7937 absolute first-crossing arm used to sit here and was DELETED. It | ||
| /// fired on `old_in_use >= 48 MiB && baseline < 48 MiB`, which is not a | ||
| /// property of the heap: every reclaim resets the baseline BELOW the | ||
| /// threshold, so the "first crossing" re-arms and fires again on the next | ||
| /// ~18 MiB of growth. Evaluating both arms per trigger on `tscwork.ts` x3 | ||
| /// measured the absolute arm true in 422/438 observations (96.3%) on | ||
| /// v0.5.1633 and 428/444 (96.4%) on PR #10931s head, the proportional arm | ||
| /// alone in 5 and 3, and BOTH TOGETHER IN ZERO. One arm did all the work. | ||
| /// | ||
| /// The other could not bind. `gc_old_reclaim_growth_band_bytes` is | ||
| /// `max(32 MiB floor, baseline/2)`, and `baseline/2` passes the floor only | ||
| /// above a 64 MiB baseline; tsc peaks at 57.6 MiB before #10931 and 49.3 MiB | ||
| /// after it, so the band read exactly 32 MiB in 438/438 and 444/444 | ||
| /// observations. Shrinking the live set moved the heap FURTHER from the | ||
| /// proportional regime, so the defect deepens as the object-model work | ||
| /// succeeds - which is why this is a rewrite and not a retuned constant. | ||
| /// | ||
| /// What is left is one proportional rule with one adaptive input: the | ||
| /// productivity of the preceding old-reclaim full. | ||
| #[inline] | ||
| pub(super) fn old_reclaim_pressure_due(old_in_use: usize, baseline: usize) -> bool { | ||
| let threshold = gc_old_gen_reclaim_threshold_dyn_bytes(); | ||
| // #7937: the absolute first-crossing arm is exempted while the heap is | ||
| // measurably RETAINING, for the reason #7592 already exempted the | ||
| // proportional arm two functions down — and it is the arm that actually | ||
| // fires. | ||
| // | ||
| // `baseline` is credited by every promotion | ||
| // (`credit_promoted_bytes_to_old_baseline`), so `old_in_use >= T && | ||
| // baseline < T` is a race between two quantities that move in the same | ||
| // direction at different granularities. Whether it fires therefore depends | ||
| // on the SIZE OF THE PROMOTION STEPS, not on any property of the heap. | ||
| // Measured on `retain.ts` (#7937, `gc-handoff/CYCLE0-NOTES.md`): same | ||
| // program, same live set, same total promotion — changing the schedule from | ||
| // (18.7 MB, 34.6 MB) to (17.7 MB, 17.8 MB) makes it fire twice and buys two | ||
| // full mark-sweeps costing 588 ms against a 55 ms GC budget, at | ||
| // `old_in_use=52.3 MB, baseline=35.5 MB, T=48 MB` with the proportional arm | ||
| // correctly declining (`band=128 MB`) and `retaining=true`. | ||
| // | ||
| // That is the futile-full shape #7592 removed one trigger over: a heap | ||
| // whose young generation is not dying is retaining live data, and a full | ||
| // mark-sweep cannot lower the number being watched. The proportional arm | ||
| // still bounds the exposure, so this defers reclamation, it does not remove | ||
| // it — the same trade the RETAINING multiplier already makes. | ||
| let crossed_absolute_threshold = old_in_use >= threshold | ||
| && baseline < threshold | ||
| && !GC_MAJOR_PACING_RETAINING.with(|c| c.get()); | ||
| crossed_absolute_threshold | ||
| || old_in_use.saturating_sub(baseline) >= gc_old_reclaim_growth_band_bytes(baseline) | ||
| let due = old_reclaim_pressure_due_inner(old_in_use, baseline); | ||
| if due { | ||
| // Recorded HERE, not at the call sites, so a future arm that decides | ||
| // to run a reclaim cannot forget to price it - the rule | ||
| // `arena_growth_full_escalation_due` already follows, and the one | ||
| // #7726 learned by wiring two sites and missing the third. | ||
| note_old_reclaim_cycle_started(); | ||
| } | ||
| due | ||
| } | ||
|
Comment on lines
1951
to
+1961
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win 🔎 Supported by static analysis🏁 Script executed: #!/bin/bash
rg -nP -C8 '\bold_reclaim_pressure_due\s*\(' crates/perry-runtime/src/gc
rg -nP -C4 '\bfinish_full_old_reclaim_baseline\s*\(' crates/perry-runtime/src/gc
rg -nP -C6 'YoungScavengeCap|DueTriggerMemo' crates/perry-runtime/src/gc/policy.rs | head -120Repository: PerryTS/perry Length of output: 28724 🏁 Script executed: #!/bin/bash
set -e
printf '%s\n' '--- policy cleanup and trigger evaluator ---'
sed -n '2310,2365p' crates/perry-runtime/src/gc/policy.rs
sed -n '3550,3680p' crates/perry-runtime/src/gc/policy.rs
printf '%s\n' '--- full-cycle completion and trigger call sites ---'
rg -n -P -C8 'finish_full_old_reclaim_baseline|gc_budgeted_due_trigger_eval|gc_budgeted_due_trigger\(|GC_ROOT_LOCK_DEPTH|should_defer|GcTriggerKind::OldReclaim|BudgetedGcTrigger::OldReclaim' crates/perry-runtime/src/gcRepository: PerryTS/perry Length of output: 42677 🏁 Script executed: #!/bin/bash
set -e
printf '%s\n' '--- old-reclaim backoff implementation ---'
rg -n -P -C18 'fn update_old_reclaim_backoff|fn note_old_reclaim_cycle_started|GC_OLD_RECLAIM_PRE_IN_USE_BYTES|GC_OLD_RECLAIM_LAST_POST_IN_USE_BYTES' crates/perry-runtime/src/gc/policy.rs
printf '%s\n' '--- all full completion/start bindings ---'
rg -n -P -C8 'finish_full_old_reclaim_baseline\(|gc_start_budgeted_full_cycle\(|gc_collect_full_mark_sweep_with_trigger|GcTriggerKind::OldGenBytes' crates/perry-runtime/src/gc
printf '%s\n' '--- trigger decision consumers ---'
sed -n '4240,4320p' crates/perry-runtime/src/gc/policy.rs
sed -n '4550,4635p' crates/perry-runtime/src/gc/policy.rsRepository: PerryTS/perry Length of output: 41782 Bind the old-reclaim pre-reading to its actual full.
Associate the snapshot with the actual 🤖 Prompt for AI Agents |
||
|
|
||
| /// The pure predicate: no pacing side effect, so the debt arithmetic and the | ||
| /// unit tests can ask the question without answering it. | ||
| #[inline] | ||
| pub(super) fn old_reclaim_pressure_due_inner(old_in_use: usize, baseline: usize) -> bool { | ||
| old_in_use.saturating_sub(baseline) >= gc_old_reclaim_growth_band_bytes(baseline) | ||
| } | ||
|
|
||
| /// #10928: snapshot the live set this old-reclaim full is about to trace, for | ||
| /// `update_old_reclaim_backoff` to price the result against. Same metric on | ||
| /// both sides (`pacing_arena_in_use_bytes`) so the two cannot disagree about | ||
| /// whether the full helped. | ||
| fn note_old_reclaim_cycle_started() { | ||
| GC_OLD_RECLAIM_PRE_IN_USE_BYTES.with(|bytes| bytes.set(pacing_arena_in_use_bytes())); | ||
| } | ||
|
|
||
| /// Cap on the old-reclaim backoff, deliberately tighter than | ||
| /// `MAJOR_PACING_BACKOFF_SHIFT_MAX`. Each shift doubles how much old-gen may | ||
| /// grow between fulls, and that growth is peak RSS: measured on tsc, the band | ||
| /// governs a ~47 MB live set inside a ~300 MB peak, so one doubling of the | ||
| /// 32 MiB band is already most of the campaigns +10% peak-RSS budget. Raise | ||
| /// this only with an RSS measurement that says the budget holds. | ||
| const OLD_RECLAIM_BACKOFF_SHIFT_MAX: u32 = 1; | ||
|
|
||
| /// Price what the just-finished old-reclaim full actually reclaimed. | ||
| /// | ||
| /// Mirrors `update_major_pacing_backoff` on what "productive" means, against | ||
| /// its own cell: the two triggers pace different things and must not share a | ||
| /// homeostat. | ||
| /// | ||
| /// #10960: an unproductive full widens the band ONLY when the live set did | ||
| /// not grow since the previous full. The backoff exists for the futile-full | ||
| /// shape - repeated traces of the same live set that free nothing - and a | ||
| /// full that reclaimed little because the program was still BUILDING its live | ||
| /// set is not that shape: the proportional band already follows the baseline | ||
| /// up, so widening on top of it double-counts the growth. Measured on | ||
| /// `gc_reclaim.ts 64 6` (64 MiB live, replaced continuously): the first full | ||
| /// runs at 33.5 MB while the heap is still ramping and frees nothing, the old | ||
| /// rule doubled the band to 64 MiB, and the next full slid out to | ||
| /// `old_in_use=135 MB` with `arena_total=281 MB` - a +29% peak-RSS excursion | ||
| /// (median 150 -> 193 MB, 15/15 interleaved pairs) that steady state never | ||
| /// needed. A growing live set leaves the shift where it is: it is evidence | ||
| /// neither that fulls are futile nor that they have become productive. | ||
| fn update_old_reclaim_backoff(post_in_use: usize) { | ||
| let prev_post = GC_OLD_RECLAIM_LAST_POST_IN_USE_BYTES.with(|bytes| bytes.replace(post_in_use)); | ||
| let pre_in_use = GC_OLD_RECLAIM_PRE_IN_USE_BYTES.with(|bytes| bytes.get()); | ||
| if pre_in_use == 0 { | ||
| return; | ||
| } | ||
| GC_OLD_RECLAIM_PRE_IN_USE_BYTES.with(|bytes| bytes.set(0)); | ||
| let reclaimed = pre_in_use.saturating_sub(post_in_use); | ||
| let productive = | ||
| reclaimed.saturating_mul(100) / pre_in_use >= MAJOR_PACING_PRODUCTIVE_YIELD_PCT; | ||
| // Same yardstick as `productive`: at least that share of what this full | ||
| // traced is live data the previous full never saw. | ||
| let grew = post_in_use.saturating_sub(prev_post).saturating_mul(100) / pre_in_use | ||
| >= MAJOR_PACING_PRODUCTIVE_YIELD_PCT; | ||
| GC_OLD_RECLAIM_BACKOFF_SHIFT.with(|shift| { | ||
| if productive { | ||
| shift.set(0); | ||
| } else if !grew { | ||
| shift.set( | ||
| shift | ||
| .get() | ||
| .saturating_add(1) | ||
| .min(OLD_RECLAIM_BACKOFF_SHIFT_MAX), | ||
| ); | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| #[cfg(test)] | ||
| pub(super) fn test_set_old_reclaim_last_post_in_use(bytes: usize) -> usize { | ||
| GC_OLD_RECLAIM_LAST_POST_IN_USE_BYTES.with(|c| c.replace(bytes)) | ||
| } | ||
|
|
||
| #[cfg(test)] | ||
| pub(super) fn test_set_old_reclaim_backoff_shift(shift: u32) -> u32 { | ||
| GC_OLD_RECLAIM_BACKOFF_SHIFT.with(|c| c.replace(shift)) | ||
| } | ||
|
|
||
| #[cfg(test)] | ||
| pub(super) fn test_old_reclaim_backoff_shift() -> u32 { | ||
| GC_OLD_RECLAIM_BACKOFF_SHIFT.with(Cell::get) | ||
| } | ||
|
|
||
| #[cfg(test)] | ||
| pub(super) fn test_old_reclaim_pre_in_use_bytes() -> usize { | ||
| GC_OLD_RECLAIM_PRE_IN_USE_BYTES.with(Cell::get) | ||
| } | ||
|
|
||
| #[cfg(test)] | ||
| pub(super) fn test_clear_old_reclaim_pre_in_use_bytes() { | ||
| GC_OLD_RECLAIM_PRE_IN_USE_BYTES.with(|b| b.set(0)); | ||
| } | ||
|
|
||
| #[cfg(test)] | ||
| pub(super) fn test_price_old_reclaim_full(pre_in_use: usize, post_in_use: usize) { | ||
| GC_OLD_RECLAIM_PRE_IN_USE_BYTES.with(|b| b.set(pre_in_use)); | ||
| update_old_reclaim_backoff(post_in_use); | ||
| } | ||
|
|
||
| /// Whether an imminent promotion justifies a full old reclaim FIRST. | ||
|
|
@@ -2187,6 +2337,8 @@ pub(super) fn finish_full_old_reclaim_baseline() { | |
| let post_in_use = crate::arena::arena_live_allocated_bytes(); | ||
| GC_LAST_FULL_ARENA_IN_USE_BYTES.with(|bytes| bytes.set(post_in_use)); | ||
| update_major_pacing_backoff(post_in_use); | ||
| // #10928: and price THIS full against the old-reclaim band that scheduled it. | ||
| update_old_reclaim_backoff(post_in_use); | ||
| GC_OLD_RECLAIM_PENDING.with(|pending| pending.set(false)); | ||
| // #7742: the dead bytes that whole-block promotion parked in old-gen are | ||
| // exactly what this collection just reclaimed, so the running budget that | ||
|
|
@@ -3353,13 +3505,11 @@ fn gc_budgeted_resume_blocked() -> bool { | |
| } | ||
|
|
||
| pub(super) fn gc_old_reclaim_debt_bytes(old_in_use: usize, baseline: usize) -> u64 { | ||
| let trigger = if baseline < gc_old_gen_reclaim_threshold_dyn_bytes() { | ||
| gc_old_gen_reclaim_threshold_dyn_bytes() | ||
| } else { | ||
| // Same proportional band as `old_reclaim_pressure_due` (#7592) — | ||
| // debt and dueness must share one trigger or they diverge. | ||
| baseline.saturating_add(gc_old_reclaim_growth_band_bytes(baseline)) | ||
| }; | ||
| // #10928: one trigger, shared with `old_reclaim_pressure_due` -- the | ||
| // absolute first-crossing arm it used to mirror no longer exists, and a | ||
| // debt computed against a trigger the predicate does not use is exactly | ||
| // the #7024 two-predicates-collapse the shared helper exists to prevent. | ||
| let trigger = baseline.saturating_add(gc_old_reclaim_growth_band_bytes(baseline)); | ||
| old_in_use.saturating_sub(trigger) as u64 | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
checked_shldoes not saturate. The band can wrap to a small value.usize::checked_shlreturnsNoneonly whenshift >= usize::BITS. It does not detect high bits shifted out of the value. Withshift == 1andband >= 1 << (usize::BITS - 1), the result wraps.On a 32-bit target, a 2 GiB band (for example,
baseline/2 = 512 MiBtimesMAJOR_PACING_RETAINING_GROWTH_MULTIPLIER) becomes0.old_reclaim_pressure_due_inneris then true on every copied minor, so the backoff meant to reduce fulls causes a full storm. The comment at Lines 1901-1904 and the change summary describe this as saturation.Saturate explicitly:
🐛 Proposed fix
📝 Committable suggestion
🤖 Prompt for AI Agents