Repository navigation
Conversation
Changelog🏠 Internal
Semantic Diff1 commit · Full page ✏️ Compare the compiler's allocations with the base branch in CI4e9aa3e · cknitt + Claude · Nov 07, 2026, 07:17 UTC · edited 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 |
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. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kbf3gG86E3DicyC4NfsFGP Signed-off-by: Christoph Knittel <ck@cca.io>
e4c2c0c to
4e9aa3e
Compare
|
@codex review |
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex Review: Didn't find any major issues. Swish! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Brings back benchmark alerts in CI, this time on a number that doesn't depend on the runner: the words the compiler allocates. The syntax benchmark job from #7100 alerted on wall-clock times above 105% and was disabled (#7380), then removed (#9147), because shared runners vary by more than that.
How it works
A new
alloc-benchmarksjob runs on pull requests. It builds the compiler, runtime and Belt for the PR and for the base branch (the merge commit's first parent, in agit worktree), and measures both withscripts/allocBenchmarks.js. It fails when a benchmark allocates more than 1% more.The benchmarks:
rescript compiler-argsgives for it. That covers the parser, type checker, Lambda passes and JS generation on real code.let recbindings, a 300-field record with spreads, a 3000-element list literal, and a 300-function inlining chain.bsc -bs-astandbsc -formaton four large files (up to 645 KB), since a build never runs the formatter.Counts come from
OCAMLRUNPARAM=v=0x400(allocated_wordsat exit), so no instrumentation library or code change is needed. The stress inputs and the parse/format files come from the PR's checkout for both sides, so their inputs are identical.If the PR changes any runtime or Belt source, the runtime and Belt rows compile different code on the two sides. They're then listed as "(inputs changed)" with a note, but don't count as regressions. Each file is compiled against the others' interfaces, so one changed source marks all of them.
Syntax benchmarks removed
The allocation comparison makes
tests/syntax_benchmarksandmake benchredundant, so they're removed. Their four data files move totests/alloc_benchmarks/data. The tool also undercounted allocations on OCaml 5:Gc.quick_statminor words only count up to the last minor collection, soParse RedBlackTree.resreported 0 words. The branch's first commit fixed that before the tool was removed; with the squash merge only the removal remains.Reporting
The report goes to the job summary and to one comment on the pull request, posted on every run and edited in place on each push (
comment-tag: alloc-benchmarks):Fork PRs only get the job summary, because their token can't comment.
The job is not in
ci-ok, so a deliberate allocation increase doesn't block a merge. The red check and the comment are the alert.Measured
Locally (linux-arm64 devcontainer):
type_expectcall): total +2.54%, 55 benchmarks over 1%, job exits 1. Largest changes:stress/ListLiteral.res+16%,stress/LetRec.res+14%,Belt_Array.res+6.8%.top_heap_words) is not usable. It moved by up to ±6.7% between the two checkouts above, because heap growth depends on when collections happen, so it isn't reported.In CI the job took 1 min 44 s:
It runs in parallel with
build-compiler, so it doesn't lengthen CI.New tests
scripts/allocBenchmarks.test.js(yarn test-alloc-benchmarks, run at the start of the job) covers:Later: instruction counts
Allocation misses slowdowns that don't allocate, such as scans over existing lists. Cachegrind instruction counts could cover those. The estimate is about 1 min more for the stress inputs and parse/format files, and 2–3 min if the ~15 largest runtime/Belt files are added. Running all ~500 compiles under valgrind would take 10–15 min.
bsccrashed under valgrind on arm64, so this part would run on an x86 runner.🤖 Generated with Claude Code
https://claude.ai/code/session_01Kbf3gG86E3DicyC4NfsFGP