zeroshade commented on code in PR #958:
URL: https://github.com/apache/arrow-go/pull/958#discussion_r4049061229
##########
.github/workflows/benchmark.yml:
##########
@@ -23,44 +23,109 @@ on:
paths:
- ".github/workflows/benchmark.yml"
- "ci/scripts/bench.sh"
+ - "ci/scripts/bench_shard.sh"
- "ci/scripts/bench_adapt.py"
workflow_dispatch:
permissions:
contents: read
jobs:
+ setup:
+ runs-on: ubuntu-latest
+ outputs:
+ matrix: ${{ steps.shards.outputs.matrix }}
+ steps:
+ - name: Checkout repository
+ uses: actions/[email protected]
+ - name: Compute benchmark shards
+ id: shards
+ run: echo "matrix=$(bash ci/scripts/bench_shard.sh 6)" >>
"$GITHUB_OUTPUT"
benchmark:
+ needs: setup
runs-on: ubuntu-latest
+ # The slowest shard (./arrow/array) takes ~40m, so 90m leaves headroom for
+ # runner variance while still failing a pathological benchmark ~4x sooner
+ # than the 6h GitHub default.
+ timeout-minutes: 100
strategy:
+ fail-fast: false
matrix:
- go: ['1.26.1']
- arch: ['amd64']
+ include: ${{ fromJson(needs.setup.outputs.matrix) }}
steps:
- name: Checkout repository
uses: actions/[email protected]
with:
submodules: recursive
- - name: Set up Python
+ - name: Install Go for Benchmarks
+ uses: actions/[email protected]
+ with:
+ go-version: '1.26.1'
Review Comment:
Done, and this was a regression introduced here: the version used to come
from
the one-value `go: ['1.26.1']` matrix that this branch replaced with shard
data,
which left `1.26.1` duplicated as a literal in both the `benchmark` and
`combine` jobs, free to drift apart. Both now use `go-version-file: go.mod`,
matching `rc.yml`.
One heads-up on the consequence, since it may not be what you expected. Our
`go.mod` has no `toolchain` directive and declares `go 1.25.0`, and setup-go
[uses the `go` directive verbatim when it carries a patch
version](https://github.com/actions/setup-go/blob/main/docs/advanced-usage.md#using-the-go-version-file-input),
so this pins the benchmarks to exactly Go 1.25.0 rather than the 1.26.1 they
were running — our compatibility floor, not the toolchain we had been
benchmarking on, and a patch behind the `GO=1.25.8` in `.env`. For Conbench
that
means a one-time step in the timeseries and benchmarking a minor behind what
`test.yml` already covers (1.25/1.26/1.27).
I went with what you asked since it removes the duplication and is easy to
revert. If you would rather keep the benchmark toolchain where it was, I am
happy to switch to a workflow-level `env: GO_VERSION` used by both jobs, or
read
`GO` from `.env` the way `test.yml` does.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
