Skip to content

Commit b91ab51

Browse files
committed
#2541 - Refactor benchmark job to run on pushes and improve PR handling
1 parent 5e7b977 commit b91ab51

2 files changed

Lines changed: 60 additions & 22 deletions

File tree

.github/workflows/main.yml

Lines changed: 50 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -239,23 +239,52 @@ jobs:
239239
retention-days: 7
240240

241241
benchmark:
242-
name: "Benchmark (PR vs ${{ github.base_ref }})"
243-
if: github.event_name == 'pull_request'
242+
name: "Benchmark"
243+
# Triggered by every push EXCEPT pushes to the default branch (which is
244+
# the comparison baseline) and tag pushes (releases). We deliberately
245+
# don't add `pull_request` to `on:` above because that would double-run
246+
# the other jobs (analyze, build-and-test) on each PR.
247+
if: github.event_name == 'push' && github.ref != 'refs/heads/development' && !startsWith(github.ref, 'refs/tags/')
244248
runs-on: ubuntu-24.04
245249
permissions:
246250
contents: read
247251
pull-requests: write
248252
env:
249253
PHP_VERSION: '8.5'
254+
BASE_BRANCH: development
250255
BENCH_PHP_FLAGS: '-d extension=ext/modules/stub.so'
251256

252257
steps:
253-
- name: Checkout PR head
258+
- name: Checkout head
254259
uses: actions/checkout@v6
255260
with:
256261
# Need full history so we can switch to the base branch in-place.
257262
fetch-depth: 0
258-
ref: ${{ github.event.pull_request.head.sha }}
263+
264+
- name: Discover associated PR
265+
id: pr
266+
env:
267+
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
268+
run: |
269+
branch="${GITHUB_REF#refs/heads/}"
270+
echo "branch=$branch" >> "$GITHUB_OUTPUT"
271+
# Look up the first open PR with this branch as head. Fork PRs are
272+
# surfaced as `owner:branch`, so we narrow by repo as well.
273+
pr_json=$(gh pr list \
274+
--repo "$GITHUB_REPOSITORY" \
275+
--head "$branch" \
276+
--state open \
277+
--json number,baseRefName,headRefName \
278+
--jq '.[0] // empty' 2>/dev/null || true)
279+
if [ -n "$pr_json" ]; then
280+
echo "has_pr=true" >> "$GITHUB_OUTPUT"
281+
echo "number=$(echo "$pr_json" | jq -r .number)" >> "$GITHUB_OUTPUT"
282+
echo "base=$(echo "$pr_json" | jq -r .baseRefName)" >> "$GITHUB_OUTPUT"
283+
else
284+
echo "has_pr=false" >> "$GITHUB_OUTPUT"
285+
echo "base=${BASE_BRANCH}" >> "$GITHUB_OUTPUT"
286+
echo "No open PR found for branch '$branch'; report will be uploaded as an artifact only."
287+
fi
259288
260289
- name: Setup PHP ${{ env.PHP_VERSION }}
261290
uses: shivammathur/setup-php@v2
@@ -275,14 +304,14 @@ jobs:
275304
- name: Resolve base ref SHA
276305
id: base
277306
run: |
278-
base_sha=$(git rev-parse origin/${{ github.base_ref }})
307+
base_sha=$(git rev-parse "origin/${{ steps.pr.outputs.base }}")
279308
echo "sha=$base_sha" >> "$GITHUB_OUTPUT"
280-
echo "Base: ${{ github.base_ref }} ($base_sha)"
281-
echo "Head: ${{ github.event.pull_request.head.sha }}"
309+
echo "Base: ${{ steps.pr.outputs.base }} ($base_sha)"
310+
echo "Head: $GITHUB_SHA"
282311
283-
- name: Build & bench base branch (${{ github.base_ref }})
312+
- name: Build & bench base branch (${{ steps.pr.outputs.base }})
284313
run: |
285-
git checkout ${{ steps.base.outputs.sha }}
314+
git checkout "${{ steps.base.outputs.sha }}"
286315
if [ ! -f tests/Benchmark/bootstrap.php ] || [ ! -f phpbench.json ]; then
287316
echo "BASE_HAS_BENCH=false" >> "$GITHUB_ENV"
288317
echo "Base branch does not yet include the benchmark suite; skipping baseline run."
@@ -299,14 +328,14 @@ jobs:
299328
mv .phpbench "$RUNNER_TEMP/phpbench/storage"
300329
fi
301330
302-
- name: Restore PR head
331+
- name: Restore head
303332
run: |
304-
git checkout ${{ github.event.pull_request.head.sha }}
333+
git checkout "$GITHUB_SHA"
305334
if [ -d "$RUNNER_TEMP/phpbench/storage" ]; then
306335
mv "$RUNNER_TEMP/phpbench/storage" .phpbench
307336
fi
308337
309-
- name: Build & bench PR head
338+
- name: Build & bench head
310339
id: bench
311340
run: |
312341
composer install --prefer-dist --no-interaction --no-ansi --no-progress
@@ -330,17 +359,17 @@ jobs:
330359
fi
331360
# PHPBench may emit a "Module already loaded" warning when the .so is
332361
# auto-loaded from php.ini AND via -d extension=. Strip those lines so
333-
# the PR comment stays readable.
362+
# the comment stays readable.
334363
sed -i '/Warning: Module .* is already loaded/d' .phpbench/report.txt
335364
336365
{
337366
echo "## Benchmark report"
338367
echo
339368
if [ "$BASE_HAS_BENCH" = "true" ]; then
340-
echo "Comparison against \`${{ github.base_ref }}\` (\`${{ steps.base.outputs.sha }}\`)."
341-
echo "Each row's \`mode\` column shows the PR-branch absolute throughput and the percent delta vs base. Positive deltas on \`Zephir*\` subjects mean the PR is faster; positive deltas on \`Php*\` baseline subjects are noise floor signal."
369+
echo "Comparison against \`${{ steps.pr.outputs.base }}\` (\`${{ steps.base.outputs.sha }}\`) on \`${{ steps.pr.outputs.branch }}\` (\`$GITHUB_SHA\`)."
370+
echo "Each row's \`mode\` column shows the head-branch absolute throughput and the percent delta vs base. Positive deltas on \`Zephir*\` subjects mean head is faster; deltas on \`Php*\` baseline subjects are noise-floor signal."
342371
else
343-
echo "Base branch \`${{ github.base_ref }}\` does not yet contain the benchmark suite; absolute throughput only."
372+
echo "Base branch \`${{ steps.pr.outputs.base }}\` does not yet contain the benchmark suite; absolute throughput only."
344373
fi
345374
echo
346375
echo '```'
@@ -354,21 +383,23 @@ jobs:
354383
if: always()
355384
uses: actions/upload-artifact@v7
356385
with:
357-
name: benchmark-report-${{ github.event.pull_request.number }}
386+
name: benchmark-report-${{ steps.pr.outputs.branch }}
358387
path: |
359388
.phpbench/report.txt
360389
.phpbench/comment.md
361390
retention-days: 14
362391

363392
- name: Post or update PR comment
364-
if: always()
393+
if: always() && steps.pr.outputs.has_pr == 'true'
365394
uses: actions/github-script@v7
395+
env:
396+
PR_NUMBER: ${{ steps.pr.outputs.number }}
366397
with:
367398
script: |
368399
const fs = require('fs');
369400
const body = fs.readFileSync('.phpbench/comment.md', 'utf8');
370401
const marker = '<!-- zephir-bench-report -->';
371-
const issue_number = context.payload.pull_request.number;
402+
const issue_number = parseInt(process.env.PR_NUMBER, 10);
372403
const { owner, repo } = context.repo;
373404
374405
const { data: comments } = await github.rest.issues.listComments({

tests/Benchmark/README.md

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,14 +24,21 @@ If you build on the host and run inside the container without fullclean, the
2424
.libs object files from your host's PHP version will conflict. Always
2525
`fullclean` when crossing PHP versions.
2626

27-
## Comparing PR vs base
27+
## Comparing two branches
2828

29-
The same flow CI uses, in two steps:
29+
CI runs the bench job on every `push` to a non-default branch (see the
30+
`benchmark` job in `.github/workflows/main.yml`). If the pushed branch has an
31+
open PR against `development`, the report is posted as an auto-updating PR
32+
comment; otherwise it is uploaded as an artifact only. The default-branch
33+
push and tag pushes are skipped so the comparison baseline doesn't run
34+
against itself.
35+
36+
Locally, the same flow in two steps:
3037

3138
```bash
3239
# 1. Capture a baseline tagged "base" on the branch you want to compare
3340
# against. Build the extension first so the bench targets exist.
34-
git checkout develop
41+
git checkout development
3542
php zephir fullclean && php zephir build
3643
php -d extension=ext/modules/stub.so vendor/bin/phpbench run \
3744
--tag=base --progress=none

0 commit comments

Comments
 (0)