test(act-pg): stop the scenario benches from measuring each other - #1614
Open
Rotorsoft wants to merge 1 commit into
Open
test(act-pg): stop the scenario benches from measuring each other#1614Rotorsoft wants to merge 1 commit into
Rotorsoft wants to merge 1 commit into
Conversation
The notify-vs-polling bench failed on an unrelated PR with `expected 10.79 to be less than 0`. Zero was the polling arm's median over an empty sample set: it recorded none of its 30 reactions, because the scenario benches run as concurrent vitest workers and a 100,000-event CPU-bound bench in the next worker starved its 50ms pump for the whole run. The same contention distorts what the benches report even when they pass — that run measured an idle p95 of 516ms where the same code alone reports 7.6ms. These numbers feed PERFORMANCE.md, so run one bench file at a time. It costs about 30% wall time; the sum of test time is unchanged. An empty sample set also had no business reaching the comparison: the percentile helper maps "no samples" to 0ms, which reads as infinitely fast. Each arm now fails where the cause can still be named, and the poll pump reports how many passes it swallowed instead of hiding them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015xAdM431gFeFBZTW5kRrjg
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.
The notify-vs-polling bench failed CI on an unrelated PR (#1612) with
expected 10.79 to be less than 0. Nothing was wrong with that PR — the bench reported a number it had no business reporting, and this fixes both halves of that.What happened
The bench compares commit→reaction latency two ways: a reader woken by Postgres
LISTEN/NOTIFY, and a reader driven by a plain 50mscorrelate → drainpump. The failing run looked like this:The polling arm recorded none of its 30 reactions.
0.0 msis what the percentile helper returns for an empty sample set, so the comparison read the arm that never ran as infinitely fast and failed on the arm that worked.The scenario benches run as concurrent vitest workers. In that run, a bench doing a 100,000-event per-event drain held a core for 109 seconds while a Postgres latency bench hammered the same database. A 50ms
setIntervalpump gets no turns under that, and every exception it might have raised was swallowed by a barecatch {}.The part that matters more than the flake
The same contention corrupts what these benches report when they pass. That CI run measured an idle p50 of 12.5 ms and p95 of 516 ms, from only 38 samples. Run alone on the same code: 5.6–7.2 ms p50, 7.6–14.9 ms p95, from 144–221 samples. Roughly a 70x distortion in the tail, in numbers that feed
PERFORMANCE.md.So the fix is to run one bench file at a time (
fileParallelism: false). Measured cost: 129s → 168s wall, about 30%. The sum of test time is unchanged at ~166s — parallelism was only overlapping it, and overlapping is exactly what invalidates a wall-clock latency measurement. Slow numbers beat contaminated ones when the output is a published benchmark.And make the silent failure loud
Serializing removes the cause that has been observed; it does not make an empty sample set legible if some other cause produces one. Each arm now fails at the point where the cause can still be named, and the pump counts what it swallows instead of hiding it. Both messages were verified by simulation:
The pump stays best-effort — the pool genuinely can close mid-flight at teardown — it just no longer loses the evidence.
Cost
Bench job wall time grows about 30%. No framework code changes; nothing here ships in a package.
Test plan
pnpm bench:scenarios— 10 passed, serialized, 168spnpm test— 238 files, 3631 passed; coverage 100% statements / 100% branches / 100% functions / 100% linespnpm typecheckpnpm lintStability charter impact
None. A bench file and the bench-only vitest config; no package source, no public surface.
rfc-gate: exempt — no
libs/*/srcfile is touched, so the stability snapshot is unchanged.🤖 Generated with Claude Code
https://claude.ai/code/session_015xAdM431gFeFBZTW5kRrjg