Skip to content

Commit 1bda256

Browse files
cknittclaude
andauthored
Compare the compiler's allocations with the base branch in CI (#9253)
Timings on shared runners are too noisy to alert on, which is why the previous benchmark job was removed. The words the compiler allocates are the same in every run, so a new job measures them for the base branch and the pull request, fails when a benchmark allocates more than 1% more, and comments the comparison on the pull request. The syntax benchmarks and `make bench` are removed; the comparison parses and formats their files with bsc, from tests/alloc_benchmarks/data. Claude-Session: https://claude.ai/code/session_01Kbf3gG86E3DicyC4NfsFGP Signed-off-by: Christoph Knittel <ck@cca.io> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 827be16 commit 1bda256

15 files changed

Lines changed: 826 additions & 316 deletions

File tree

‎.github/workflows/ci.yml‎

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -354,6 +354,105 @@ jobs:
354354
name: api
355355
path: scripts/res/apiDocs/
356356

357+
# Compares the words the compiler allocates on fixed inputs with the base
358+
# branch (scripts/allocBenchmarks.js). Allocation counts don't depend on the
359+
# runner's speed, so this can fail on a growth of 1%. It is not needed by
360+
# ci-ok: a deliberate growth shouldn't block a merge. The report goes to the
361+
# job summary and, for a pull request from a branch of this repository, to
362+
# a comment on it; a fork's pull request gets a token that can't comment.
363+
alloc-benchmarks:
364+
if: github.event_name == 'pull_request'
365+
runs-on: ubuntu-26.04-arm
366+
permissions:
367+
contents: read
368+
pull-requests: write
369+
env:
370+
# The same build as the linux-arm64 build-compiler job, so that the
371+
# OPAM environment and the dune cache are shared.
372+
DUNE_PROFILE: static
373+
OCAML_COMPILER: ocaml-variants.5.5.2+options,ocaml-option-static
374+
steps:
375+
- name: Checkout
376+
uses: actions/checkout@v7
377+
with:
378+
# The merge commit and its first parent, the base branch.
379+
fetch-depth: 2
380+
381+
- name: Setup Node and dependencies
382+
uses: ./.github/actions/setup-node
383+
384+
- name: Test the benchmark script
385+
run: yarn test-alloc-benchmarks
386+
387+
- name: Setup Linux dependencies
388+
uses: ./.github/actions/setup-linux-dependencies
389+
390+
- name: Setup OCaml
391+
id: setup_ocaml
392+
uses: ./.github/actions/setup-ocaml
393+
with:
394+
compiler: ${{ env.OCAML_COMPILER }}
395+
os: ubuntu-26.04-arm
396+
cache-prefix: opam-env-v9
397+
398+
- name: Compiler build state key
399+
id: compiler-build-state-key
400+
run: |
401+
echo "value=compiler-build-state-v1-ubuntu-26.04-arm-${{ steps.setup_ocaml.outputs.setup_version }}-$OCAML_COMPILER-${{ hashFiles('*.opam') }}" \
402+
| sed 's/,/-/g' >> "$GITHUB_OUTPUT"
403+
404+
- name: Restore compiler build state
405+
uses: actions/cache/restore@v6
406+
with:
407+
# The paths are part of the cache's version, so they must be the
408+
# ones build-compiler saves.
409+
path: |
410+
C:\.cache\dune
411+
~/.cache/dune
412+
_build
413+
key: ${{ steps.compiler-build-state-key.outputs.value }}
414+
415+
- name: Build the compiler, runtime and Belt
416+
run: |
417+
opam exec -- dune build --display quiet
418+
node scripts/allocBenchmarks.js prepare .
419+
420+
- name: Build the base branch's compiler, runtime and Belt
421+
run: |
422+
git worktree add --detach "$RUNNER_TEMP/base" HEAD^1
423+
opam exec -- dune build --root "$RUNNER_TEMP/base" --display quiet
424+
node scripts/allocBenchmarks.js prepare "$RUNNER_TEMP/base"
425+
426+
- name: Measure and compare
427+
id: compare
428+
env:
429+
RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}
430+
run: |
431+
node scripts/allocBenchmarks.js measure "$RUNNER_TEMP/base" "$RUNNER_TEMP/base.json"
432+
node scripts/allocBenchmarks.js measure . "$RUNNER_TEMP/head.json"
433+
status=0
434+
node scripts/allocBenchmarks.js compare "$RUNNER_TEMP/base.json" "$RUNNER_TEMP/head.json" \
435+
--threshold 1 --summary "$RUNNER_TEMP/alloc-summary.md" || status=$?
436+
cat "$RUNNER_TEMP/alloc-summary.md" >> "$GITHUB_STEP_SUMMARY"
437+
printf '\nPull request %s compared with base %s in [this run](%s).\n' \
438+
"$(git rev-parse --short=9 HEAD^2)" "$(git rev-parse --short=9 HEAD^1)" "$RUN_URL" \
439+
>> "$RUNNER_TEMP/alloc-summary.md"
440+
echo "summary=$RUNNER_TEMP/alloc-summary.md" >> "$GITHUB_OUTPUT"
441+
exit "$status"
442+
443+
# The pull request's one comment, edited in place on each push, also
444+
# when nothing changed, so that it shows the comparison ran.
445+
- name: Comment on the pull request
446+
if: >-
447+
!cancelled() && steps.compare.outputs.summary != '' &&
448+
github.event.pull_request.head.repo.full_name == github.repository
449+
uses: thollander/actions-comment-pull-request@v3
450+
with:
451+
pr-number: ${{ github.event.pull_request.number }}
452+
comment-tag: alloc-benchmarks
453+
mode: upsert
454+
file-path: ${{ steps.compare.outputs.summary }}
455+
357456
dev-playground:
358457
needs:
359458
- build-compiler

‎Makefile‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -125,9 +125,6 @@ artifacts: lib
125125

126126
# Tests
127127

128-
bench: compiler
129-
$(DUNE_BIN_DIR)/syntax_benchmarks
130-
131128
test: lib
132129
node scripts/test.js -all
133130

@@ -351,4 +348,4 @@ clean: clean-lib clean-compiler clean-coverage
351348

352349
.DEFAULT_GOAL := build
353350

354-
.PHONY: yarn-install build compiler lib artifacts bench test test-analysis test-reanalyze benchmark-reanalyze test-tools test-syntax test-syntax-roundtrip test-gentype test-rewatch test-all playground playground-compiler playground-test playground-cmijs playground-release format checkformat semantic-diff clean-compiler clean-lib clean-gentype clean-tests clean
351+
.PHONY: yarn-install build compiler lib artifacts test test-analysis test-reanalyze benchmark-reanalyze test-tools test-syntax test-syntax-roundtrip test-gentype test-rewatch test-all playground playground-compiler playground-test playground-cmijs playground-release format checkformat semantic-diff clean-compiler clean-lib clean-gentype clean-tests clean
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
- Compare the words the compiler allocates with the base branch in CI, and fail on a growth of more than 1%.
2+
- Remove the syntax benchmarks (`make bench`); the CI comparison parses and formats their files with `bsc` instead.

‎package.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@
5252
"test-build": "node scripts/test.js -build",
5353
"test-ocaml": "node scripts/test.js -ounit",
5454
"test-release-plan": "node --test scripts/releasePlan.test.js",
55+
"test-alloc-benchmarks": "node --test scripts/allocBenchmarks.test.js",
5556
"check": "biome check --no-errors-on-unmatched .",
5657
"check:all": "biome check .",
5758
"format": "biome check --no-errors-on-unmatched . --fix",

0 commit comments

Comments
 (0)