Skip to content

PR benchmark check flags unrelated benchmarks as regressions #144

Description

@lukashes

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:

  • PgOutputDecoder 52.42μs → 58.41μs, +11.4% 🔴
  • KafkaProducer produce 296.39μs → 317.17μs, +7.0% 🔴
  • Converter allocations 13 → 14 (INSERT/DELETE), 22 → 23 (UPDATE)

Likely causes

  • 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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingsize/SRelative effort: small

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions