Skip to content

Commit bf35764

Browse files
cknittclaude
andauthored
Count the compiler's instructions with cachegrind in the benchmarks (#9254)
Allocation misses slowdowns that don't allocate, such as a scan of a list that already exists. The benchmark job now runs on x64 and also counts, with cachegrind, the instructions the compiler executes on the stress inputs, the parse and format runs and the 15 largest runtime and Belt files; the comment shows both counts in a section each. Base and head are built and measured from the same path, and the runs under cachegrind use a minor heap larger than any benchmark allocates, so two builds of the same compiler give the same counts. Four more stress inputs cover constructs whose compile time and allocations grow faster than their size, and the script, its data and the comment are named after the compiler. 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 1bda256 commit bf35764

11 files changed

Lines changed: 917 additions & 550 deletions

File tree

‎.github/workflows/ci.yml‎

Lines changed: 64 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -354,20 +354,22 @@ 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:
357+
# Compares the words the compiler allocates on fixed inputs, and the
358+
# instructions it executes on some of them, with the base branch
359+
# (scripts/compilerBenchmarks.js). Neither count depends on the runner's
360+
# speed, so this can fail on a growth of 1%. It is not needed by ci-ok: a
361+
# deliberate growth shouldn't block a merge. The report goes to the job
362+
# summary and, for a pull request from a branch of this repository, to a
363+
# comment on it; a fork's pull request gets a token that can't comment.
364+
benchmarks:
364365
if: github.event_name == 'pull_request'
365-
runs-on: ubuntu-26.04-arm
366+
# x64, since bsc doesn't run under valgrind on arm64.
367+
runs-on: ubuntu-26.04
366368
permissions:
367369
contents: read
368370
pull-requests: write
369371
env:
370-
# The same build as the linux-arm64 build-compiler job, so that the
372+
# The same build as the first linux-x64 build-compiler job, so that the
371373
# OPAM environment and the dune cache are shared.
372374
DUNE_PROFILE: static
373375
OCAML_COMPILER: ocaml-variants.5.5.2+options,ocaml-option-static
@@ -382,23 +384,29 @@ jobs:
382384
uses: ./.github/actions/setup-node
383385

384386
- name: Test the benchmark script
385-
run: yarn test-alloc-benchmarks
387+
run: yarn test-compiler-benchmarks
386388

387389
- name: Setup Linux dependencies
388390
uses: ./.github/actions/setup-linux-dependencies
389391

392+
- name: Install valgrind
393+
run: |
394+
sudo apt-get update -qq
395+
sudo apt-get install -y -qq --no-install-recommends valgrind
396+
valgrind --version
397+
390398
- name: Setup OCaml
391399
id: setup_ocaml
392400
uses: ./.github/actions/setup-ocaml
393401
with:
394402
compiler: ${{ env.OCAML_COMPILER }}
395-
os: ubuntu-26.04-arm
403+
os: ubuntu-26.04
396404
cache-prefix: opam-env-v9
397405

398406
- name: Compiler build state key
399407
id: compiler-build-state-key
400408
run: |
401-
echo "value=compiler-build-state-v1-ubuntu-26.04-arm-${{ steps.setup_ocaml.outputs.setup_version }}-$OCAML_COMPILER-${{ hashFiles('*.opam') }}" \
409+
echo "value=compiler-build-state-v1-ubuntu-26.04-${{ steps.setup_ocaml.outputs.setup_version }}-$OCAML_COMPILER-${{ hashFiles('*.opam') }}" \
402410
| sed 's/,/-/g' >> "$GITHUB_OUTPUT"
403411
404412
- name: Restore compiler build state
@@ -412,46 +420,73 @@ jobs:
412420
_build
413421
key: ${{ steps.compiler-build-state-key.outputs.value }}
414422

415-
- name: Build the compiler, runtime and Belt
416-
run: |
417-
opam exec -- dune build --display quiet
418-
node scripts/allocBenchmarks.js prepare .
423+
- name: Build the compiler
424+
run: opam exec -- dune build --display quiet
419425

420-
- name: Build the base branch's compiler, runtime and Belt
426+
- name: Build the base branch's compiler
421427
run: |
422428
git worktree add --detach "$RUNNER_TEMP/base" HEAD^1
423429
opam exec -- dune build --root "$RUNNER_TEMP/base" --display quiet
424-
node scripts/allocBenchmarks.js prepare "$RUNNER_TEMP/base"
425430
426431
- name: Measure and compare
427-
id: compare
428432
env:
429433
RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}
430434
run: |
431-
node scripts/allocBenchmarks.js measure "$RUNNER_TEMP/base" "$RUNNER_TEMP/base.json"
432-
node scripts/allocBenchmarks.js measure . "$RUNNER_TEMP/head.json"
435+
# Both checkouts are measured from the same path, one after the
436+
# other: the paths in the compiler's arguments change its counts a
437+
# little, and the instructions of the runtime and Belt files by up
438+
# to 0.2%. The runtime and Belt are built there too, since the
439+
# replayed builds would otherwise write interfaces that don't
440+
# match those of the files they don't replay yet.
441+
measure() {
442+
sudo mkdir -p /bench
443+
sudo mount --bind "$1" /bench
444+
node scripts/compilerBenchmarks.js prepare /bench
445+
node scripts/compilerBenchmarks.js measure /bench "$2" --instructions
446+
sudo umount /bench
447+
}
448+
measure "$RUNNER_TEMP/base" "$RUNNER_TEMP/base.json"
449+
measure "$GITHUB_WORKSPACE" "$RUNNER_TEMP/head.json"
433450
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"
451+
report="$RUNNER_TEMP/benchmarks-report.md"
452+
node scripts/compilerBenchmarks.js compare "$RUNNER_TEMP/base.json" "$RUNNER_TEMP/head.json" \
453+
--threshold 1 --summary "$report" || status=$?
454+
cat "$report" >> "$GITHUB_STEP_SUMMARY"
437455
printf '\nPull request %s compared with base %s in [this run](%s).\n' \
438456
"$(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"
457+
>> "$report"
441458
exit "$status"
442459
460+
# When a step failed before the comparison, the comment says so instead
461+
# of keeping the report of an earlier commit.
462+
- name: Report a failure to run the benchmarks
463+
if: ${{ !cancelled() }}
464+
env:
465+
RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}
466+
HEAD_SHA: ${{ github.event.pull_request.head.sha }}
467+
run: |
468+
report="$RUNNER_TEMP/benchmarks-report.md"
469+
if [ ! -f "$report" ]; then
470+
{
471+
echo "## ⚠️ Compiler benchmarks"
472+
echo
473+
echo "The benchmarks couldn't run for pull request ${HEAD_SHA:0:9}; [the run]($RUN_URL) shows why."
474+
} > "$report"
475+
cat "$report" >> "$GITHUB_STEP_SUMMARY"
476+
fi
477+
443478
# The pull request's one comment, edited in place on each push, also
444479
# when nothing changed, so that it shows the comparison ran.
445480
- name: Comment on the pull request
446481
if: >-
447-
!cancelled() && steps.compare.outputs.summary != '' &&
482+
!cancelled() &&
448483
github.event.pull_request.head.repo.full_name == github.repository
449484
uses: thollander/actions-comment-pull-request@v3
450485
with:
451486
pr-number: ${{ github.event.pull_request.number }}
452-
comment-tag: alloc-benchmarks
487+
comment-tag: compiler-benchmarks
453488
mode: upsert
454-
file-path: ${{ steps.compare.outputs.summary }}
489+
file-path: ${{ runner.temp }}/benchmarks-report.md
455490

456491
dev-playground:
457492
needs:
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
- Count the instructions the compiler executes on some of the CI benchmarks with cachegrind, next to the words it allocates.

‎package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +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",
55+
"test-compiler-benchmarks": "node --test scripts/compilerBenchmarks.test.js",
5656
"check": "biome check --no-errors-on-unmatched .",
5757
"check:all": "biome check .",
5858
"format": "biome check --no-errors-on-unmatched . --fix",

0 commit comments

Comments
 (0)