Skip to content

Commit f4d6cf4

Browse files
joseph-isaacsclaude
andcommitted
Comment on benchmarks that exceed the 1 ms iteration budget
The benchmarking guide asks that a single benchmark iteration stay under 1 ms, but nothing enforced it, so new benchmarks landed at 23.8 ms and 123.4 ms and were caught only by a maintainer reading CodSpeed's table by hand. CodSpeed already measures per-iteration time and publishes it in its sticky PR comment. Read that comment rather than measuring anything: an `issue_comment` job parses CodSpeed's report when it is posted or edited, and comments listing the benchmarks over budget. Nothing is rebuilt and nothing is re-run. The trade is scope, not accuracy, and the rendered comment says so: only benchmarks CodSpeed reports as new or changed are listed, and CodSpeed truncates its own table at 20 rows. Benchmarks this PR made faster are not flagged for being over budget. Report only, never a failing check. The all-clear text updates an existing complaint but never opens a new comment, so a PR that is within budget stays quiet. Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FZJAp4wVgwTVhLyErCWFZn
1 parent 4ebe623 commit f4d6cf4

5 files changed

Lines changed: 714 additions & 1 deletion

File tree

‎.github/workflows/bench-budget.yml‎

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
name: Benchmark Budget
2+
3+
# Enforces the "keep per-iteration execution time under 1 ms" rule from
4+
# docs/developer-guide/benchmarking.md, reported as a comment rather than a failing check.
5+
#
6+
# CodSpeed already measures per-iteration time and publishes it in its own sticky PR
7+
# comment, so this workflow reads that comment instead of building or running anything.
8+
# The trade is scope, not accuracy: CodSpeed only reports benchmarks a PR added or
9+
# changed, so an untouched benchmark that is already over budget is not caught here.
10+
#
11+
# `issue_comment` workflows always run from the default branch, so edits to this file (or
12+
# to the script) only take effect once merged. Use the `workflow_dispatch` entry point to
13+
# try a change against a real PR's report before merging it.
14+
15+
on:
16+
issue_comment:
17+
types: [created, edited]
18+
workflow_dispatch:
19+
inputs:
20+
pr-number:
21+
description: "Pull request to re-check against the budget"
22+
required: true
23+
type: string
24+
25+
permissions:
26+
contents: read
27+
28+
concurrency:
29+
group: ${{ github.workflow }}-${{ github.event.issue.number || inputs.pr-number }}
30+
cancel-in-progress: true
31+
32+
jobs:
33+
bench-budget:
34+
name: "Check benchmark iteration budget"
35+
# CodSpeed edits one sticky comment per PR, so this fires on every update to its
36+
# report. Anyone can post a comment containing the marker; pinning the author is what
37+
# makes the parsed table trustworthy.
38+
if: >-
39+
github.event_name == 'workflow_dispatch'
40+
|| (github.event.issue.pull_request != null
41+
&& github.event.comment.user.login == 'codspeed-hq[bot]'
42+
&& contains(github.event.comment.body, '__CODSPEED_PERFORMANCE_REPORT_COMMENT__'))
43+
runs-on: ubuntu-latest
44+
timeout-minutes: 10
45+
permissions:
46+
contents: read
47+
issues: read
48+
pull-requests: write
49+
env:
50+
PR_NUMBER: ${{ github.event.issue.number || inputs.pr-number }}
51+
steps:
52+
# On `issue_comment` this checks out the default branch, never the pull request's
53+
# head. That is deliberate: the job holds a `pull-requests: write` token, and must
54+
# run this repository's script rather than a version a fork could edit.
55+
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7
56+
57+
# The comment body is untrusted input, so it reaches the script through the
58+
# environment and never through shell interpolation.
59+
- name: Read CodSpeed report from the triggering comment
60+
if: github.event_name != 'workflow_dispatch'
61+
env:
62+
COMMENT_BODY: ${{ github.event.comment.body }}
63+
run: printenv COMMENT_BODY > codspeed-comment.md
64+
65+
- name: Fetch CodSpeed report for the requested pull request
66+
if: github.event_name == 'workflow_dispatch'
67+
env:
68+
GH_TOKEN: ${{ github.token }}
69+
run: |
70+
set -Eeuo pipefail
71+
gh api "repos/${{ github.repository }}/issues/${PR_NUMBER}/comments" --paginate \
72+
--jq '[.[] | select(.user.login == "codspeed-hq[bot]") | .body] | last' \
73+
> codspeed-comment.md
74+
test -s codspeed-comment.md
75+
76+
- name: Check against the budget
77+
id: check
78+
run: |
79+
python3 scripts/check-bench-budget.py \
80+
--comment-file codspeed-comment.md \
81+
--output comment.md
82+
cat comment.md >> "$GITHUB_STEP_SUMMARY"
83+
84+
- name: Report over-budget benchmarks
85+
if: steps.check.outputs.violations != '0'
86+
uses: thollander/actions-comment-pull-request@24bffb9b452ba05a4f3f77933840a6a841d1b32b # v3
87+
with:
88+
file-path: comment.md
89+
comment-tag: bench-budget
90+
pr-number: ${{ env.PR_NUMBER }}
91+
92+
# Nothing is over budget: update an existing complaint to say so, but never open a
93+
# new all-clear comment on every PR that touches a benchmark.
94+
- name: Clear a resolved report
95+
if: steps.check.outputs.violations == '0'
96+
uses: thollander/actions-comment-pull-request@24bffb9b452ba05a4f3f77933840a6a841d1b32b # v3
97+
with:
98+
file-path: comment.md
99+
comment-tag: bench-budget
100+
pr-number: ${{ env.PR_NUMBER }}
101+
create-if-not-exists: false

‎.github/workflows/ci.yml‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,24 @@ jobs:
166166
uv run --no-project --with pytest --with xxhash \
167167
pytest scripts/tests/test_measurement_id.py
168168
169+
# Covers scripts/check-bench-budget.py, which parses CodSpeed's PR comment. It runs from
170+
# the `issue_comment` workflow where a parsing bug surfaces as a wrong comment on someone
171+
# else's PR rather than as a red check, so the parser is tested here instead.
172+
bench-budget-script:
173+
name: "Benchmark budget script"
174+
runs-on: ubuntu-latest
175+
timeout-minutes: 10
176+
steps:
177+
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7
178+
# sync: false — pure-stdlib script, so skip the ~6 min vortex-data extension build.
179+
- uses: spiraldb/actions/.github/actions/setup-uv@a746510eafaa926484c354541cfc49b2ec06cc63 # 0.18.6
180+
with:
181+
sync: false
182+
- name: Pytest - benchmark budget check
183+
run: |
184+
uv run --no-project --with pytest \
185+
pytest scripts/tests/test_check_bench_budget.py
186+
169187
python-cuda-test:
170188
name: "Python CUDA (test)"
171189
if: github.repository == 'vortex-data/vortex'

‎docs/developer-guide/benchmarking.md‎

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,26 @@ gate it with `#[cfg(not(codspeed))]` if it genuinely cannot be made to fit.
132132
The number to check against the budget is the per-iteration time, not the time the whole
133133
benchmark binary takes. CodSpeed reports exactly that: its performance report on a pull
134134
request lists the per-iteration time under `HEAD` for every benchmark the pull request adds
135-
or changes, so check any new benchmark there before merging.
135+
or changes.
136+
137+
CI reads that report on every pull request, and comments rather than failing the build. The
138+
`bench-budget` job in `.github/workflows/bench-budget.yml` pulls the per-iteration times out
139+
of CodSpeed's report and comments listing any benchmark over the budget. Nothing is rebuilt
140+
or re-run: the numbers are the ones CodSpeed already published.
141+
142+
Two limits follow from using that report as the source:
143+
144+
- Only benchmarks CodSpeed reports as **new or changed** are checked, because those are the
145+
only ones it lists. A benchmark you did not touch is not re-checked, so the budget is
146+
enforced going forward rather than retroactively.
147+
- CodSpeed truncates its table at 20 rows. When it does, the comment says so rather than
148+
implying the rest were checked — open the full report in CodSpeed to see them.
149+
150+
To check a report by hand, for example while iterating on the check itself:
151+
152+
```bash
153+
python3 scripts/check-bench-budget.py --comment-file codspeed-comment.md --output comment.md
154+
```
136155

137156
### Gate CodSpeed-incompatible benchmarks
138157

0 commit comments

Comments
 (0)