perf(runtime): drop the saturated buffer address filter (-2.46% command CPU) - #10108
perf(runtime): drop the saturated buffer address filter (-2.46% command CPU)#10108proggeramlug wants to merge 1 commit into
Conversation
BUFFER_LIKE_ADDR_FILTER ends every measured claude-code run with all 1,024 bits set and rejects nothing, while costing three hash rounds and up to three dependent loads on every one of ~26.6 M admitted probes per run. Its adoption note asked the capacity question and answered it from a 400-character reply: 213 cumulative registrations, live_max 201, true positives 0.207% of admits, predicting a 10.0% false-positive rate. An ordinary command (startup, two real Read calls, streamed reply) falsifies both premises. The population is 15x larger - 3,232 cumulative admissions and 1,618 live against 1,024 bits - so the filter saturates, and every rejection comes from the window in front of it at 15.51%, not the 25.94% the note quotes. And 88.87% of admitted probes find a real registered buffer, against 0.207% before, so a filter cannot remove work the registry genuinely has to do. Six interleaved pairs, one binary and one environment variable: minimum command CPU 1.22 -> 1.19 s (-2.46%), medians 1.28 -> 1.20 s, paired median -3.60%, faster in five pairs and tied in the sixth, peak RSS maxima 664.5 -> 635.2 MiB. Node anchor in the same session 0.41/0.42 s. The min/max window stays and keeps doing all of the rejecting, including the debug-build machine-check that re-derives every rejection from the authoritative tables. Deleting a negative accelerator cannot produce a wrong answer.
|
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 (1)
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughThe buffer registry removes ChangesBuffer admission simplification
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Refactor Merge Risk: ⚪ Minimal · up to The admission simplification preserves exact buffer identification and introduces no unresolved merge-blocking risk. 🚥 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 |
The PR changed crates/ without one, which the changeset gate requires. Body summarised from the commit message's own measurements.
|
Landed on Your commits are on One maintainer follow-up: the PR changed Closing this PR as landed — GitHub cannot auto-close it because the train merges as its own branch. |
1 participant
Summary
BUFFER_LIKE_ADDR_FILTERis saturated on any realisticclaude-codeworkload — it ends every measured run with all 1,024 bits set and rejects nothing, while costing three hash rounds and up to three dependent loads on every one of ~26.6 M admitted probes per run. Removing it is measurably faster. The min/max window in front of it stays and keeps doing all of the rejecting.Minimum command CPU 1.22 → 1.19 s (−2.46 %), paired median −3.60 %, faster in five of six interleaved pairs and tied in the sixth, peak RSS no worse.
Why the filter's own adoption note no longer holds
The note asked the capacity question and answered it from a 400-character reply: 213 cumulative registrations,
live_max201,true_positives=53,109 (0.207 % of admits)→ a predicted 10.0 % false-positive rate, "the filter rejects about nine of every ten addresses the window admits".Both premises fail on an ordinary command (startup, two real
Readtool calls, streamed reply).PERRY_BUFFER_DIAG— the instrument that already ships beside the filter — two rows:BUFFER_LIKE_ADDR_WINDOW, at 15.51 % — not the 25.94 % the note quotes.--help-shaped workload the note measured. A filter cannot remove work the registry genuinely has to do; even a correctly sized one could only have taken the ~2.96 M false positives per row off the slow path.So the structure is pure overhead on this workload, and sizing it up would not have helped much either.
Measurement
Six interleaved pairs plus two node rows on the quiet Linux box, campaign lock held, one binary and one environment variable — the filter's own env-var arm against the default — so the arms are the same bytes and the same compiler.
Minima 1.22 → 1.19 s (−2.46 %), medians 1.28 → 1.20 s, paired median −3.60 %. Peak RSS maxima 664.5 → 635.2 MiB. Node anchor in the same session: command CPU 0.41 / 0.42 s, peak RSS 335.5 MiB. CPU accounting has hundredth-second granularity, so six pairs support a positive result of this size rather than a precise universal figure, and this is one workload on one host.
Risk
Deleting a negative accelerator can only cost the lookup that was already there; it cannot produce a wrong answer, and the release build's authoritative path is untouched. The debug-build machine-check that re-derives every rejection from the authoritative tables is unchanged, so a registration route added without admitting to the window still panics in the first test that exercises it.
perry-runtimebuffer tests pass unchanged. The change is confined tocrates/perry-runtime/src/buffer/header.rs: the static, the env-var accessor, twoadmitcalls and one term in the probe, with the measured rationale left in place of the old note.Context for the structure itself: #10109 (two of the four instances of this filter type had stopped discriminating, and nothing in a shipping build can tell). The sibling owner where the remedy is the opposite one — size it up rather than delete it — is #10110.
Summary by CodeRabbit