Skip to content

ci: shard the benchmark workflow across parallel jobs - #958

Open
zeroshade wants to merge 2 commits into
apache:mainfrom
zeroshade:bench-parallel-shards
Open

ci: shard the benchmark workflow across parallel jobs#958
zeroshade wants to merge 2 commits into
apache:mainfrom
zeroshade:bench-parallel-shards

Conversation

@zeroshade

Copy link
Copy Markdown
Member

Rationale for this change

The Benchmarks workflow runs go test -bench=. ./... sequentially across every
package. Because go test's -timeout is applied per package, the wall-clock time
is the sum over all packages — recent main runs 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, writing
    raw output to a .dat) and --aggregate (merge one or more .dat files into a
    single bench_stats.json via gobenchdata) modes. The existing
    bench.sh <dir> [--json|-json] interface is unchanged, so nothing else that calls
    it needs to change.
  • ci/scripts/bench_shard.sh (new) — prints a GitHub Actions matrix that buckets
    the packages containing benchmarks into N shards.
  • .github/workflows/benchmark.yml — reworked into three jobs: setup (compute
    the shard matrix) → benchmark (matrix; each shard runs its packages and uploads
    its .dat) → combine (download all .dat, aggregate into one bench_stats.json,
    and — only on push to main — upload once to Conbench).
  • ci/scripts/bench_adapt.py — reuses an existing bench_stats.json (produced by
    combine) 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_id fragmentation).

Are these changes tested?

Locally:

  • shellcheck clean on both scripts; actionlint clean on the workflow; py_compile
    OK on bench_adapt.py.
  • Verified the split→merge end to end: ran --run on two packages, then --aggregate
    produced one bench_stats.json containing both suites, in the exact shape
    bench_adapt.py consumes.
  • The legacy bench.sh <dir> --json path 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

  • Sharding is currently round-robin by package, not runtime-weighted, so a single
    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 for
    any single package. Once this runs, per-shard timings can seed a runtime-weighted
    split or tune the shard count.

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
zeroshade force-pushed the bench-parallel-shards branch from 8b6901a to 8fb9c4c Compare September 4, 2026 20:06
@zeroshade
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.
@zeroshade
zeroshade requested a review from lidavidm September 8, 2026 18:17

@lidavidm lidavidm left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants