You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The PR benchmark comment regularly reports "🔴 slower" and shifting allocation counts for benchmarks the PR does not touch. The check is informational (it never fails the build), but the noise makes it useless as a signal, and it fires on almost every PR.
Timing noise. Benchmarks run on ubuntu-latest. Comparing PR head and base on the same runner cancels hardware differences between runs but not intra-run jitter (noisy neighbors, CPU frequency scaling). min over 3 passes does not stabilize the heavy benchmarks, and the ≥50μs threshold is 5%, tighter than the run-to-run swing, so the decoder and producer trip red on noise alone.
Baseline is the base branch tip, not the merge-base.benchmarks.yml compares PR head against pull_request.base.sha. When the PR is behind main, the delta includes commits merged into main that the PR does not have, so unrelated changes get charged to the PR. The Converter alloc 13 → 14 on Capture the slot's consistent point and split connect from streaming #143 is exactly this: the branch predates a main change and has not rebased.
Expected vs actual
Expected: a 🔴 means this PR's changes regressed that benchmark.
Actual: unrelated benchmarks flip red on most PRs, and deterministic alloc counts drift from base-branch movement.
Possible fixes
Compare against the merge-base of the PR and its base, not the base tip.
Loosen the heavy-op threshold, or require the delta to exceed measured run-to-run variance before flagging.
Raise BENCH_RUNS and/or add a warmup pass, and report deterministic alloc changes separately from noisy timing.
Ask: make the benchmark comment trustworthy, so a 🔴 only appears when this PR regressed that benchmark.
The PR benchmark comment regularly reports "🔴 slower" and shifting allocation counts for benchmarks the PR does not touch. The check is informational (it never fails the build), but the noise makes it useless as a signal, and it fires on almost every PR.
Evidence (PR #143)
#143 changes only
replication_protocol.zig,source.zig,main.zig, and tests. None of the benchmarked hot paths. The comment still reported:PgOutputDecoder52.42μs → 58.41μs, +11.4% 🔴KafkaProducer produce296.39μs → 317.17μs, +7.0% 🔴Converterallocations 13 → 14 (INSERT/DELETE), 22 → 23 (UPDATE)Likely causes
ubuntu-latest. Comparing PR head and base on the same runner cancels hardware differences between runs but not intra-run jitter (noisy neighbors, CPU frequency scaling).min over 3 passesdoes not stabilize the heavy benchmarks, and the ≥50μs threshold is 5%, tighter than the run-to-run swing, so the decoder and producer trip red on noise alone.benchmarks.ymlcompares PR head againstpull_request.base.sha. When the PR is behindmain, the delta includes commits merged intomainthat the PR does not have, so unrelated changes get charged to the PR. TheConverteralloc 13 → 14 on Capture the slot's consistent point and split connect from streaming #143 is exactly this: the branch predates amainchange and has not rebased.Expected vs actual
Possible fixes
BENCH_RUNSand/or add a warmup pass, and report deterministic alloc changes separately from noisy timing.Ask: make the benchmark comment trustworthy, so a 🔴 only appears when this PR regressed that benchmark.