perf(gc): live-proportional collection budgets at both generations (#7592) - #7596
Conversation
…7592) Three changes, one principle: no constant band may pace a collector whose per-cycle cost is O(live) -- total work goes quadratic in the live set. 1. The scavenge nursery cap is now also proportional to the TENURED live set (old-gen reclaimable pressure / 2), with the influx-driven product as the floor. Keyed on old-gen occupancy, NOT total arena in-use: the cap gates young_scavenge_cap_due() against from-space occupancy, and a cap defined by a total that includes the young generation is a fixed point from-space can never cross (measured: scavenging stopped entirely, 0 copying minors at 200k records). 2. The old-reclaim growth band is now max(constant, baseline/2) -- Go's GOGC shape, shared by old_reclaim_pressure_due and gc_old_reclaim_debt_bytes so dueness and debt cannot diverge. 3. Two guaranteed-futile fulls eliminated: - The survivor-promotion handoff now fires on CURRENT old-gen pressure only. It used to fire on old + promotable -- but promotable bytes sit in the survivor space, where a full mark-sweep can neither reclaim them nor the old-gen space they have not yet occupied (measured: 1,015 ms over 4.2 MB of old-gen, 0 freed). - A copying minor's promoted bytes are credited to the old-reclaim baseline: they are live by construction, so a reclaim fired because promotion crossed a threshold finds them all live and frees nothing (measured: 2,100 ms over 274 MB just-promoted, 0.0 MB freed). json_pipeline build_out, 500k records: 57.9 s (main) / 10.6 s (#7594 latch alone) -> 5.1 s, 11.3x vs main, and ns/record is flat within 21% across a 5x size range (main: 11x growth). Output hash identical on every row; RSS +1.7% over the latch arm.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe GC now uses proportional old-generation pressure for reclaim pacing, promotion handoff, and nursery sizing. Copying minor collections credit promoted bytes to the old-reclaim baseline. Trigger tests cover thresholds, debt boundaries, and promotion crediting. The package version is updated to ChangesGC reclaim pacing
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/perry-runtime/src/gc/tenuring.rs`:
- Around line 131-135: Add PR-run tests that directly exercise
scavenge_nursery_cap_effective_bytes, covering both outcomes: influx_driven
being the maximum and old_gen_reclaimable_pressure_bytes() /
TENURED_EDEN_DIVISOR being the maximum. Configure the relevant scale and
reclaimable-pressure inputs for each case, then assert the effective cap matches
the winning branch.
- Around line 138-142: Correct the documentation comment above the tenuring
nursery cap divisor to describe `tenured / 2` only as the tenured-proportional
term. Remove the claim that young-generation RSS is bounded by that value, and
avoid asserting logarithmic collection behavior as a guaranteed consequence when
`scavenge_nursery_cap_effective_bytes` may select the larger `influx_driven`
cap.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 2ba981cb-538b-4f4e-9f15-fa3cd0eb2fa6
📒 Files selected for processing (5)
changelog.d/7596-live-proportional-gc-budgets.mdcrates/perry-runtime/src/gc/copying.rscrates/perry-runtime/src/gc/policy.rscrates/perry-runtime/src/gc/tenuring.rscrates/perry-runtime/src/gc/tests/triggers.rs
| pub(super) fn scavenge_nursery_cap_effective_bytes() -> usize { | ||
| gc_scavenge_nursery_cap_bytes().saturating_mul(NURSERY_CAP_SCALE.with(Cell::get) as usize) | ||
| let influx_driven = | ||
| gc_scavenge_nursery_cap_bytes().saturating_mul(NURSERY_CAP_SCALE.with(Cell::get) as usize); | ||
| let tenured_proportional = old_gen_reclaimable_pressure_bytes() / TENURED_EDEN_DIVISOR; | ||
| influx_driven.max(tenured_proportional) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Add direct coverage for both effective-cap branches.
The supplied trigger tests do not call scavenge_nursery_cap_effective_bytes. Add PR-run tests where influx_driven wins and where old_gen_reclaimable_pressure_bytes() / 2 wins. This protects the new aggregation and pressure source.
As per coding guidelines, “Put acceptance coverage in tests that run on PRs when possible.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/perry-runtime/src/gc/tenuring.rs` around lines 131 - 135, Add PR-run
tests that directly exercise scavenge_nursery_cap_effective_bytes, covering both
outcomes: influx_driven being the maximum and
old_gen_reclaimable_pressure_bytes() / TENURED_EDEN_DIVISOR being the maximum.
Configure the relevant scale and reclaimable-pressure inputs for each case, then
assert the effective cap matches the winning branch.
Source: Coding guidelines
| /// #7592: divisor for the tenured-proportional nursery cap — Eden may grow to | ||
| /// half the tenured live set before a scavenge is forced. Peak young-gen RSS | ||
| /// contribution is therefore bounded at `tenured / 2`; the young collection | ||
| /// count is logarithmic in heap growth on promote-heavy workloads | ||
| /// (`old_{n+1} ≈ old_n × (1 + 1/2)`) instead of linear in bytes allocated. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Correct the RSS-bound claim.
scavenge_nursery_cap_effective_bytes returns max(influx_driven, tenured_proportional). If influx_driven is larger, the cap can exceed tenured / 2. Describe tenured / 2 as the proportional term, not as an upper bound on young-generation RSS.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/perry-runtime/src/gc/tenuring.rs` around lines 138 - 142, Correct the
documentation comment above the tenuring nursery cap divisor to describe
`tenured / 2` only as the tenured-proportional term. Remove the claim that
young-generation RSS is bounded by that value, and avoid asserting logarithmic
collection behavior as a guaranteed consequence when
`scavenge_nursery_cap_effective_bytes` may select the larger `influx_driven`
cap.
Audit before merge — verified end to end, merged as v0.5.1339Three-arm census reproduced on my own builds (same fixture, same trace
Both futile classes are gone: the handoff-over-empty-old-gen AND the Both tested changes survive my sabotage:
The design reasoning holds up under adversarial reading. The credit is a Maintainer decision on the gated ratchet cell, recorded here: Two follow-ups, both already assigned
#7592 stays open — 5.8 s vs bun's 618 ms is still ~9×. The named remainder |
…mic bitwise helper (#7592) (#7601) Two defects: charCodeAt was not statically Number (is_numeric_expr had no String-method arm, so integer xor went through the BigInt-aware dynamic helper), and there was no inline path (four opaque FFI calls per character). Adds the String-method arm (charCodeAt/indexOf/lastIndexOf/search/localeCompare; codePointAt deliberately excluded) and an inline ASCII fast path riding PERRY_STATIC_STRING_LOWERING, falling back to the same calls otherwise. fnv1a 11.2x (17.72 -> 1.58 ns/char), RSS unchanged. Also adds the missing test for #7596's tenured-proportional nursery cap. #7592 stays open.
…gate (#7554) An artifact defect used to abort validation on the first problem, and the artifact-validation step runs BEFORE the measurement step. So one cell — 12_large_live_set.heap_used_bytes, spread 6,768 bytes — meant none of the twelve probes executed on any branch for three days, while two GC pacing changes (#7594, #7596) merged with hand-run both-arms A/Bs standing in for the gate. The defect was a claim about ONE cell. Nothing about it voided the other 143, and nothing about it made the probes unrunnable. Defects now carry a scope. `artifact` (unreadable, tampered, missing metric) stays fatal and stays in preflight. `probe` (pinned without an oracle diff, or with no collection) and `cell` (contradicts the bit-identity premise of its own band) demote their subject out of the gating family and are reported as failures — so `check` still measures everything, still evaluates the other cells, and still names a regression elsewhere in the matrix, while the defect itself keeps the job red. `validate --scope structural` (what CI preflight now runs) fails only on the fatal kind. It cannot suppress: `check` re-derives the same list and fails on it, and a test asserts that coupling per planted defect shape. `assemble` is unchanged — pin time still refuses any defect outright, so this cannot be used to freeze a new unfit artifact.
…ost (#7554) gc-ratchet had not been green on main since 2026-08-01T05:39Z — 179 consecutive red main runs. The 2026-08-05 window where it could not reach its probes at all (#7554, fixed by #7557) was an episode inside that, not the whole of it: after #7557 restored measurement the job stayed red against a 0.5.1280 artifact that no longer described the collector. Re-pinned at origin/main 26b9c9d (0.5.1346) on perry-macos — the same Mac mini and the same rustc/cargo/clang the 2026-08-05 pin used, so this is like-for-like. All 12 probes oracle-pass; heap_used_bytes spread 0 on eleven and 864 B on 12_large_live_set. Full per-cell attribution is in the artifact's own `notes`. Three of the four moved groups are explained: - 03/04's copy and promote counters collapsing 40–99.8% is #7594 + #7596 doing what they said (less futile promotion). Recorded caveat: 03's promoted_* now pin at 0, where the allowance floor and the liveness assert both go quiet. - 02 +2.77% and 05 +16.44% retention are conservative-scan false roots, not retention. `classify` on this host gives 05 precise 5,329,880 — byte-identical to what #7571 measured at both ends of its window — and 02 precise 9,416,632, BELOW the number this baseline previously recorded. That is #7559's answer, reproduced rather than assumed. The fourth is flagged, not explained: 12_large_live_set.wall_ms 3,056 -> 3,471 ms (+13.58%), two non-overlapping 7-sample clusters on one host, while 06 and 11 got 9.6% and 28.4% faster. #7596 reported -7.4% on that cell, so by its own evidence this is not #7596. It is gated on pinned_host only. #7596's accepted 12_large_live_set.heap_total_bytes +36% did NOT reproduce here (110,100,480 -> 110,100,480, +0.00%), so nothing was re-pinned for it.
1 participant
Second half of #7592, stacked on #7594. Three changes, one principle: no constant band may pace a collector whose per-cycle cost is O(live) — total work goes quadratic in the live set, and a bigger constant only moves the cliff.
What was left after #7594
With the livelock latched, the 200k/500k trace still spent ~100 % of
build_outin GC pause, and two of the six remaining cycles were guaranteed futile:The changes
1. The survivor-promotion handoff fires on CURRENT old-gen pressure only. It used to fire on
old + promotable— a prediction of where old-gen would land after the promotion. But promotable bytes sit in the survivor space, where a full mark-sweep can neither reclaim them (they are live) nor reclaim the old-gen space they have not yet occupied. That is the #7594 mistake in another coat: scheduling a non-moving collection for bytes it cannot affect. The only useful work a handoff can do is clear current old garbage so the promotion lands in reused holes; over 4.2 MB of old-gen it is a 1,015 ms no-op.2. A copying minor's promoted bytes credit the old-reclaim baseline. Promoted bytes are live by construction — only marked-live objects get copied — so a reclaim fired because promotion crossed a threshold finds them all live and frees nothing. The credit is exactly the promoted delta, never a resync: pre-existing old garbage still counts. The trade is the standard GOGC one — promoted-then-dead bytes now wait for the growth band — and it is the one visible ratchet cost (below).
3. Both pacing bands become live-proportional.
max(32 MB, baseline / 2)— Go's GOGC shape. Shared byold_reclaim_pressure_dueandgc_old_reclaim_debt_bytesso dueness and debt cannot diverge (gc-matrix: --pressure disables the very path #7019 added — the 'default' arm runs ZERO copying minors on all 22 corpus rows #7024's two-predicates family).max(influx_driven, tenured_live / 2). Keyed on old-gen reclaimable pressure, NOT total arena in-use — the cap gatesyoung_scavenge_cap_due()against from-space occupancy, and a cap defined by a total that includes the young generation is a fixed point from-space can never cross (measured on the first attempt: scavenging stopped entirely, 0 copying minors at 200k). Removing this term costs an extra 1,100 ms minor at 500k — each extra minor pays the O(old-gen) fixed cost ([perf] Minor GC does O(old-gen) work: full-region sweep walk, whole-heap RS rebuild, per-cycle page-meta iterations #6181) — and the count grows with N, so it is load-bearing, not tuning.Measurements
Three arms, identically built and linked against a pinned
PERRY_RUNTIME_DIR, interleaved, output hash checked every row:Output hash identical on every row. RSS is +1.7 % over the latch arm at 500k (and −27 % of main's regression budget: main was 1,064 MB but 57 s slower). ns/record is flat within ~30 % across a 20× size range; on main it grows 70×.
The 500k trace after: 4 cycles, none futile — one parse-garbage reclaim (1,045 ms, frees 111 MB), one Eden scavenge, one promotion pass.
GC ratchet — one gated cell moves, and I am flagging it, not hiding it
Both arms measured back to back on one host (the pinned baseline is 0.5.1315 on another machine and cannot separate this change from drift). 144 metric medians across all 12 probes: every semantic counter identical except:
12_large_live_set.heap_total_bytes: 95.4 MB → 130.0 MB (+36 %) — gated, band 2 %, so the official check goes red on this cell.I attribution-tested it: with only the promoted-bytes credit disabled, the probe is byte-identical to the latch arm — the growth is 100 % the deferred post-promotion reclaim, i.e. the intended GOGC trade. On the same probe, same run:
heap_used_bytes+0.02 % (nothing extra retained),peak_rss_bytes−0.7 %,wall_ms−7.4 %, and every copy/promote/cycle counter identical. The growth is reserved-block high-water from collecting less often, not resident memory and not retention.This PR therefore needs a maintainer decision on that one baseline cell (
--update-baselinescoped to it, per the ratchet's own flow). If the reserved high-water is judged unacceptable, the credit (change 2) can be dropped independently — it is one call site — at the cost of reinstating the 2,100 ms futile full at 500k.Tests
test_copying_minor_promotion_handoff_requires_current_old_pressure— pins the perf: json_pipeline at 500k records is 97.6x bun (60.4s vs 618ms) while the same workload at 100 records BEATS bun — a scaling cliff, not a constant factor #7592 shape (108 MB promotable over 4.2 MB old-gen: NOT due) and that real current pressure still fires.test_old_reclaim_band_is_proportional_and_promotion_credits_baseline— the band floor/scaling, dueness/debt sharing one trigger, the credit arithmetic, and that a reclaim is not due immediately after promotion.perry-runtimesuite: 1,844 passed, 0 failed.cargo fmt --checkandcheck_file_size.shclean.What this does not fix
build_outat 500k is ~23,000 ns/record — flat, but still ~30× off the 764 ns/record a collection-free run shows. The remaining structural cost is the two-hop promotion (Eden→survivor→old copies 268 MB twice; 3.9 s of the 5.1 s). Collapsing it needs promote-on-first-copy within the first copying minor, which is a tenuring-policy design with #7432's determinism constraint — follow-up on #7592.Summary by CodeRabbit
Performance
Bug Fixes
Tests
Documentation