ci: shard the benchmark workflow across parallel jobs - #958
Open
zeroshade wants to merge 2 commits into
Open
Conversation
The Benchmarks workflow ran `go test -bench=. ./...` sequentially over every package, and because `-timeout` applies per package the wall-clock time summed to roughly three hours. Split the run so it can be parallelized: - bench.sh gains --run (benchmark a subset of packages into a .dat) and --aggregate (combine .dat files into one JSON) modes; the legacy "<dir> [--json]" interface is unchanged. - bench_shard.sh emits a GitHub Actions matrix that buckets the packages containing benchmarks into shards. - benchmark.yml becomes setup -> benchmark (shard matrix) -> combine. Each shard uploads its .dat; combine merges them into a single bench_stats.json and, on push to main, uploads once to Conbench. - bench_adapt.py reuses an existing bench_stats.json instead of re-running the suite. Aggregating into one JSON preserves a single Conbench run (no fragmentation).
zeroshade
force-pushed
the
bench-parallel-shards
branch
from
September 4, 2026 20:06
8b6901a to
8fb9c4c
Compare
zeroshade
marked this pull request as ready for review
September 8, 2026 16:36
Shard 0 was cancelled at the 6h GitHub Actions limit. Two causes: 1. generateJSONData padded each record with make([]byte, 500), i.e. 500 NUL bytes. json.Marshal escapes every one of them as a 6-character \u0000 sequence, so a "500 byte" field became ~3KB of escapes, and goccy/go-json's decodeUnicode memmoves the remainder of the buffer once per escape. Profiling shows 99.5% of the run in runtime.memmove under decodeUnicode. That quadratic blowup accounted for 249 of shard 0's 290 observed minutes, with two cases still unfinished at cancellation. Padding with a printable string instead drops the whole set from hours to ~19s, and the benchmarks now get enough iterations (63-627 vs 1) to report meaningful numbers. 2. go test -timeout does not cover benchmarks -- the testing package calls stopAlarm() before runBenchmarks() -- so the 40m timeout was inert and a runaway shard burned the full 6h instead of failing fast. Wrap the run in timeout(1) instead, signalling QUIT so the test binary dumps goroutines and names the stuck benchmark. The remaining work in shard 0 is ~40m, so shards get a 90m budget and the job a 100m cap.
lidavidm
reviewed
Sep 8, 2026
lidavidm
left a comment
Member
There was a problem hiding this comment.
Since we've been trying to reduce the CI usage...the aggregate runner minutes used is still about the same, right? 3 hours of worker resources/commit seems pretty heavy
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Rationale for this change
The
Benchmarksworkflow runsgo test -bench=. ./...sequentially across everypackage. Because
go test's-timeoutis applied per package, the wall-clock timeis the sum over all packages — recent
mainruns have taken roughly 3.3 hours(198–208 min).
What changes are included in this PR?
Split the benchmark run so it can be parallelized, then combine the results into a
single upload:
ci/scripts/bench.sh— adds--run(benchmark a subset of packages, writingraw output to a
.dat) and--aggregate(merge one or more.datfiles into asingle
bench_stats.jsonviagobenchdata) modes. The existingbench.sh <dir> [--json|-json]interface is unchanged, so nothing else that callsit needs to change.
ci/scripts/bench_shard.sh(new) — prints a GitHub Actions matrix that bucketsthe packages containing benchmarks into N shards.
.github/workflows/benchmark.yml— reworked into three jobs:setup(computethe shard matrix) →
benchmark(matrix; each shard runs its packages and uploadsits
.dat) →combine(download all.dat, aggregate into onebench_stats.json,and — only on push to
main— upload once to Conbench).ci/scripts/bench_adapt.py— reuses an existingbench_stats.json(produced bycombine) instead of re-running the whole suite.Because the shards are merged into one JSON and uploaded once, Conbench still sees a
single run (no
run_idfragmentation).Are these changes tested?
Locally:
shellcheckclean on both scripts;actionlintclean on the workflow;py_compileOK on
bench_adapt.py.--runon two packages, then--aggregateproduced one
bench_stats.jsoncontaining both suites, in the exact shapebench_adapt.pyconsumes.bench.sh <dir> --jsonpath still runs → aggregates → cleans up.Opened as a draft to exercise the reworked workflow in CI end to end (it triggers
on changes to these files).
Are there any user-facing changes?
No. This only touches CI / benchmark tooling.
Notes / follow-ups
shard can hold two heavy packages (e.g.
arrow/compute+parquet/internal/encoding)and become the long pole. The per-package
-timeout(40m) remains the hard floor forany single package. Once this runs, per-shard timings can seed a runtime-weighted
split or tune the shard count.