gc: old-generation hole free list — swept holes become reusable capacity (#7437) - #7443
Conversation
📝 WalkthroughWalkthroughThis PR adds exact-size old-generation hole reuse after sweep, rebuilds reusable-hole state from surviving old blocks, removes stale holes during block reclaim, updates pressure and heap accounting to subtract reusable bytes, extracts old-page defragmentation selection, and adds tests and changelog notes. ChangesOld-generation hole reuse flow
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant Sweep as gc::oldgen sweep
participant FreeList as gc::old_free
participant Alloc as arena_alloc_gc_old
participant Stats as js_arena_stats
Sweep->>FreeList: old_free_rebuild_from_live_old_blocks(...)
FreeList-->>Alloc: old_free_take_exact(total_size, excluded_pages)
Alloc-->>Alloc: reuse hole or fall back to arena allocation
FreeList-->>Stats: old_free_bytes()
Stats-->>Stats: subtract reusable bytes from used
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 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 |
Old-gen allocation was pure bump: a swept dead old object stayed dead capacity until its entire block died, and a block with one live object never resets. Scattered survivors therefore pinned 105 MB of blocks for a ~1 MB live set (12_large_live_set), and every dropped tree.ts cohort left holes no future promotion could fill. Swept dead old objects in still-live blocks now become exact-size reusable holes (size-bucketed map; exact fit keeps GcHeader::size in agreement with per-object promotion accounting). Both sweep variants populate it; old-block reset/dealloc paths filter their ranges; the old-reclaim pacing arms and process.memoryUsage().heapUsed subtract the reusable bytes so dead-but-reusable capacity no longer reads as pressure a full collection cannot relieve. Splits old-page defrag SELECTION out of oldgen.rs into oldgen_defrag.rs (2000-line lint cap).
…uring the walk The staging vector was O(dead old objects) — ~17 MB of peak RSS on 12_large_live_set, added to the very number this feature lowers. The rebuild walks only surviving old blocks at sweep completion (invalidated headers are exactly obj_type == 0) and is idempotent: a reused hole gains a real obj_type and drops out, a dead block is never visited, so no cross-sweep dedup bookkeeping can drift.
The walkable-gated walkers step over invalidated (obj_type == 0) headers without invoking the callback, so the rebuild recorded zero holes — probe 12's retained heap went straight back to 105.6 MB. The new old_arena_walk_all_headers_filtered visits every header in the selected old blocks; stepping stays safe because invalidation preserves GcHeader::size exactly.
ac07f1e to
689661c
Compare
![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/arena/allocators.rs`:
- Around line 124-140: Preserve requested alignment when reusing old-generation
holes in the arena allocation paths around the exact-fit branches and the
corresponding allocation at lines 166-180. Update old_free_take_exact and
old_free_push usage so free entries retain and are filtered by alignment,
passing the caller’s required alignment and accepting only suitably aligned user
pointers before initializing GcHeader and registering the object.
In `@crates/perry-runtime/src/gc/tests/oldgen.rs`:
- Around line 1410-1479: Add a test-only exact-bucket free-list membership query
near the existing old-free helpers, then update both tests to use it: after the
first sweep in the relevant test, assert every pointer in dead is present in the
exact dead_total bucket, and after the second sweep in
test_old_holes_are_dropped_when_their_block_is_reclaimed, assert every dead
pointer is absent. Keep the existing allocation and count assertions, but do not
rely on old_free_entry_count() alone to establish these test-created holes.
🪄 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: 4e131ac9-5d96-4ddb-a440-fc1b13a01fc9
📒 Files selected for processing (12)
changelog.d/7443-oldgen-hole-free-list.mdcrates/perry-runtime/src/arena/allocators.rscrates/perry-runtime/src/arena/mod.rscrates/perry-runtime/src/arena/reset.rscrates/perry-runtime/src/arena/stats.rscrates/perry-runtime/src/arena/walk.rscrates/perry-runtime/src/gc/mod.rscrates/perry-runtime/src/gc/old_free.rscrates/perry-runtime/src/gc/oldgen.rscrates/perry-runtime/src/gc/oldgen_defrag.rscrates/perry-runtime/src/gc/policy.rscrates/perry-runtime/src/gc/tests/oldgen.rs
| // #7437: reuse a swept same-size hole before bumping — otherwise a | ||
| // block with any live object never yields its dead bytes back and old | ||
| // capacity only ever grows. Exact fit keeps `GcHeader::size` equal to | ||
| // what per-object promotion accounting records for this allocation. | ||
| if let Some(user_ptr) = crate::gc::old_free_take_exact(total, None) { | ||
| let raw = (user_ptr - GC_HEADER_SIZE) as *mut u8; | ||
| unsafe { | ||
| let header = raw as *mut GcHeader; | ||
| (*header).obj_type = obj_type; | ||
| (*header).gc_flags = GC_FLAG_ARENA | crate::gc::gc_birth_extra_flags(); | ||
| crate::gc::gc_note_black_birth(header); | ||
| (*header)._reserved = 0; | ||
| (*header).size = total as u32; | ||
| } | ||
| register_old_object_pages(raw as usize, total); | ||
| return user_ptr as *mut u8; | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the old-arena allocation contract and all old-generation callers.
ast-grep outline crates/perry-runtime/src/arena/allocators.rs --items all
rg -n -P -C 8 'fn\s+alloc(?:_excluding_pages)?\s*\(' crates/perry-runtime/src/arena
rg -n -P -C 4 'arena_alloc_gc_old(?:_excluding_pages)?\s*\(' crates/perry-runtime/src
# Verify whether each returned user pointer is aligned for every supported
# caller alignment, or update old_free_take_exact to filter by alignment.Repository: PerryTS/perry
Length of output: 50370
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "## allocators.rs relevant sections"
sed -n '100,190p' crates/perry-runtime/src/arena/allocators.rs
echo "## old_free_* definitions/usages"
rg -n -C 5 'old_free_(take_exact|add|_|state|size|addr)' crates/perry-runtime/src
echo "## padding total/alignment behavior in allocators"
sed -n '190,320p' crates/perry-runtime/src/perry-runtime/src/arena/allocators.rs 2>/dev/null || sed -n '190,320p' crates/perry-runtime/src/arena/allocators.rs
echo "## all old-generation arena_alloc_gc_old occurrences (non-test summary)"
rg -n 'arena_alloc_gc_old\(|arena_alloc_gc_old_excluding_pages\(' crates/perry-runtime/src --glob '!**/gc/tests/**/*.rs' --glob '!**/native_arena.rs' | head -n 200Repository: PerryTS/perry
Length of output: 16565
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "## old_free.rs implementation"
sed -n '1,200p' crates/perry-runtime/src/gc/old_free.rs
echo "## prod non-test old-generation allocation calls with context"
for f in crates/perry-runtime/src/buffer/header.rs crates/perry-runtime/src/typedarray/mod.rs crates/perry-runtime/src/gc/copying.rs crates/perry-runtime/src/gc/oldgen.rs crates/perry-runtime/src/gc/copying.rs; do
echo "### $f"
rg -n -C 4 'arena_alloc_gc_old|arena_alloc_gc_old_excluding_pages' "$f"
done
echo "## all arena_alloc_gc call sites passing non-8 align (prod/tests)"
rg -n 'arena_alloc_gc\([[:space:]]*,[[:space:]]*[^0-9].*?\)' crates/perry-runtime/src | head -n 200Repository: PerryTS/perry
Length of output: 12146
Preserve alignment when reusing old-generation holes.
old_free_take_exact selects by total size only, and old_free_push stores the returned user pointer without an alignment key. A swept object from a lower-alignment allocation can be returned to a higher-alignment caller once padding happens to produce the same total. Pass the required alignment through old_free_take_exact, take only aligned entries, or key holes by alignment as well as total size.
Also applies to: 166-180
🤖 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/arena/allocators.rs` around lines 124 - 140,
Preserve requested alignment when reusing old-generation holes in the arena
allocation paths around the exact-fit branches and the corresponding allocation
at lines 166-180. Update old_free_take_exact and old_free_push usage so free
entries retain and are filtered by alignment, passing the caller’s required
alignment and accepting only suitably aligned user pointers before initializing
GcHeader and registering the object.
| assert!( | ||
| old_free_entry_count() >= dead.len(), | ||
| "each swept dead neighbor must become a hole (got {})", | ||
| old_free_entry_count() | ||
| ); | ||
| let free_bytes = old_free_bytes(); | ||
| assert!(free_bytes >= dead.len() * dead_total); | ||
| assert_eq!( | ||
| old_gen_reclaimable_pressure_bytes(), | ||
| crate::arena::old_gen_in_use_bytes().saturating_sub(free_bytes), | ||
| "reclaim pacing must see in-use minus the reusable holes" | ||
| ); | ||
|
|
||
| // Exact-size allocation reuses a hole instead of growing the bump. | ||
| let reused = crate::arena::arena_alloc_gc_old(40, 8, GC_TYPE_STRING) as usize; | ||
| assert!( | ||
| dead.contains(&reused), | ||
| "a same-size old allocation must land in a swept hole" | ||
| ); | ||
| assert_eq!(old_free_bytes(), free_bytes - dead_total); | ||
|
|
||
| // A different size must NOT take a hole (exact fit keeps GcHeader::size | ||
| // in agreement with per-object promotion accounting). | ||
| let other = crate::arena::arena_alloc_gc_old(96, 8, GC_TYPE_STRING) as usize; | ||
| assert!( | ||
| !dead.contains(&other), | ||
| "a different-size allocation must not be placed in a mismatched hole" | ||
| ); | ||
| assert_eq!(old_free_bytes(), free_bytes - dead_total); | ||
|
|
||
| old_free_reset_for_test(); | ||
| clear_marks(); | ||
| remembered_set_clear(); | ||
| } | ||
|
|
||
| /// A hole's block can die on a LATER cycle; the block reclaim that recycles | ||
| /// its bytes must drop the block's holes so the free list never hands out a | ||
| /// pointer into recycled memory. | ||
| #[test] | ||
| fn test_old_holes_are_dropped_when_their_block_is_reclaimed() { | ||
| let _isolation = copying_nursery_isolation_lock(); | ||
| reset_remembered_set(); | ||
| clear_marks(); | ||
| clear_mark_seeds(); | ||
| old_free_reset_for_test(); | ||
| crate::arena::old_pages_begin_gc_cycle(); | ||
|
|
||
| let live = crate::arena::arena_alloc_gc_old(40, 8, GC_TYPE_STRING) as usize; | ||
| let mut dead = Vec::new(); | ||
| for _ in 0..4 { | ||
| dead.push(crate::arena::arena_alloc_gc_old(40, 8, GC_TYPE_STRING) as usize); | ||
| } | ||
| let (live_header, _) = old_test_header_and_size(live); | ||
| let (_, dead_total) = old_test_header_and_size(dead[0]); | ||
| unsafe { | ||
| (*live_header).gc_flags |= GC_FLAG_MARKED; | ||
| } | ||
| let _sweep = sweep_with_age_bump_and_old_reclaim(false, true); | ||
| assert!(old_free_entry_count() >= dead.len()); | ||
|
|
||
| // Second cycle: the anchor dies too, the whole block reclaims, and the | ||
| // filter must remove every hole inside it. | ||
| crate::arena::old_pages_begin_gc_cycle(); | ||
| let _sweep2 = sweep_with_age_bump_and_old_reclaim(false, true); | ||
| while let Some(ptr) = old_free_take_exact(dead_total, None) { | ||
| assert!( | ||
| !dead.contains(&ptr) && ptr != live, | ||
| "a hole inside a reclaimed block must never be handed out" | ||
| ); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Assert the identity of every expected hole.
old_free_entry_count() counts holes from all swept old objects. old_free_reset_for_test() clears only free-list metadata. It does not clear existing old-arena objects. Unrelated holes can satisfy >= dead.len() while entries from this test are missing.
The first test proves reuse for only one dead pointer. The second test can pass without proving that its block created holes before the later reclaim. Add a test-only exact-bucket membership query. Assert that every pointer in dead is present after the first sweep. In the second test, assert those entries are absent after the second sweep.
🤖 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/tests/oldgen.rs` around lines 1410 - 1479, Add a
test-only exact-bucket free-list membership query near the existing old-free
helpers, then update both tests to use it: after the first sweep in the relevant
test, assert every pointer in dead is present in the exact dead_total bucket,
and after the second sweep in
test_old_holes_are_dropped_when_their_block_is_reclaimed, assert every dead
pointer is absent. Keep the existing allocation and count assertions, but do not
rely on old_free_entry_count() alone to establish these test-created holes.
|
Audited and merged. I re-verified the load-bearing invariants from source rather than from the description, and rebased the branch off the now-merged #7432 (it was still based on
No duplicate handout. Rebuild clears the map and visits each header once, so a pointer cannot enter two buckets — which is the real safety argument for rebuild-over-staging, beyond the ~17 MB RSS you measured. Range-invalidation coverage is complete. I checked this the other way round — enumerating every path that recycles old bytes rather than trusting the three call sites. Reuse is byte-for-byte symmetric with the bump path — same header fields written in the same order, same Tests: 611 pass / 0 fail ( Two notes, neither blocking:
|
…tention win (#7446) Captured on the new pinned quiet host (Mac mini M1, 8 GB, Spotlight disabled, cpu_active 4-7% at capture) at main 5e236e6. Locks 12_large_live_set's heap_used at 59.9 MB — under the previous 105.6 MB pin, a regression back to unreclaimable scattered-survivor retention would have PASSED the increase-direction band. Counter families are unchanged from the previous pin (0% spread across 7 repeats); memory and wall medians re-baseline on the dedicated host, whose observed wall spread (~1-2%) is far inside the pinned_host bands. Co-authored-by: Ralph Küpper <ralph@skelpo.com>
1 participant
Fixes #7437. Was stacked on #7432; now rebased onto
main(post-merge) and retargeted.Problem
Old-gen allocation was pure bump: a swept dead old object stayed dead capacity until its entire block died, and a block with even one live object never resets. The
12_large_live_setratchet probe (from #7432) pins the pathology: promote a ~100 MB linked structure, keep every 64th node (~1 MB truly live), drop the rest,gc()— the full collection frees 87.6 MB of objects and reclaims nothing: 49 of 50 blocks still hold a live object,heapUsedstays at 105.6 MB, and the dead bytes also keep counting as old-gen pressure, so old-reclaim keeps re-firing full collections that cannot lower the number they are watching.Fix — old-generation hole free list (
gc/old_free.rs)Swept dead old objects in still-live blocks become exact-size reusable holes:
old_page_account_promoted_object), so a reused slot must be exactly the requested size or the page live-byte accounting diverges from the header. Promoted cohorts are dominated by uniform class-instance sizes — exact fit hits where the pathology lives.obj_type == 0. Two measured reasons over the staging-vector draft: the staging vector was O(dead old objects) and added ~17 MB of peak RSS to the very number this feature lowers, and rebuild is idempotent (a reused hole gains a realobj_typeand drops out; a dead block is never visited), so no cross-sweep dedup bookkeeping can drift. The raw walker (old_arena_walk_all_headers_filtered) is load-bearing: the walkable-gated walkers step over invalidated headers without invoking the callback — a rebuild written against them records zero holes, which the unit tests caught.arena_alloc_gc_old(promotions, large-object births) and its defrag-aware_excluding_pagesvariant; reuse rewrites the header through the standard birth path (flags, black-birth note, page re-registration).OLD_GEN_IN_USE_BYTESkeeps its sum-of-offsets definition (debug-asserted); consumers that want live pressure subtract the hole bytes instead — the old-reclaim pacing arms (old_gen_reclaimable_pressure_bytes) andprocess.memoryUsage().heapUsed.PERRY_GC_DIAG=1prints[gc-old-free] reusable_bytes=after each rebuild so a run can prove the subject is live.Also splits old-page defrag selection out of
oldgen.rsintooldgen_defrag.rs(2000-line lint cap).Measurements (macOS arm64, same binary)
heapUsedafter release-phasegc()heap_usedimprovements ride within the ratchet'sincrease-direction tolerance, but the baseline will be deliberately re-pinned on the quiet host so the ratchet locks the better number in — otherwise a future regression back to 105 MB would pass.Tests
test_dead_old_holes_in_live_blocks_are_reused_by_exact_size— sweep populates the map, same-size old allocation lands in a hole, different size does not, pressure = in-use − holes.test_old_holes_are_dropped_when_their_block_is_reclaimed— a hole's block dying on a later cycle removes its entries (these two tests caught the zero-holes walker bug during development).gc::module green serialized (610/610).Summary by CodeRabbit
Performance Improvements
Diagnostics