perf(gc): batch dead old-object page unregistration per sweep step - #10147
perf(gc): batch dead old-object page unregistration per sweep step#10147proggeramlug wants to merge 1 commit into
Conversation
A full collection unregistered each dead old object from the page index on its own: two `Vec` allocations, a deferral-buffer flush, a promoted-run materialization, both table borrows, and a linear `position` over the page's object list before `swap_remove`. Freeing every object on a page that way is quadratic in objects per page, and a full frees whole pages of small objects. On records_array_8m:scan (~720k dead records per full) `invalidate_dead_old_arena_header` was 15.8% of ALL samples -- the single largest cost of a full collection. The registration side already batches for exactly this reason (`flush_deferred_old_page_registrations_batch`, #7624). This is its mirror: `ArenaSweepObjectsState` still invalidates each dead header's fields immediately, so no walker can read it as live, but queues the page-index removal and flushes once per sweep step (and every 4096 headers, so an unbudgeted sweep never stages an unbounded buffer). The batch does one flush, one run materialization per touched page, one `retain` per page against that page's sorted dead headers, and one page-meta update per page -- allocated bytes and object counts only fall here, so applying a page's decrements together and resetting/refreshing once is the same state the per-object path reaches. Nothing inside a sweep step reads page-index membership, and the queue is empty at every step boundary. Four fulls on records_array_8m:scan: 241 ms -> 187 ms (-22%). The batched flush is 6.3% of samples where the per-object remover was 15.8%. `every_page_object_reader_expands_promoted_runs` covers the new remover (it touches OLD_GEN_PAGE_OBJECTS and expands first). A new test drives both removers over the same population in one arena -- page-spanning objects, a fully emptied page, partial pages -- and requires identical page membership and metadata; it was sabotage-tested (dropping the object-count decrement fails it with "page metadata diverged").
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (6)
Included review availability: Your plan provides up to 8 included reviews per hour; 3 remain after this review. 📝 WalkthroughWalkthroughThe arena now unregisters dead old-generation objects in batches. Old-generation sweeping queues dead headers and flushes them at step boundaries or after 4096 entries. Tests compare batched and per-object page metadata and membership results. ChangesOld-generation batch unregister
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Refactor Sequence Diagram(s)sequenceDiagram
participant ArenaSweepObjectsState
participant PendingOldUnregister
participant unregister_old_objects_batch
participant PageIndex
participant PageMetadata
ArenaSweepObjectsState->>PendingOldUnregister: defer dead header and size
ArenaSweepObjectsState->>PendingOldUnregister: flush at step boundary
PendingOldUnregister->>unregister_old_objects_batch: submit queued removals
unregister_old_objects_batch->>PageIndex: remove dead headers by page
unregister_old_objects_batch->>PageMetadata: update bytes and object counts
Merge Risk: ⚪ Minimal · up to The batched sweep change has no remaining actionable correctness risk from the reviewed behavior and is ready to merge. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 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 |
|
Validation status for taking this out of draft:
|
A full collection unregistered every dead old object from the page index one at a time, and that removal is quadratic in objects per page.
The cost
For each dead old object,
invalidate_dead_old_arena_header→unregister_old_object_pagesdid:Vecallocations (page overlaps,removed_pages)RefCelltable borrows and HashMap lookupspositionthrough the page's object list beforeswap_removeA full collection frees whole pages of small objects, so removing each page's objects one by one is O(k²) in objects per page. Profiled on
records_array_8m:scan(~720k dead records per full),invalidate_dead_old_arena_headerwas 15.8% of all samples, the single largest cost of a full collection.The registration side already batches for exactly this reason (
flush_deferred_old_page_registrations_batch, #7624). This is its mirror.The change
ArenaSweepObjectsStatestill invalidates each dead header's fields immediately, so no walker can read it as live. It queues the page-index removal and flushes:step(the queue is empty at every step boundary, so no stale entry is ever visible to the mutator or a minor), andunregister_old_objects_batchdoes one deferral flush, one run materialization per touched page, oneretainper page against that page's sorted dead headers (binary search), and one page-meta update per page. Allocated bytes and object counts only fall during removal, so applying a page's decrements together and then resetting/refreshing once reaches the same state as the per-object path.reset_cycle_sweep_accountingandrefresh_policy_bitsare pure recomputes of the page's own fields. Its scratch buffer is caller-owned and reused, so a warm flush allocates nothing (the #7624 lesson about per-batch staging buffers and peak RSS).Nothing inside a sweep step reads page-index membership.
Results
Four fulls on
records_array_8m:scan: 241 ms → 187 ms (−22%). The batched flush is 6.3% of samples where the per-object remover was 15.8%.Validation
cargo test --release -p perry-runtime --lib: 3698 passed, 0 failed (twice) on this branch's base.arena::tests_batch_unregister: drives the per-object and batched removers over the same population in one arena (page-spanning objects, a fully emptied page, partial pages) and requires identical page membership and metadata. Sabotage-tested: dropping the object-count decrement fails it withpage … metadata diverged.every_page_object_reader_expands_promoted_runscovers the new remover. It touchesOLD_GEN_PAGE_OBJECTSand expands runs first, on substance, not just on the pattern the test greps for.PERRY_GC_SCHEDULE_SEED11–14, rate 0.1,PERRY_GC_PROTECT_FROMSPACE=1, depth 32) over a workload that promotes 40k records per round, frees half, forces fulls and verifies every survivor: 0 bad values, no faults across ~9,100 copying minors per seed. It was confirmed to exercise the path: 30 fulls withreclaim_dead_old_blocks, 582 MB freed, 325 MB of old holes made reusable.--filter test_gap_gc, Node 26.5.1 oracle): 50/51. The one failure,test_gap_gc_http2_pending_event_callback_rooting, is a 10 s timeout that reproduces identically on the pristine merge base5603d63d17(2/2), so it's pre-existing on this host, not caused here.Before merging
This changes the full sweep, so the gc-ratchet corpus should run before/after. Draft until it has.