Skip to content

Commit 882be57

Browse files
author
Ralph Küpper
committed
fix(gc): scope the retaining band to the full-collection decision, not survivor placement
gc-ratchet 11_collect_at_depth turned 6,150 promoted objects into 6,139 copied ones: copied_minor_promotion_handoff_pressure_due shares old_reclaim_pressure_due with the OldReclaim escalation, so widening the shared band also stopped the survivor-promotion handoff from firing on a retaining heap. Placement and collection are different questions and only the second one was paying for a futile full, so the multiplier moves to old_reclaim_full_due and the shared band goes back to what it was. Pinned by a test that asserts both directions from one reading.
1 parent 0e55c74 commit 882be57

2 files changed

Lines changed: 92 additions & 15 deletions

File tree

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

Lines changed: 37 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1351,25 +1351,45 @@ const OLD_RECLAIM_GROWTH_DIVISOR: usize = 2;
13511351
/// the "is it due" predicate and the debt arithmetic cannot diverge (#7024's
13521352
/// two-predicates-collapse family).
13531353
pub(super) fn gc_old_reclaim_growth_band_bytes(baseline: usize) -> usize {
1354-
let band = gc_old_gen_reclaim_growth_dyn_bytes().max(baseline / OLD_RECLAIM_GROWTH_DIVISOR);
1355-
// Survival-adaptive, the same signal and the same multiplier the
1356-
// arena-growth escalation uses (`MAJOR_PACING_RETAINING_GROWTH_MULTIPLIER`).
1357-
//
1358-
// `credit_promoted_bytes_to_old_baseline` already exempts old-gen growth
1359-
// that a minor PROVED live, but a large object is allocated straight into
1360-
// old-gen and never passes through promotion, so its bytes are uncredited
1361-
// growth even when they are the program's live data. On `retain.ts` that is
1362-
// the element array itself: with the arena-growth escalation correctly
1363-
// declining, this band became the binding constraint and fired a 452 ms
1364-
// full that reclaimed 7.6% — the same futile-full shape one trigger over,
1365-
// reached by the same route. While the young generation is not dying, old
1366-
// growth is priced as live here too.
1354+
gc_old_gen_reclaim_growth_dyn_bytes().max(baseline / OLD_RECLAIM_GROWTH_DIVISOR)
1355+
}
1356+
1357+
/// The same band, widened while the heap is RETAINING, for the decisions that
1358+
/// answer **"run a FULL collection now?"**.
1359+
///
1360+
/// `credit_promoted_bytes_to_old_baseline` already exempts old-gen growth that
1361+
/// a minor PROVED live, but a large object is allocated straight into old-gen
1362+
/// and never passes through promotion, so its bytes are uncredited growth even
1363+
/// when they are the program's live data. On `retain.ts` that is the element
1364+
/// array itself: with the arena-growth escalation correctly declining, this
1365+
/// band became the binding constraint and fired a 452 ms full that reclaimed
1366+
/// 7.6% — the same futile-full shape one trigger over, reached by the same
1367+
/// route.
1368+
///
1369+
/// **Deliberately not folded into `gc_old_reclaim_growth_band_bytes`.** That
1370+
/// predicate has a second caller,
1371+
/// `copied_minor_promotion_handoff_pressure_due`, which decides where a
1372+
/// copying minor's survivors LIVE — not whether to collect. Widening it there
1373+
/// too made the handoff stop firing on a retaining heap, and the gc-ratchet's
1374+
/// `11_collect_at_depth` recorded exactly that: 6,150 promoted objects became
1375+
/// 6,139 copied ones. Placement and collection are different questions and only
1376+
/// the second one is paying for a futile full.
1377+
fn old_reclaim_full_growth_band_bytes(baseline: usize) -> usize {
1378+
let band = gc_old_reclaim_growth_band_bytes(baseline);
13671379
if GC_MAJOR_PACING_RETAINING.with(|c| c.get()) {
13681380
return band.saturating_mul(MAJOR_PACING_RETAINING_GROWTH_MULTIPLIER);
13691381
}
13701382
band
13711383
}
13721384

1385+
/// [`old_reclaim_pressure_due`] for the callers that respond by running a full
1386+
/// collection. Same shape, retaining-adaptive band.
1387+
pub(super) fn old_reclaim_full_due(old_in_use: usize, baseline: usize) -> bool {
1388+
(old_in_use >= gc_old_gen_reclaim_threshold_dyn_bytes()
1389+
&& baseline < gc_old_gen_reclaim_threshold_dyn_bytes())
1390+
|| old_in_use.saturating_sub(baseline) >= old_reclaim_full_growth_band_bytes(baseline)
1391+
}
1392+
13731393
#[inline]
13741394
pub(super) fn old_reclaim_pressure_due(old_in_use: usize, baseline: usize) -> bool {
13751395
(old_in_use >= gc_old_gen_reclaim_threshold_dyn_bytes()
@@ -1576,7 +1596,9 @@ pub(super) fn maybe_schedule_old_reclaim_after_copied_minor() {
15761596
let old_in_use =
15771597
old_gen_reclaimable_pressure_bytes().saturating_add(external_side_live_bytes());
15781598
let baseline = GC_LAST_OLD_RECLAIM_IN_USE_BYTES.with(|bytes| bytes.get());
1579-
if old_reclaim_pressure_due(old_in_use, baseline) {
1599+
// `_full_due`: this schedules a FULL collection, so it reads the
1600+
// retaining-adaptive band. The survivor-placement caller does not.
1601+
if old_reclaim_full_due(old_in_use, baseline) {
15801602
GC_OLD_RECLAIM_PENDING.with(|pending| pending.set(true));
15811603
}
15821604
}
@@ -2453,7 +2475,7 @@ fn gc_budgeted_due_trigger() -> Option<BudgetedGcTrigger> {
24532475
let old_in_use =
24542476
old_gen_reclaimable_pressure_bytes().saturating_add(external_side_live_bytes());
24552477
let old_baseline = GC_LAST_OLD_RECLAIM_IN_USE_BYTES.with(|bytes| bytes.get());
2456-
if old_pending || old_reclaim_pressure_due(old_in_use, old_baseline) {
2478+
if old_pending || old_reclaim_full_due(old_in_use, old_baseline) {
24572479
return Some(BudgetedGcTrigger::OldReclaim);
24582480
}
24592481

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

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1339,3 +1339,58 @@ fn retaining_rebaseline_never_lowers_the_pacing_baseline() {
13391339
"a larger post-minor occupancy must raise it"
13401340
);
13411341
}
1342+
1343+
/// The retaining band must widen the FULL-collection decision and leave the
1344+
/// survivor-PLACEMENT decision alone.
1345+
///
1346+
/// These two read the same numbers through predicates that used to be one, and
1347+
/// collapsing them is not hypothetical: widening the shared band made
1348+
/// `copied_minor_promotion_handoff_pressure_due` stop firing on a retaining
1349+
/// heap, and the gc-ratchet's `11_collect_at_depth` turned 6,150 promoted
1350+
/// objects into 6,139 copied ones. So the divergence is pinned rather than
1351+
/// left as a convention.
1352+
#[test]
1353+
fn the_retaining_band_widens_the_full_decision_but_not_survivor_placement() {
1354+
use super::super::policy::{
1355+
gc_old_reclaim_growth_band_bytes, note_copying_minor_young_survival, old_reclaim_full_due,
1356+
old_reclaim_pressure_due, test_reset_major_pacing_backoff, test_set_pacing_arena_in_use,
1357+
};
1358+
1359+
// Pacing's re-baseline reads the arena; pin it so this test only moves the
1360+
// retaining flag.
1361+
let previous_reading = test_set_pacing_arena_in_use(Some(0));
1362+
test_reset_major_pacing_backoff();
1363+
1364+
// A baseline high enough that the proportional band, not the constant
1365+
// floor, decides — and an `old_in_use` inside `(band, 4 × band]`.
1366+
let baseline = 512 * 1024 * 1024;
1367+
let band = gc_old_reclaim_growth_band_bytes(baseline);
1368+
let old_in_use = baseline + band + 1;
1369+
1370+
note_copying_minor_young_survival(0);
1371+
let off_full = old_reclaim_full_due(old_in_use, baseline);
1372+
let off_placement = old_reclaim_pressure_due(old_in_use, baseline);
1373+
1374+
note_copying_minor_young_survival(1000);
1375+
let on_full = old_reclaim_full_due(old_in_use, baseline);
1376+
let on_placement = old_reclaim_pressure_due(old_in_use, baseline);
1377+
1378+
test_set_pacing_arena_in_use(previous_reading);
1379+
test_reset_major_pacing_backoff();
1380+
1381+
assert!(
1382+
off_full && off_placement,
1383+
"without the retaining arm this reading must be due on both, or the \
1384+
test proves nothing"
1385+
);
1386+
assert!(
1387+
!on_full,
1388+
"a retaining heap must not schedule a full for old growth inside the \
1389+
widened band"
1390+
);
1391+
assert!(
1392+
on_placement,
1393+
"survivor placement must be unaffected: it decides where survivors \
1394+
live, not whether to collect"
1395+
);
1396+
}

0 commit comments

Comments
 (0)