Skip to content

Commit 4ae02d8

Browse files
Make benchmark CI guard robust against run-to-run noise (#196)
Adopt observ's benchmark strategy. Single benchmark runs on hosted runners are too noisy to gate on: process-level effects (hash randomization, memory layout, runner load) shift individual timings by 20-40% on identical code, so the old single-run 5% mean gate flagged code paths a diff never touched. Instead, interleave 3 runs each of master and PR code with a fixed PYTHONHASHSEED, then compare via bench/compare_runs.py: a regression is only reported when the fastest PR run is slower than the slowest master run by more than the threshold. The 25% threshold sits above the observed run-to-run noise floor, making this a tripwire for gross accidental regressions rather than a precision instrument. Also add a concurrency group to cancel superseded in-flight PR runs. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 5b8018d commit 4ae02d8

3 files changed

Lines changed: 230 additions & 20 deletions

File tree

.github/workflows/benchmark.yml

Lines changed: 54 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,11 @@ on:
55
branches:
66
- master
77

8+
# Cancel in-flight runs when newer commits are pushed to the same PR.
9+
concurrency:
10+
group: ${{ github.workflow }}-${{ github.ref }}
11+
cancel-in-progress: true
12+
813
jobs:
914
benchmark:
1015
name: Benchmarks
@@ -24,29 +29,58 @@ jobs:
2429
- name: Install dependencies
2530
run: uv sync
2631

27-
# On PRs: run benchmarks twice (PR code vs master code) and compare
32+
# On PRs: run benchmarks alternating between master code and PR
33+
# code (3 runs each), then check that the PR is not *consistently*
34+
# slower than master. Single runs are far too noisy to gate on:
35+
# process-level effects (hash randomization, memory layout, runner
36+
# load) shift individual results by 20-40% on identical code.
37+
# Interleaving the runs and comparing the fastest PR run against
38+
# the slowest master run makes the guard robust against that noise
39+
# (see bench/compare_runs.py).
2840
- name: Run benchmarks
41+
env:
42+
# Fixed hash seed: hash randomization moves dict/set benchmark
43+
# timings between processes, which would make the master and
44+
# PR runs incomparable.
45+
PYTHONHASHSEED: "0"
2946
run: |
30-
# Checkout master version of collagraph directory
3147
git fetch origin master
32-
git checkout origin/master -- collagraph/
33-
34-
# Run benchmarks with master code as baseline
35-
uv run --no-sync pytest bench \
36-
--benchmark-only \
37-
--benchmark-save=master \
38-
--benchmark-sort=mean || true
39-
40-
# Restore PR code
41-
git checkout HEAD -- collagraph/
42-
43-
# Run benchmarks on PR code and compare
44-
uv run --no-sync pytest bench \
45-
--benchmark-only \
46-
--benchmark-compare \
47-
--benchmark-compare-fail=mean:5% \
48-
--benchmark-save=branch \
49-
--benchmark-sort=mean
48+
49+
for i in 1 2 3; do
50+
# Run benchmarks with master code as baseline. Tolerate
51+
# failures: the PR's benchmarks may exercise APIs that don't
52+
# exist on master yet.
53+
git checkout origin/master -- collagraph/
54+
uv run --no-sync pytest bench \
55+
--benchmark-only \
56+
--benchmark-save=master$i \
57+
--benchmark-sort=mean || true
58+
59+
# Run benchmarks on PR code
60+
git checkout HEAD -- collagraph/
61+
uv run --no-sync pytest bench \
62+
--benchmark-only \
63+
--benchmark-save=branch$i \
64+
--benchmark-sort=mean
65+
done
66+
67+
# The 25% threshold makes this guard a tripwire for gross
68+
# accidental regressions (an accidental copy in a hot path, an
69+
# algorithmic slip), not a precision instrument. Even with the
70+
# interleaved fastest-vs-slowest gate above, whole runs of
71+
# identical code on hosted runners have been observed to differ
72+
# by 10-30% (the same commit went fail/fail/pass across three
73+
# attempts at a 10% threshold, flagging code paths its diff never
74+
# touched). Gating below that noise floor just breeds re-run
75+
# rituals. Regressions too small to trip this guard should be
76+
# measured deliberately instead: repeated local runs of the bench
77+
# suite on an idle machine.
78+
- name: Compare benchmarks
79+
run: |
80+
uv run --no-sync python bench/compare_runs.py \
81+
--baseline '.benchmarks/*/*_master?.json' \
82+
--branch '.benchmarks/*/*_branch?.json' \
83+
--threshold 0.25
5084
5185
- name: Upload benchmarks
5286
if: always()

bench/compare_runs.py

Lines changed: 175 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,175 @@
1+
"""
2+
Compare two sets of pytest-benchmark runs (baseline vs branch) and fail
3+
when a benchmark has regressed.
4+
5+
Single benchmark runs are too noisy to gate on: process-level effects
6+
(hash randomization, memory layout, CPU frequency, runner load) shift
7+
individual timings by 20-40% between runs of identical code. Instead of
8+
comparing one run against one run, this script expects several
9+
interleaved runs per side and only reports a regression when the branch
10+
is *consistently* slower than the baseline:
11+
12+
fail if min(branch medians) > max(baseline medians) * (1 + threshold)
13+
14+
i.e. the fastest branch run must be slower than the slowest baseline
15+
run by more than the threshold. A whole-process outlier on either side
16+
then can't produce a false positive, because the comparison always uses
17+
the branch's luckiest run against the baseline's unluckiest run.
18+
19+
Even that gate has a noise floor: on shared CI runners, whole runs of
20+
identical code have been observed to differ by 10-30%. The threshold
21+
therefore has to sit above that floor, which makes this a tripwire for
22+
gross accidental regressions rather than a precision instrument --
23+
changes too small to trip it should be measured with deliberate
24+
repeated local runs instead.
25+
26+
Usage:
27+
28+
python bench/compare_runs.py \
29+
--baseline '.benchmarks/*/*_master?.json' \
30+
--branch '.benchmarks/*/*_branch?.json' \
31+
--threshold 0.25
32+
"""
33+
34+
import argparse
35+
import glob
36+
import json
37+
import os
38+
import sys
39+
from statistics import median
40+
41+
42+
def load_runs(pattern):
43+
"""Load benchmark stats from every file matching the glob pattern.
44+
45+
Returns a list of runs, each a dict mapping benchmark name to its
46+
median timing in seconds.
47+
"""
48+
runs = []
49+
for path in sorted(glob.glob(pattern)):
50+
with open(path) as fh:
51+
data = json.load(fh)
52+
runs.append({b["name"]: b["stats"]["median"] for b in data["benchmarks"]})
53+
return runs
54+
55+
56+
def format_time(seconds):
57+
for unit, factor in [("s", 1), ("ms", 1e3), ("us", 1e6), ("ns", 1e9)]:
58+
if seconds * factor >= 1:
59+
return f"{seconds * factor:,.1f}{unit}"
60+
return f"{seconds * 1e9:.2f}ns"
61+
62+
63+
def compare(baseline_runs, branch_runs, threshold):
64+
"""Compare runs and return (rows, failed_names)."""
65+
baseline_names = set().union(*(r.keys() for r in baseline_runs))
66+
branch_names = set().union(*(r.keys() for r in branch_runs))
67+
68+
rows = []
69+
failed = []
70+
for name in sorted(branch_names):
71+
if name not in baseline_names:
72+
rows.append((name, None, None, None, "new"))
73+
continue
74+
base = [r[name] for r in baseline_runs if name in r]
75+
branch = [r[name] for r in branch_runs if name in r]
76+
# Estimated change, for reporting only: middle-of-the-road runs
77+
# on both sides.
78+
change = (median(branch) - median(base)) / median(base)
79+
# Gate: the fastest branch run against the slowest baseline run.
80+
excess = (min(branch) - max(base)) / max(base)
81+
if excess > threshold:
82+
verdict = "FAIL"
83+
failed.append(name)
84+
else:
85+
verdict = ""
86+
rows.append((name, base, branch, change, verdict))
87+
for name in sorted(baseline_names - branch_names):
88+
rows.append((name, None, None, None, "removed"))
89+
return rows, failed
90+
91+
92+
def render_table(rows, markdown=False):
93+
header = ["benchmark", "baseline medians", "branch medians", "change", ""]
94+
body = []
95+
for name, base, branch, change, verdict in rows:
96+
body.append(
97+
[
98+
name,
99+
" / ".join(format_time(t) for t in sorted(base)) if base else "-",
100+
" / ".join(format_time(t) for t in sorted(branch)) if branch else "-",
101+
f"{change:+.1%}" if change is not None else "-",
102+
verdict,
103+
]
104+
)
105+
if markdown:
106+
lines = [
107+
"| " + " | ".join(header) + " |",
108+
"|" + "|".join("---" for _ in header) + "|",
109+
]
110+
lines.extend("| " + " | ".join(row) + " |" for row in body)
111+
return "\n".join(lines)
112+
widths = [max(len(row[i]) for row in [header, *body]) for i in range(len(header))]
113+
lines = [
114+
" ".join(cell.ljust(width) for cell, width in zip(row, widths)).rstrip()
115+
for row in [header, *body]
116+
]
117+
lines.insert(1, "-" * max(len(line) for line in lines))
118+
return "\n".join(lines)
119+
120+
121+
def main(argv=None):
122+
parser = argparse.ArgumentParser(description=__doc__)
123+
parser.add_argument("--baseline", required=True, help="glob for baseline runs")
124+
parser.add_argument("--branch", required=True, help="glob for branch runs")
125+
parser.add_argument(
126+
"--threshold",
127+
type=float,
128+
default=0.25,
129+
help="max allowed consistent slowdown, as a fraction (default: 0.25)",
130+
)
131+
args = parser.parse_args(argv)
132+
133+
baseline_runs = load_runs(args.baseline)
134+
branch_runs = load_runs(args.branch)
135+
136+
if not branch_runs:
137+
print(f"error: no branch runs match {args.branch!r}")
138+
return 2
139+
if not baseline_runs:
140+
# The baseline couldn't run at all (e.g. the branch's benchmarks
141+
# exercise APIs that don't exist on the baseline yet); there is
142+
# nothing to compare against, so pass.
143+
print(f"warning: no baseline runs match {args.baseline!r}, skipping compare")
144+
return 0
145+
146+
rows, failed = compare(baseline_runs, branch_runs, args.threshold)
147+
print(
148+
f"Comparing {len(branch_runs)} branch run(s) against "
149+
f"{len(baseline_runs)} baseline run(s), threshold {args.threshold:.0%}:\n"
150+
)
151+
print(render_table(rows))
152+
153+
summary_path = os.environ.get("GITHUB_STEP_SUMMARY")
154+
if summary_path:
155+
with open(summary_path, "a") as fh:
156+
fh.write("## Benchmark comparison\n\n")
157+
if failed:
158+
fh.write(f"**{len(failed)} benchmark(s) regressed.**\n\n")
159+
fh.write(render_table(rows, markdown=True))
160+
fh.write("\n")
161+
162+
if failed:
163+
print(
164+
f"\n{len(failed)} benchmark(s) consistently slower than baseline "
165+
f"by more than {args.threshold:.0%}:"
166+
)
167+
for name in failed:
168+
print(f" {name}")
169+
return 1
170+
print("\nNo benchmark consistently regressed beyond the threshold.")
171+
return 0
172+
173+
174+
if __name__ == "__main__":
175+
sys.exit(main())

pyproject.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ pyside-dev = [
5757

5858
[tool.ruff.lint.per-file-ignores]
5959
"bench/*" = ["F821", "N806"] # undefined-name, non-lowercase-variable-in-function
60+
"bench/compare_runs.py" = ["T201"] # flake8-print (reporting script)
6061
"collagraph/__init__.py" = ["I001"] # unsorted-imports
6162
"collagraph/__pyinstaller/*" = ["N999"] # invalid-module-name
6263
"collagraph/renderers/**/__init__.py" = ["F401"] # unused-import

0 commit comments

Comments
 (0)