perf(gc): negative-cache the page classes the map does not hold — reach is 52.8-62.1% of remaining misses, not #9852's 90.7% - #9878
Conversation
`gc_check_trigger` runs on every `gc_malloc`, and `gc_budgeted_due_trigger` resolved eleven raw `thread_local!` declarations one `_tlv_get_addr` call at a time. Measured with `sample` on the compiled claude-code TUI streaming a 3300-char reply (14,578 active main-thread samples, callers resolved by an explicit ancestor walk): `_tlv_get_addr` was 380 main-thread leaf samples, 71 of them with `gc_budgeted_due_trigger` as the immediate caller, 36 in `old_page_account_dirty_slots`, 31 in `scan_dirty_object_slots`, 27 in `gc_malloc_header_is_tracked`. Sixty-seven declarations move to `crate::perry_thread_local!`. Why they were still cold is a measurement bug in the gate, not an oversight: `scripts/check_thread_locals.py` ratchets on raw `thread_local!` BLOCKS per file, and a block holds any number of declarations — so `gc/policy.rs` counted as 6 while declaring 28, and adding a `static` to a recorded block passed silently. In the same unit as the hot side, main was 318 hot against 339 cold declarations. The gate now ratchets on declarations (385/272) and `--self-test` gained the direction that catches it. `ARENA_TOTAL_BYTES`, `BLOCK_POOL` and `BLOCK_POOL_BYTES` stay raw and say so: they are read from `Arena::new`, which runs as `tls_hot::fill`'s first provider, so a `HotKey` there re-enters `fill` — which has not yet written the `temp_roots` field it gates on — and re-runs `ARENA`'s initializer without bound. Claude-Session: https://claude.ai/code/session_014UZWia6L37DpA93VLtNK9m
…TABLE
WIP — committed to preserve state while the lane is paused for box load
(load 298, 19.8/21.5 GB swap). NOT measured on the rig; do not land as is.
Replaces the 4-way round-robin page-generation cache with a direct-indexed
table over the arena's 1 MiB address classes. `PageGenerationMap` stays
authoritative: every miss falls through to it exactly as before, so this is a
cache replacement, not a map replacement. The 4-way set is retained in the same
binary behind `PERRY_GC_PAGE_CLASS_TABLE=0` as the positive control.
STATE OF THIS COMMIT
Applied, complete:
* the table itself (`lookup`/`insert`/`rebase_to_cover`/`invalidate`), base
taken from the first insert, epoch-stamped entries, O(1) whole-table
invalidation;
* sizing: `INITIAL_SPAN = 4096`, `BASE_SLACK = SPAN / 2`. The draft's
`S = 1024` was mis-tuned — with base `first_key - S` the span covered is
`min(S + 1, N - S)`, maximised at `S = N / 2`, so `S = 1024` covered 1,025
classes against a measured span of 1,021 while `S = N / 2` covers 2,048 for
the same 160 KB. A `const` assert now fails the build for any pairing
covering less than twice the measured span; the old pairing fails it;
* out-of-span coverage: an insert outside the table rebases it up to a
16,384-class cap, and past the cap the key is left uncached and falls
through to the map — never silently mis-indexed;
* the arm is a plain `u8` field in the set's first cache line, not the
`OnceLock` env read the draft had on the lookup path. That path runs ~440 M
times per turn and an acquire load on each would have been paid by BOTH
arms of the A/B while still being charged against main;
* `#[repr(C, align(64))]` so "the hot fields share one cache line" is true
rather than likely;
* counters (`hits`/`misses`/`inserts`/`oos`/`rebases`/`refused`) and the
`[gc-page-class]` line, emitted per copying minor under `PERRY_GC_DIAG`
because the rig SIGKILLs the process. `oos` is on the miss path only and is
what distinguishes a residual miss that is an unregistered address from one
that is the table failing.
Verified:
* all four tests pass on the pristine tree (`cargo test -p perry-runtime
--lib page_class_table`, dev profile, 4 passed);
* the four sabotages each fail on their own named assertion — base-from-first-
registration, out-of-span handling, range containment, and invalidation.
Two of them produce a literal misclassified pointer (`left: Old, right:
Nursery` and `left: Nursery, right: Old`), which is the failure mode this
structure has to be proof against;
* every `PAGE_GENERATIONS` mutation site was enumerated (three, plus one
read-only census walk) and each ends with an unconditional
`invalidate_generation_cache()`. The table holds ~2,000 entries where the
4-way set held 4, so a missing invalidation the old structure survived by
luck would be a live misclassification here.
NOT done — this is what the lane owes:
* the rig. The relink was killed mid-`cargo build` at the coordinator's
pause, so there is no candidate binary and NO number in this commit has
been measured on cc;
* `cargo test -p perry-runtime --release -- gc:: arena::`;
* `cargo fmt` (the `arena/mod.rs` re-export is not in sorted order) and
clippy;
* a changelog fragment.
Pre-registered falsifiers, written before any measurement, are in
`secret-tests/cc-perf-campaign/RESULT_page_class_table.md`. The headline is
that the spec's "miss rate below 2 %" bar is arithmetically unreachable: 22.3 %
of today's misses are on addresses in no registered block, which the map cannot
answer either, so nothing is cached for them in either arm. The derived floor
is ~4.5 %, and the decision turns on misses to REGISTERED classes going to ~0.
…xport Formatting and documentation only; no behaviour change. `cargo fmt` on the touched files, restricted to the lines this branch added. Note for whoever runs the fmt gate: `arena/mod.rs` is ALREADY not rustfmt-clean on main at an unrelated `#[cfg(test)]` re-export, and reformatting it would have put that pre-existing churn in this diff, so it is deliberately left alone.
After PerryTS#9853's direct-indexed table took classification misses from 18.5% to ~6% of lookups, what remains is dominated by addresses the authoritative PageGenerationMap cannot answer either: conservative-scan candidates, interior pointers and non-heap words that every classification path re-asks about millions of times per turn. Nothing was ever cached for them, so each one paid a full failing hash lookup. COUNTER FIRST, and it corrects the issue it closes. PerryTS#9852 states that the "unregistered" miss population is 90.7% of every remaining miss and that all of it is negative-cacheable. Only the first half is close. The uncached population collapses two cases that `pages.get(&key).and_then(|s| s.find(addr))` had never separated, so this change adds the two counters that split them before it adds the cache: * pages.get(&key) is None -- no registered range covers this 1 MiB class. Sound to remember at class granularity. * the class EXISTS but no range in it contains addr. Remembering the class as absent would answer wrongly for the addresses it really covers. Measured on the compiled claude-code TUI, table arm, three 3300-char runs and two at 400 (neg_absent + neg_present == miss_unregistered exactly in all five): 3300 absent/miss 52.8 / 55.6 / 62.1 % present/miss 27.4 / 16.3 / 21.4 % 400 absent/miss 41.5 / 66.2 % present/miss 28.7 / 15.9 % So the reach is 52.8-62.1% of remaining misses at 3300, not 90.7%: 19-34% of the population PerryTS#9852 counted is the unsound case. The share is quoted as a range because it varies that much between runs of one binary. MECHANISM. A negative is an ordinary table entry carrying the current epoch, with the sentinel range base=usize::MAX end=0 -- contains() is false for every address, so the hit arm is byte-for-byte what it was and the marker test is reached only after contains() has already failed, i.e. on the miss path. lookup() becomes a tri-state; KnownAbsent returns exactly what the fall-through would have returned (HeapGeneration::Unknown / None) without asking the map. INVALIDATION NEEDS NO NEW RULE. All three PageGenerationMap mutation sites already end with invalidate_generation_cache(), which bumps the epoch. A registration into this class is precisely the event that would make a negative wrong, and precisely the event that kills it. A stale negative is the worst answer this structure can give -- the collector declines to trace a live young object -- so that equivalence is the whole correctness argument. Two further preconditions, each one a way this could be WRONG rather than merely slow: the table arm only (the 4-way set has no negative concept and its lookup does not test for one), and never the insert that allocates the table (the base must come from a REGISTERED key; an unregistered candidate address can be anywhere at all). PERRY_GC_PAGE_CLASS_NEGATIVE=0 restores the previous behaviour in the same binary, mirroring PERRY_GC_PAGE_CLASS_TABLE, so the A/B has no build difference to confound it. Four tests, each with its sabotage named at the test, including the one whose sabotage is caching the negative unconditionally. The first asserts on neg_hits rather than on the returned value, because the value is Unknown whether or not the negative was consulted -- a test that checked only the answer would pass with the feature deleted. cargo test --release -p perry-runtime --lib -- gc:: arena:: --test-threads=1: 1,142 passed, 0 failed. Refs PerryTS#9852. Claude-Session: https://claude.ai/code/session_014knX724SYDogwzsXybCGxp
|
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 |
jdalton
left a comment
There was a problem hiding this comment.
Review of a529223465ef8c0db72219ee91e1e14c6f8570f2 (2026-09-07).
The negative cache correctly distinguishes a wholly absent class from an address gap inside a registered class. Please retain a regression that first caches absence, then registers a range in that same class and immediately classifies it, plus unregister/re-register and multiple-range gap cases. The independent PERRY_GC_PAGE_CLASS_NEGATIVE=0 switch also needs an exercised OFF arm per CLAUDE.md's GC-knob policy, rather than only a manual campaign comparison. Separately, the declaration-count TLS ratchet is a useful correction; please keep that self-test when rebasing the page-meta split onto current main.
Validation scope: source/diff inspection; I have not run this PR's build or test suite locally.
The counter came first, and it corrects the issue this closes
#9852 says the "unregistered" miss population is 90.7 % of every remaining
miss after #9853's table, and that all of it is negative-cacheable. The first
half is close; the second is wrong, and nothing had measured it, because
pages.get(&key).and_then(|slot| slot.find(addr))collapses two cases into oneNone:pages.get(&key)is itselfNone— no registered range covers this 1 MiBclass. Sound to remember at class granularity.
addr— remembering theclass as absent would answer wrongly for the addresses it really covers.
So this change adds the two counters that split them before it adds the
cache. Measured on the compiled claude-code TUI, table arm, three 3300-char
runs and two at 400.
neg_absent + neg_present == miss_unregisteredexactly inall five, and
registered + unregistered == missesexactly, so the split iscomplete by construction:
Reach is 52.8–62.1 % of remaining misses at 3300 and 41.5–66.2 % at 400 — not
90.7 %. Of the population #9852 counted as one thing, only 65.9–80.6 % is
a class the map does not hold; the other 19–34 % is the unsound case. The
share is quoted as a range, not a point, because it varies that much between
runs of one binary — two runs before a number is quoted, three before a target
is built on it.
Why the unsound case is common rather than a corner: a registered block is
BLOCK_SIZE=1 << GENERATION_CLASS_SHIFT, so one block is one class onlywhen its base is class-aligned. A block at an unaligned base covers the tail
of one class and the head of the next, and every address in the uncovered
halves lands here.
What the change is
A negative is an ordinary table entry carrying the current epoch, with the
sentinel range
base = usize::MAX, end = 0.contains()is false for everyaddress, so:
after
contains()has already failed, i.e. on the miss path, never on a hit;lookupbecomes a tri-state, andKnownAbsentreturns exactly what thefall-through would have returned (
HeapGeneration::Unknownfor the generationclassifier,
Noneforclassify_heap_space_in_range) without asking the map.Invalidation needs no new rule — and that is the whole correctness argument
All three
PageGenerationMapmutation sites already end withinvalidate_generation_cache(), which bumps the epoch and makes every entry —positive and negative alike — stale at once. A registration into this class is
precisely the event that would make a negative wrong, and precisely the event
that kills it. A stale negative is the worst answer this structure can give:
the collector is told "not a GC page" about a live young object and declines to
trace it. The equivalence above is what makes that unreachable, and
a_negative_entry_is_killed_by_a_later_registration_in_that_classis thestanding guard.
Two further preconditions, each a way this could be wrong rather than merely
slow:
lookupdoes not test for one;first key stored, and only a registered key is guaranteed to sit inside the
arena's eventual span. An unregistered candidate address can be anywhere.
PERRY_GC_PAGE_CLASS_NEGATIVE=0restores the previous behaviour in the samebinary, mirroring
PERRY_GC_PAGE_CLASS_TABLE, so an A/B has no builddifference to confound it.
Tests, with the sabotage named at each
cargo test --release -p perry-runtime --lib -- gc:: arena:: --test-threads=1— 1,142 passed, 0 failed, all eight page-class tests included.
a_class_with_no_registered_range_is_answered_from_the_negative_entryinsert_negativecall — the second classification misses againa_partially_registered_class_is_never_remembered_as_absentinsert_negativeunconditionally instead of underclass_absenta_negative_entry_is_killed_by_a_later_registration_in_that_classself.epocha_negative_never_fixes_the_tables_basetable.is_empty()guard ininsert_negativeThe first asserts on the
neg_hitscounter, not on the returned value: thevalue is
Unknownwhether or not the negative was consulted, so a test thatchecked only the answer would pass with the whole feature deleted.
What this is worth, stated before anyone measures it
Small, and this PR does not promise a rig delta. After #9853 the two pure-miss
leaf symbols are 0.59 % of active main-thread samples, so removing ~53–62 %
of what is left bounds the win at roughly 0.3 % of the turn — under this
campaign's measured n=1 noise floor. The ground this stands on is work
permanently removed — ~13.9 M failing hash lookups per 3300-char reply — proved
by a counter, not by a stopwatch. Anyone reading a flat rig table here is
reading the expected outcome.
Counters are load-independent, so the table above is quotable; no CPU or memory
number is claimed at all.
Closes #9852 as measured, with its 90.7 % headline corrected to 52.8–62.1 %.
https://claude.ai/code/session_014knX724SYDogwzsXybCGxp