perf(tests): stabilize subscribe_user_trio so the gate stops flipping - #85
Closed
joeblau wants to merge 1 commit into
Closed
perf(tests): stabilize subscribe_user_trio so the gate stops flipping#85joeblau wants to merge 1 commit into
joeblau wants to merge 1 commit into
Conversation
`subscription/subscribe_user_trio` produced coin-flip regression verdicts on unrelated PRs. On #84 — which changes only two files under src/signing — it reported +39.6% with a band of [-29.7%, +177.2%], a band spanning zero because the three rounds disagreed in sign. The scenario built one transport per sample and awaited three subscriptions, so a sample measured async scheduling and first-transport warmup rather than the cost of subscribing. Every neighbouring scenario was stable across the same measurement positions, so this was the scenario's own instability, not runner noise. Two changes: - The scenario runs 20 iterations per sample instead of 1, so per-sample warmup and scheduling jitter amortize across 60 subscriptions rather than landing on 3. `unitsPerIteration` still normalizes to a per-subscription figure, so the reported unit is unchanged — it now reports the steady-state cost instead of a number dominated by the first transport in each sample. - `MockWebSocket` keeps only the most recently constructed instance rather than pushing every one into a static array. `lastMockWebSocket` only ever read `.at(-1)`, but the array kept every socket — and through the listeners on it, every transport and its keep-alive timers — reachable until the scenario ended. That mattered little at one transport per sample and matters a great deal at twenty. Measured over three full-suite runs on one machine: before 14.68 µs (rme 112.0%), 11.00 µs (9.6%), 13.22 µs (12.7%) after 9.90 µs (rme 6.6%), 9.61 µs (5.6%), 9.57 µs (6.8%) Run-to-run spread falls from 33% to 3.4% and worst-case rme from 112% to 6.8%. Baseline rme feeds the comparator's tolerance, so the tighter figure also stops this scenario inflating the gate's noise allowance for every other scenario in the report. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Author
|
Superseded by #86, which absorbed this branch's work: |
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
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.
subscription/subscribe_user_triohas been producing coin-flip regression verdicts on unrelated PRs. On #84 — which changes only two files undersrc/signing— it reported +39.6% with a band of[-29.7%, +177.2%], a band spanning zero because the three rounds disagreed in sign.Why it was unstable
It built one transport per sample and awaited three subscriptions, so a sample measured async scheduling and first-transport warmup rather than the cost of subscribing. Every neighbouring scenario was stable across the same measurement positions, which is what rules out runner noise:
l2book_dispatch_50_coinssubscribe_200_coinshttp_requestsubscribe_user_trioWhat changed
20 iterations per sample instead of 1. Per-sample warmup and scheduling jitter now amortize across 60 subscriptions rather than landing on 3.
unitsPerIterationstill normalizes to a per-subscription figure, so the reported unit is unchanged — it now reports the steady-state cost instead of a number dominated by the first transport in each sample.MockWebSocketkeeps only the most recent instance instead of pushing every one into a static array.lastMockWebSocketonly ever read.at(-1), but the array kept every socket — and through the listeners on it, every transport and its keep-alive timers — reachable until the scenario ended. That mattered little at one transport per sample and matters a great deal at twenty.Result
Three full-suite runs on one machine:
Run-to-run spread falls from 33% to 3.4% and worst-case rme from 112% to 6.8%. Since baseline rme feeds the comparator's tolerance, the tighter figure also stops this scenario inflating the gate's noise allowance for every other scenario in the report.
I tried 50 iterations too: it tightens the median further but pushes rme back to 16–26% from GC pressure at 50 transports per sample, so 20 is the better trade.
Expected: the
perfcheck will be redThis edits
tests/perf, so the suite fingerprint changes and the gate fails closed by design — "Land an intentional suite change with the gate red, or as its own PR." This is that own PR. The absolute figure moves too (~13 µs → ~9.6 µs locally), which is expected: it no longer bills first-transport warmup to every sample.bun run checkexits 0; 1652 pass, 236 skip, 0 fail.🤖 Generated with Claude Code