Dandandan opened a new pull request, #24562:
URL: https://github.com/apache/datafusion/pull/24562
## Which issue does this PR close?
- Closes #.
<!-- No existing issue; happy to file one if that is preferred. -->
## Rationale for this change
Performance regressions currently slip into `main` unnoticed unless someone
runs the benchmarks by hand and posts the numbers on the PR. That is easy to
forget, and the numbers are hard to compare when the two runs come from
different machines or different data.
This adds an opt-in CI job that does the run for you: it benchmarks the base
branch and the PR merged into it, on the same runner, against the same
generated dataset, and fails when the PR is measurably slower.
## What changes are included in this PR?
New `Benchmarks` workflow (`.github/workflows/benchmark.yml`), one job:
- generates TPC-H SF1 parquet data with `tpchgen-cli` (same settings as
`bench.sh data tpch`)
- resolves the base commit from the first parent of the PR merge commit, so
the baseline is exactly the commit the PR would land on, and checks it out
into a `git worktree`
- builds `benchmark_runner` `--release` from both trees, into separate target
directories
- runs `benchmark_runner tpch` on each side (5 iterations by default) and
compares the two result files with `benchmarks/compare.py`
- writes the comparison table to the job summary, uploads both result JSON
files as an artifact, and fails the job on a regression
It is opt-in because two release builds plus two benchmark runs take about an
hour: add the `performance` label to a PR, or start it from the Actions tab,
where the iteration count and both limits can be overridden.
Defaults fail above `1.20x` for a single query or `1.05x` in total, comparing
the fastest iteration of each query. Runners are shared machines, so the
limits are deliberately loose -- the goal is to catch clear regressions
without flagging noise.
Supporting changes in `benchmarks/compare.py`:
- `--fail-threshold` / `--fail-total-threshold` make it exit non-zero on a
per-query or total-time regression. Both are off by default, so existing
usage is unchanged.
- a query that failed on only one side is now reported instead of being
silently dropped from the comparison
- named query ids from `benchmark_runner` (`tpch/Q01/sf1`) are no longer
rendered as `Qtpch/Q01/sf1`; numeric ids still render as `Q1`
- the summary table gained a `Total Change` row
## Are these changes tested?
Yes, though not by the Rust test suite:
- `compare.py` was exercised against real `benchmark_runner tpch --output`
results and against synthetic ones covering each gate: a per-query
regression, a total-time regression, a query failing on one side only,
everything within limits, and no gate flags at all (exit 0, unchanged
output). Numeric and named query ids were both checked, with and without
`--detailed`.
- the exact `benchmark_runner tpch --scale-factor 1 --format parquet
--iterations N --path ... --output ...` invocation the workflow uses was
run
locally.
- `ci/scripts/check_asf_yaml_status_checks.py`, the license header check, and
the typos check pass. The new job is deliberately **not** added to
`.asf.yaml` required checks, since it is opt-in and inherently noisy.
The workflow itself cannot be fully exercised before it is on `main`; adding
the `performance` label to this PR is the way to see it run end to end.
## Are there any user-facing changes?
No API changes. `benchmarks/README.md` documents the new workflow and the two
new `compare.py` flags.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
