perf(gc): prune the per-object layout tables from a young-entry log on a minor - #9895
Draft
proggeramlug wants to merge 5 commits into
Draft
perf(gc): prune the per-object layout tables from a young-entry log on a minor#9895proggeramlug wants to merge 5 commits into
proggeramlug wants to merge 5 commits into
Conversation
added 3 commits
September 6, 2026 06:56
… filter that admits when it is outgrown `prune_dead_per_object_layout_owners` walked every live key three times per collection — `retain`, `layout_addr_filter_rebuild` (which buffered them all into a `Vec<usize>` first), then `recount_young_layout_records`. The last two want exactly the survivor set `retain` already visits, so they fold into its closure and the `Vec` disappears; on the compiled claude-code TUI that `Vec` alone allocated 50.6 MB per 400-character reply. The new `PERRY_LAYOUT_DIAG` instrument reports what made this expensive: 162,258 live keys against a 4,096-bit address sketch documented for "one or two entries", with all 4,096 bits set. Every probe answers "may hold", so the early returns the sketch exists to serve never fire, and each rebuild is an O(live keys) walk that restores the all-ones state it started from. Past one eighth of the bits the rebuild now reaches that state in O(1) instead. Widening the sketch is a codegen change — the geometry and hash are mirrored in `emit_gated_forget_object_layout` — and would need ~190 KB of inline TLS per thread to discriminate at this occupancy. `transfer_per_object_descriptor` gains the emptiness test its shared flag cannot express: one `len` load instead of two hashes per evacuated object, for a map that is empty for the whole of a cc turn. `LAYOUT_DIAG` is declared with `crate::perry_thread_local!`, as `scripts/check_thread_locals.py` requires of every new declaration. Claude-Session: https://claude.ai/code/session_014UZWia6L37DpA93VLtNK9m
…n a minor A minor's death prune for `LAYOUT_SLOT_MASKS` + `TYPED_LAYOUTS` asked "which owner died?" of every key in tables sized by everything the program ever created. Both of a minor's deadness predicates require the owner to be in the nursery — `owner_is_dead_copied_minor_from_space` demands eden or the active survivor half, `PostTraceProbe::owner_is_dead` on a minor demands an in-arena, untenured `HeapGeneration::Nursery` address — so an owner that was old at the last prune is still old and the visit cannot remove anything. Measured on a compiled claude-code streamed reply (`PERRY_GC_DIAG=1`, counter on a measurement-only branch): 6,792,375 entries visited across 109 minor prunes, of which at most 125,367 (1.85 %) could possibly have died; median `young_before/visited` 0.0000; 40 of those 109 minors had NOTHING that could have died while carrying 39 % of all visits. 54x over-visit at 3300 characters, 25x at 400. `dead <= young_before` held on 152/152 minor prunes. So both maps take the young-entry log of PerryTS#9754 (`gc/young_log.rs`), kept in the existing `PerObjectLayoutHint` hot slot so no new thread-local is declared and every writer arms it with a TLS resolution it has already paid for: * every writer notes a key `layout_key_may_be_nursery` admits BEFORE the entry becomes findable (rule 1) — the two insert wrappers, the in-borrow mask mint in `layout_note_slot`, and both per-object move hooks; * a minor walks the log: a logged key in neither map is stale and drops, a present dead key is removed from both maps, a live key is re-logged only while still young, so a promoted owner leaves the log for good; * a full prune keeps its whole-table walk (old owners DO die in a full trace) and rebuilds the log from the survivors it is already classifying; * under `debug_assertions` the young prune re-derives the candidate set from the maps and panics on any young key the log does not name (rule 2); * `note_walk` records logged/visited/kept/table_len per prune, so `[gc-young-log] table=gc.layout_tables` prints the skip and the tests read it back (rule 3). The address filter is deliberately NOT rebuilt on a minor: the whole-table walk that rebuilt it is what this removes, its `false` is the only load-bearing answer, and a stale set bit is a false positive. The amortised rebuild in `layout_addr_filter_add` and the full prune keep it selective. Why this table pays where PerryTS#9754's scanners did not: a scanner keeps `addr_is_minor_relevant`, true for `Longlived` by design, and cc allocates its shape-key arrays longlived, so those logs never drain (`kept/logged` median 1.000, and `closure.dynamic_props` is a 2.56x regression there). A prune excludes `Longlived` and `Old` both, and cc promotes every survivor after one survival. Same mechanism, opposite sign, decided by the predicate. `YoungLog::clear` loses its `#[cfg(test)]`: a prune that early-returns on the emptiness proof must drop the log with it, or a workload that repeatedly fills and empties the maps between collections accumulates stale keys for ever.
…rse than full PerryTS#9754 gave four side tables a minor-scoped root scan. Measured per table on the compiled claude-code TUI, three of them earn it and this one does not. Both arms of a 3-turn 3300-character run on a quiet host (`[gc-young-log]` rows, 273 and 272 minor cycles): | table | skipped | verdict | |---|---|---| | `gc.layout_tables` | 99.0 % (366,347 visited of 36,938,896) | earns it | | `object.descriptors` | 95.7 % | earns it | | `object.transition_cache` | 83 % | earns it | | `shapes.families+indices` | 80 % | earns it | | **`closure.dynamic_props`** | **2.2-2.7 %** | **WORSE THAN FULL on 223/273 and 217/272 cycles** | The cause is one predicate, and it is why the same technique lands so differently on adjacent tables. A SCANNER's keep-test is `addr_is_minor_relevant`, which returns true for `Longlived` BY DESIGN — a longlived object can point at a young one — so the "relevant" set is close to the whole table and the log never drains: `kept/logged` has median 1.000 here. Multi-round re-logging then makes the young walk visit MORE entries than the full walk it replaces. That is the same finding, and the same fix, as the shape-cache young log dropped for skipping 0 % and costing 35 % more. So every pass over this table is a full pass again. The LOG ITSELF STAYS. `prune_dead_closure_side_table_owners_young` uses it, and a PRUNE's predicate is not the scanner's: it asks who DIED, so it excludes `Longlived` and `Old` both. That asymmetry is the whole point — the same mechanism is a 99.0 % skip on one walk and a 2.2 % skip on another, decided entirely by which predicate the walk keeps on. The full scanner rebuilds the log from what it finds, so the prune's candidate set stays complete. Rule 2 moves with it. The log-completeness re-derivation ran at the top of the minor-scoped scanner; that walk is gone, and the prune is now the log's only consumer, so the machine check that catches a writer publishing without arming moves into the prune. Dropping the scanner without moving it would have deleted the only guard on a log a prune still trusts — the failure would not have been a slow walk but a dead owner's entries surviving in silence. Not yet verified: this file has only been compiled with `debug_assertions` OFF, so the moved `#[cfg(debug_assertions)]` call has not been type-checked and the moved rule 2 has not been observed to fire.
proggeramlug
added
the
run-extended-tests
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks 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 |
added 2 commits
September 6, 2026 17:40
… contract, and prove the moved rule-2 guard can fail Two problems with the previous commit, both mine. **1. It broke three tests and I did not run them.** `41a8af7da` reversed the walk policy for `closure.dynamic_props` without grepping for the tests that assert it. On a debug-assertions run: 3225 passed, 3 failed — `young_closure_prop_value_is_moved_through_the_log`, `young_value_under_an_old_closure_owner_is_logged_by_the_value` and `old_closure_entries_are_skipped_by_a_minor`, each asserting `row.partial`, i.e. exactly the policy that commit reverses. They fail in release too. Re-pointed at the current contract rather than deleted, because in every case the property under test survives and only the walk that delivers it changed: * `..._is_traced_and_moved_by_a_minor` — a value reachable ONLY through the side table is still traced, evacuated and re-keyed. Now asserts the FULL walk, and that it rebuilds the log for the death prune. * `..._is_traced_by_a_minor` — a young value and prototype under an OLD owner are still traced. Renamed off "is_logged_by_the_value": the log is still armed by the value, but its only consumer is now the prune, which keys on OWNERS, so value-based arming has no reader left. Flagged in the test, not removed — dropping it is a separate change needing its own measurement. * `old_closure_entries_survive_a_minor_full_walk` — was "are skipped by a minor", asserting `visited == 0`. There is no skip left to observe for this table, which IS the finding; it now asserts the full walk and the surviving correctness property. **2. The rule-2 guard I moved had been exercised but never seen to fail.** The layout-prune tests drive the prune and passed, so the moved `debug_assert_closure_young_log_complete` compiles and runs — but "did not fire" is not "can fire", and a check that cannot fail is documentation. `dropping_a_logged_closure_owner_trips_the_prune_rule2_check` drops every logged owner while LEAVING the three tables populated — precisely what a writer publishing without arming leaves behind — and requires the prune to panic with its own named message. Clearing the tables as well would empty the relevant set and the guard would return early, i.e. prove nothing; the new `test_drop_closure_young_log` hook exists to avoid exactly that. It runs in a child process: the panic is raised inside a collection and can cross an `extern "C"` frame and abort rather than unwind, which `#[should_panic]` cannot catch. Asserting on the child's status and stderr is robust to both, and is the isolation pattern `test_armed_per_object_layout_thread_exit_disarms_global_count` already uses. It is `#[cfg(debug_assertions)]`, since a release build has no guard to trip and the child would exit 0. NOT COMPILED. Disk on this box is at 9 GB against a 12 GB floor, so this commit has not been type-checked, formatted-checked beyond `cargo fmt`, or run. It needs a debug `cargo test -p perry-runtime --lib` before it is believed.
`old_closure_entries_survive_a_minor_full_walk` failed on
`assert!(row.table_len >= 2)` with
`YoungLogWalk { partial: false, logged: 1, visited: 1, kept: 0, table_len: 1 }`.
It is the assertion, not the semantics, and the reason is a field-name
collision I carried over without checking.
**`table_len` means two different things in this one table's two walks.** The
young walk computed `props.len() + prototypes.len() + deleted.len()`; this
fixture's single owner appears in `props` AND `deleted_keys`, so it counted 2.
The full walk builds a deduped OWNER vec, so the same state counts 1. I moved
the assertion from one walk to the other and kept the old number.
**`kept=0` with `logged=1` is correct and is not a stale log entry.** On a
FULL row `logged` is set to `table_len` by construction (`logged: table_len,
visited: table_len`), so it means "one owner considered", not "one entry still
logged". The log after the walk holds `kept` entries, and 0 is right: an old
owner whose only value is a number and whose only other state is a deleted key
has nothing a minor can act on, so it must not be re-logged. That property is
now asserted (`kept < table_len`) instead of being an unexplained field in a
failure message.
Test-only. No product code changes.
The collision cuts the safe way for the verdict that motivated the revert, and
that is worth stating rather than leaving to be rechecked. `young_log_an.py`
derives `skipped = table_len * passes - visited` and flags `visited >
table_len * passes` as worse-than-full. In young mode `table_len` is the
INFLATED sum-of-three-maps, so it makes worse-than-full HARDER to trigger and
makes the reported skip percentage LARGER than the truth. The measured
`closure.dynamic_props` result — 2.2-2.7 % skipped, worse than full on 223/273
and 217/272 cycles — is therefore conservative in both directions, and the
real case for taking that table off its young walk is stronger, not weaker.
jdalton
reviewed
Sep 7, 2026
jdalton
left a comment
jdalton
left a comment
Contributor
There was a problem hiding this comment.
Review of 0a427a39f2d3ad08c0be0c57b27e0d329f525c80 (2026-09-07).
The distinction between death-pruning young owners and scanning old owners that may reference young values is essential, and the diff makes it explicit. Please resolve the telemetry units alongside #9897: the layout full walk reports map-entry counts (masks.len() + typed.len()), while the young log deduplicates keys and reports unique-key visits/kept counts. A key in both maps counts twice in one mode and once in the other. Add a fixture with an owner present in both maps and name/report map entries and unique owners separately before using their ratio as a speedup. This does not establish a GC correctness defect; it affects the performance evidence.
Validation scope: source/diff inspection; I have not run this PR's build or test suite locally.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Draft. A clean 5-round rotation on main decides ready, and the head commit
is not yet compiled (see Known gaps).
What
A minor's death prune for the two per-object layout tables
(
LAYOUT_SLOT_MASKS+TYPED_LAYOUTS) walks a young-entry log(
gc/young_log.rs) instead of both tables. The second commit takesclosure.dynamic_propsoff its young scanner walk, which the samemeasurement shows is worse than the full walk it replaced. The third fixes the
tests the second broke and proves the guard it moved can fail.
Why the prune, and why only the prune
Both of a minor's deadness predicates require the owner to be in the nursery:
owner_is_dead_copied_minor_from_spacedemands eden or the active survivorhalf, and
PostTraceProbe::owner_is_deadon a minor demands an in-arena,untenured
HeapGeneration::Nurseryaddress. An owner that was old at the lastprune is still old, so visiting it cannot remove anything.
layout_key_may_be_nurseryis a strict superset of both, anddead <= young_beforeheld on 152/152 minor prunes when measured directly.A full prune keeps its whole-table walk — old owners do die in a full trace —
and rebuilds the log from the survivors it already classifies.
Measured
Quiet host.
HC= main35c36f425+ #9838 + #9860 + #9857 + a measurementcounter;
HCY= HC + this branch; both relinked on the same cached objects, sothe pair differs by this change alone.
Peak RSS 559–566 vs 557–563 MB — flat.
[gc-time]minor_us22.12 s (48.3 %)→ 20.28 s (46.0 %) at 3300, 2.72 → 2.54 at 400.
Load confound, stated: 1-minute load fell from 12–15 during the counter
runs to about 2 by the last timing round, which favours later runs. HCY was
lower in the two rounds where HC ran first, so ordering does not account
for the direction — but the 5-round rotation against plain main is what makes
this ready.
An independent counter says the win is not something else moving: the #5029
dirty-coverage restore walk is the same work in both arms (
dirty_old_pages992 → 1,008,
slots_visited58,505 → 57,972, productivity 0.0245 vs 0.0246).The mechanism, which does not depend on a timing run
From
[gc-young-log] table=gc.layout_tables, 273 minor cycles: 366,347entries visited against 36,938,896 the whole-table walk would visit — 99.0 %
skipped, zero cycles worse than a full walk, and inert in full mode (0
skipped) by design. On an earlier base the same rows read 96.2 % / 26×, with
median entries-visited-per-prune of 1 and 41 of 111 minors visiting
zero — the walk is not shrunk, it is deleted.
kept/loggedmedian 0.000:this log drains.
Second commit:
closure.dynamic_propsback to full modegc.layout_tablesobject.descriptorsobject.transition_cacheshapes.families+indicesclosure.dynamic_propsOne predicate explains the spread. A scanner keeps
addr_is_minor_relevant, true forLonglivedby design because a longlivedobject can point at a young one, so its candidate set is close to the whole
table and the log never drains (
kept/loggedmedian 1.000 there). A pruneasks who died, so it excludes
LonglivedandOldboth. Same mechanism,opposite sign — which is why this PR adds a young walk to one table and removes
one from another.
The log stays: the death prune still uses it and the full scanner rebuilds it.
Rule 2's log-completeness re-derivation moves into that prune, because the
scanner it lived in is gone and the prune is now the log's only consumer.
Dropping it would have removed the only guard on a log a prune still trusts,
and the failure mode is a dead owner's entries surviving silently, not a slow
walk.
Third commit: the tests the second one broke, and a guard proved able to fail
The policy reversal broke three tests that assert
row.partialforclosure.dynamic_props— I changed the contract without grepping for itstests. They are re-pointed rather than deleted: in each case the property under
test (a side-table-only value is traced and evacuated; a young value under an
old owner is traced; an old owner's entries survive) is unchanged, and only the
walk delivering it differs.
old_closure_entries_are_skipped_by_a_minorbecame..._survive_a_minor_full_walk, because there is no skip left to observe forthat table — which is the finding.
The moved rule-2 guard had been exercised and never fired, which is not the
same as being able to fire.
dropping_a_logged_closure_owner_trips_the_prune_rule2_checkdrops every logged owner while leaving the tables populated — what a writer
publishing without arming leaves behind — and requires the prune to panic with
its own named message. It runs in a child process because that panic can cross
an
extern "C"frame and abort rather than unwind.Correctness
in-borrow mask mint in
layout_note_slot, and both per-object move hooks.gc/layout.rs:958makesgc::tests::runtime_roots::hook_dispatch_handles::test_bound_timer_dispatch_roots_args_during_async_hook_init_gcpanic with "young log for gc.layout_tables does not name …". The witness is
an unrelated fixture, not the test written for it — the stronger result.
note_walkper prune; three tests read the rows back.PERRY_YOUNG_LAYOUT_RECORDSstays exact — the codegen gateemit_gated_forget_object_layoutdepends on it — because the young walkcounts per map entry (
in_masks + in_typed), as the full prune does.what this removes, its
falseis the only load-bearing answer, and a staleset bit is a false positive.
cargo test --release -p perry-runtime --config 'profile.release.package.perry-runtime.debug-assertions=true' -- --test-threads=1 gc::tests::— 969 passed, 0 failed on the first commit.Known gaps
was at 9 GB against a 12 GB floor, so it is not type-checked and not run. It
needs a debug
cargo test -p perry-runtime --libbefore it is believed.Verdict on the landing base (perrymaster, main
33e2856c5vs main + this branch, 5×3300 + 3×400, quiet box, 2026-09-06 22:40 CEST)Measured flat on main: CPU ±0.07 s at both lengths; peak within +10 MB; settled +29 MB at 3300 on one row. The pre-registered prediction ("flat on a post-#9857 base") holds here; FC2's −9 % was a property of the I2 base, where the restore pass was ~20× dearer. The counter result (99 % of young-mode layout-table visits skipped) stands as measured, and the
closure.dynamic_propsfull-mode revert (commit 2) is correct on its own evidence — but with turn CPU unchanged and a settled-footprint cost, this PR does not land as a performance change. Left as draft; the revert ofclosure.dynamic_props' young walk could be split out if #9897's instrument fix wants it.Raw: perrymaster
/root/rig9831/combPY.jsonl,idlePY.jsonl.Superseded by #9976 (2026-09-08)
Only the layout-prune commit (19a6cd2) survives, replayed patch-identically onto the current tree in #9976 and measured there (LAYOUT + TYPED pruning 3.8–4.5 ms → 0.3 ms per steady cc minor). 390796a is already on main; the closure-log removal (41a8af7 and its two test follow-ups) is dropped because MP measured that log paying (closure_dynamic_props 3.75 → 3.06 ms) on the current tree.
https://claude.ai/code/session_011dhBmdn4vGgNibjo3oZqTo