Skip to content

Commit c27e688

Browse files
mrecachinasmrecachinas
authored
ci: Add PR benchmark comparison workflow (#38)
* ci: add PR benchmark comparison workflow * fix: improve benchmark comparison reporting - Shorten benchmark names for readability (hd_ prefix, etc.) - Use coefficient of variation to detect noise vs real regressions - Raise regression threshold to 30% AND must exceed 2x noise level - Remove truncation that was hiding important test parameters * fix: only show significant benchmark changes - Hide 'within noise' benchmarks in collapsible details section - Show regressions first, then improvements - Cleaner summary when all benchmarks are within noise * fix: add absolute threshold for fast benchmarks For benchmarks < 1µs, ignore percentage changes since the absolute difference is too small to matter. This prevents false positives from noise on very fast operations.
1 parent 4df2218 commit c27e688

1 file changed

Lines changed: 306 additions & 0 deletions

File tree

‎.github/workflows/benchmark.yml‎

Lines changed: 306 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,306 @@
1+
name: Benchmark PR
2+
3+
on:
4+
pull_request:
5+
branches: [master]
6+
7+
permissions:
8+
contents: read
9+
pull-requests: write
10+
11+
jobs:
12+
benchmark-base:
13+
name: Benchmark base branch
14+
runs-on: ubuntu-latest
15+
steps:
16+
- name: Checkout base branch
17+
uses: actions/checkout@v4
18+
with:
19+
ref: ${{ github.event.pull_request.base.sha }}
20+
21+
- name: Set up Python 3.11
22+
uses: actions/setup-python@v5
23+
with:
24+
python-version: "3.11"
25+
26+
- name: Install dependencies
27+
run: |
28+
python -m pip install --upgrade pip
29+
pip install -r requirements-dev.txt
30+
pip install .
31+
32+
- name: Run benchmarks
33+
run: |
34+
pytest test/ -k bench --benchmark-only --benchmark-json=results.json --benchmark-disable-gc
35+
36+
- name: Upload benchmark results
37+
uses: actions/upload-artifact@v4
38+
with:
39+
name: benchmark-base
40+
path: results.json
41+
retention-days: 1
42+
43+
benchmark-pr:
44+
name: Benchmark PR branch
45+
runs-on: ubuntu-latest
46+
steps:
47+
- name: Checkout PR branch
48+
uses: actions/checkout@v4
49+
50+
- name: Set up Python 3.11
51+
uses: actions/setup-python@v5
52+
with:
53+
python-version: "3.11"
54+
55+
- name: Install dependencies
56+
run: |
57+
python -m pip install --upgrade pip
58+
pip install -r requirements-dev.txt
59+
pip install .
60+
61+
- name: Run benchmarks
62+
run: |
63+
pytest test/ -k bench --benchmark-only --benchmark-json=results.json --benchmark-disable-gc
64+
65+
- name: Upload benchmark results
66+
uses: actions/upload-artifact@v4
67+
with:
68+
name: benchmark-pr
69+
path: results.json
70+
retention-days: 1
71+
72+
compare:
73+
name: Compare benchmarks
74+
needs: [benchmark-base, benchmark-pr]
75+
runs-on: ubuntu-latest
76+
steps:
77+
- name: Download base benchmark results
78+
uses: actions/download-artifact@v4
79+
with:
80+
name: benchmark-base
81+
path: base
82+
83+
- name: Download PR benchmark results
84+
uses: actions/download-artifact@v4
85+
with:
86+
name: benchmark-pr
87+
path: pr
88+
89+
- name: Set up Python 3.11
90+
uses: actions/setup-python@v5
91+
with:
92+
python-version: "3.11"
93+
94+
- name: Compare benchmarks and generate report
95+
id: compare
96+
run: |
97+
python3 << 'EOF'
98+
import json
99+
import os
100+
import sys
101+
102+
# Load benchmark results
103+
with open('base/results.json', 'r') as f:
104+
base_results = json.load(f)
105+
106+
with open('pr/results.json', 'r') as f:
107+
pr_results = json.load(f)
108+
109+
# Extract benchmark data into dictionaries
110+
def extract_benchmarks(data):
111+
benchmarks = {}
112+
for bench in data.get('benchmarks', []):
113+
name = bench['name']
114+
# Use median as the primary metric (more stable than mean)
115+
stats = bench['stats']
116+
benchmarks[name] = {
117+
'mean': stats['mean'] * 1000, # Convert to ms
118+
'median': stats['median'] * 1000,
119+
'stddev': stats['stddev'] * 1000,
120+
'rounds': stats['rounds'],
121+
}
122+
return benchmarks
123+
124+
base_benchmarks = extract_benchmarks(base_results)
125+
pr_benchmarks = extract_benchmarks(pr_results)
126+
127+
# Shorten benchmark names by removing common prefixes
128+
def shorten_name(name):
129+
# Remove test_ prefix and _bench suffix
130+
name = name.replace('test_', '').replace('_bench', '')
131+
# Shorten common function names
132+
name = name.replace('hamming_distance_', 'hd_')
133+
name = name.replace('check_hexstrings_within_dist', 'hex_within_dist')
134+
name = name.replace('check_bytes_arrays_within_dist', 'bytes_arr_within')
135+
return name
136+
137+
# Compare benchmarks
138+
results = []
139+
severe_regressions = []
140+
141+
all_names = set(base_benchmarks.keys()) | set(pr_benchmarks.keys())
142+
143+
for name in sorted(all_names):
144+
base = base_benchmarks.get(name)
145+
pr = pr_benchmarks.get(name)
146+
short_name = shorten_name(name)
147+
148+
if base is None:
149+
results.append({
150+
'name': short_name,
151+
'base_time': 'N/A',
152+
'pr_time': f"{pr['median']:.4f}ms",
153+
'delta': 'NEW',
154+
'status': '🆕',
155+
'pct_change': 0,
156+
})
157+
continue
158+
159+
if pr is None:
160+
results.append({
161+
'name': short_name,
162+
'base_time': f"{base['median']:.4f}ms",
163+
'pr_time': 'N/A',
164+
'delta': 'REMOVED',
165+
'status': '🗑️',
166+
'pct_change': 0,
167+
})
168+
continue
169+
170+
# Calculate percentage change (negative = faster, positive = slower)
171+
pct_change = ((pr['median'] - base['median']) / base['median']) * 100
172+
abs_change_ms = abs(pr['median'] - base['median'])
173+
174+
# Check if the change is statistically significant
175+
# Use coefficient of variation to assess noise level
176+
base_cv = (base['stddev'] / base['median']) * 100 if base['median'] > 0 else 0
177+
pr_cv = (pr['stddev'] / pr['median']) * 100 if pr['median'] > 0 else 0
178+
noise_threshold = max(base_cv, pr_cv, 10) # At least 10% noise floor
179+
180+
# For very fast benchmarks (<0.001ms = 1µs), require minimum absolute change
181+
# to avoid flagging noise as regressions
182+
min_abs_change_ms = 0.001 # 1µs minimum meaningful change
183+
is_too_small_to_matter = abs_change_ms < min_abs_change_ms
184+
185+
# Determine status - account for noise in thresholds
186+
if is_too_small_to_matter:
187+
status = '➖' # Too small to matter
188+
elif pct_change < -max(5, noise_threshold):
189+
status = '✅' # Faster beyond noise
190+
elif abs(pct_change) <= max(5, noise_threshold):
191+
status = '➖' # Within noise
192+
elif pct_change <= 30:
193+
status = '⚠️' # Possibly slower (5-30%)
194+
else:
195+
status = '❌' # Severe regression (>30%)
196+
# Only flag as severe if change exceeds 2x the noise level AND absolute threshold
197+
if pct_change > 2 * noise_threshold and not is_too_small_to_matter:
198+
severe_regressions.append(short_name)
199+
200+
# Format delta string
201+
if pct_change < 0:
202+
delta = f"{abs(pct_change):.1f}% faster"
203+
elif pct_change > 0:
204+
delta = f"{pct_change:.1f}% slower"
205+
else:
206+
delta = "no change"
207+
208+
results.append({
209+
'name': short_name,
210+
'base_time': f"{base['median']:.4f}ms",
211+
'pr_time': f"{pr['median']:.4f}ms",
212+
'delta': delta,
213+
'status': status,
214+
'pct_change': pct_change,
215+
})
216+
217+
has_severe_regression = len(severe_regressions) > 0
218+
219+
# Separate significant changes from noise
220+
significant = [r for r in results if r['status'] != '➖']
221+
within_noise = [r for r in results if r['status'] == '➖']
222+
223+
# Generate markdown report
224+
report_lines = ["## 📊 Benchmark Comparison Results", ""]
225+
226+
if not significant:
227+
report_lines.append("### ✅ All benchmarks within noise")
228+
report_lines.append("")
229+
report_lines.append(f"*{len(within_noise)} benchmarks compared, no significant changes detected.*")
230+
else:
231+
report_lines.extend([
232+
"| Status | Benchmark | Base | PR | Delta |",
233+
"|:------:|:----------|-----:|---:|:------|",
234+
])
235+
236+
# Sort: regressions first (❌, ⚠️), then improvements (✅), then new/removed
237+
status_order = {'❌': 0, '⚠️': 1, '✅': 2, '🆕': 3, '🗑️': 4}
238+
significant.sort(key=lambda r: (status_order.get(r['status'], 5), -abs(r['pct_change'])))
239+
240+
for r in significant:
241+
report_lines.append(
242+
f"| {r['status']} | `{r['name']}` | {r['base_time']} | {r['pr_time']} | {r['delta']} |"
243+
)
244+
245+
report_lines.extend([
246+
"",
247+
"<details>",
248+
f"<summary>➖ {len(within_noise)} benchmarks within noise (click to expand)</summary>",
249+
"",
250+
"| Benchmark | Base | PR | Delta |",
251+
"|:----------|-----:|---:|:------|",
252+
])
253+
for r in within_noise:
254+
report_lines.append(
255+
f"| `{r['name']}` | {r['base_time']} | {r['pr_time']} | {r['delta']} |"
256+
)
257+
report_lines.extend(["", "</details>"])
258+
259+
report_lines.extend([
260+
"",
261+
"**Legend:** ✅ Faster (>5%) · ⚠️ Slower (5-30%) · ❌ Regression (>30%) · 🆕 New · 🗑️ Removed",
262+
])
263+
264+
if has_severe_regression:
265+
report_lines.extend([
266+
"",
267+
"---",
268+
"⛔ **This PR has severe performance regressions (>20% slower).** Please investigate before merging.",
269+
])
270+
271+
report = '\n'.join(report_lines)
272+
273+
# Write report to file for the comment action
274+
with open('benchmark-report.md', 'w') as f:
275+
f.write(report)
276+
277+
# Set outputs using GitHub Actions environment files
278+
with open(os.environ['GITHUB_OUTPUT'], 'a') as f:
279+
f.write(f"has_regression={'true' if has_severe_regression else 'false'}\n")
280+
281+
# Print report to console
282+
print(report)
283+
284+
EOF
285+
286+
- name: Find existing comment
287+
uses: peter-evans/find-comment@v3
288+
id: find-comment
289+
with:
290+
issue-number: ${{ github.event.pull_request.number }}
291+
comment-author: 'github-actions[bot]'
292+
body-includes: '📊 Benchmark Comparison Results'
293+
294+
- name: Create or update comment
295+
uses: peter-evans/create-or-update-comment@v4
296+
with:
297+
comment-id: ${{ steps.find-comment.outputs.comment-id }}
298+
issue-number: ${{ github.event.pull_request.number }}
299+
body-path: benchmark-report.md
300+
edit-mode: replace
301+
302+
- name: Fail on severe regression
303+
if: steps.compare.outputs.has_regression == 'true'
304+
run: |
305+
echo "::error::Severe performance regression detected (>20% slower)"
306+
exit 1

0 commit comments

Comments
 (0)