diff --git a/.github/workflows/benchmark.yml b/.github/workflows/benchmark.yml new file mode 100644 index 0000000000000..684df57683ad9 --- /dev/null +++ b/.github/workflows/benchmark.yml @@ -0,0 +1,349 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +# Catches performance regressions: runs TPC-H SF10 for a candidate commit +# (`head`) and for the commit it sits on (`base`), on one machine, and fails +# when `head` is slower than the limits allow. The sides are measured +# interleaved, a pass each per round, because measuring one after the other +# lets machine drift read as a code change. + +name: Benchmarks + +concurrency: + group: ${{ github.repository }}-${{ github.head_ref || github.sha }}-${{ github.workflow }} + cancel-in-progress: true + +# Every push to `main`, so an unlabelled regression is still pinned to one +# merge. Opt-in on a PR, at half an hour of runner time: add the `performance` +# label, or start it from the Actions tab. +on: + push: + branches: + # A fork whose default branch is named differently has to add it here. + - main + paths-ignore: + - "docs/**" + - "**.md" + - ".github/ISSUE_TEMPLATE/**" + - ".github/pull_request_template.md" + pull_request: + types: [opened, synchronize, reopened, labeled] + workflow_dispatch: + inputs: + rounds: + description: 'Measurement rounds (a full pass of each side per round; keep it even)' + type: string + default: '6' + iterations: + description: 'Iterations per query within a round' + type: string + default: '1' + query_regression: + description: 'Fail if a single query is more than this much slower' + type: string + default: '1.20' + min_delta_ms: + description: 'Never fail a query that got less than this many ms slower' + type: string + default: '25' + total_regression: + description: 'Fail if the total time is more than this much slower' + type: string + default: '1.05' + scale_factor: + description: 'TPC-H scale factor' + type: choice + options: + - '10' + - '1' + default: '10' + profile: + description: 'Cargo profile to build both sides with' + type: choice + options: + - release-nonlto + - release + default: release-nonlto + +permissions: + contents: read + +env: + # `release-nonlto` is `release` without fat LTO, which roughly halves the + # build. Dispatch with `release` for cross-crate inlining effects, or for + # numbers comparable with locally posted `bench.sh` results. + CARGO_PROFILE: ${{ inputs.profile || 'release-nonlto' }} + SCALE_FACTOR: ${{ inputs.scale_factor || '10' }} + # Even, so each side leads the same number of rounds. + ROUNDS: ${{ inputs.rounds || '6' }} + ITERATIONS: ${{ inputs.iterations || '1' }} + QUERY_REGRESSION: ${{ inputs.query_regression || '1.20' }} + TOTAL_REGRESSION: ${{ inputs.total_regression || '1.05' }} + # A 1.20x swing on a 20ms query is 4ms, below what a shared runner resolves. + MIN_DELTA_MS: ${{ inputs.min_delta_ms || '25' }} + # `benchmark_runner` finds `sql_benchmarks` through the CARGO_MANIFEST_DIR + # baked in at compile time, so its tree has to sit at the same absolute path + # in the job that builds it and the job that runs it -- hence a fixed root. + BENCH_ROOT: /tmp/df-bench + # Same cargo network settings as .github/actions/setup-rust-runtime, without + # its RUSTFLAGS: benchmark binaries are built with the defaults. + CARGO_HTTP_MULTIPLEXING: "false" + CARGO_NET_RETRY: "10" + CARGO_HTTP_RETRY: "10" + +jobs: + # Resolved once, so the three jobs below cannot disagree about what "base" is. + resolve: + name: resolve base commit + # Every event except `pull_request` runs unconditionally; a PR needs the label. + if: github.event_name != 'pull_request' || contains(github.event.pull_request.labels.*.name, 'performance') + runs-on: ${{ vars.USE_RUNS_ON == 'true' && format('runs-on={0},family=c8a+m8a,cpu=2,image=ubuntu24-full-x64,extras=s3-cache,tag=datafusion', github.run_id) || 'ubuntu-latest' }} + timeout-minutes: 15 + outputs: + base_sha: ${{ steps.base.outputs.sha }} + steps: + - uses: runs-on/action@46910bf61b41721b0579f237e186afb35477007a # v2.3.0 + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + # Depth 2 reaches the first parent, which is the base in both cases. + fetch-depth: 2 + - name: Resolve base commit + id: base + env: + BASE_REF: ${{ github.event.pull_request.base.ref || github.event.repository.default_branch }} + run: | + set -euo pipefail + if [ "$GITHUB_EVENT_NAME" = "push" ]; then + # A merge that just landed: its first parent is the branch as it + # was before, whether the merge was squashed or not. + base_sha=$(git rev-parse "HEAD^1") + echo "push to $GITHUB_REF_NAME, comparing its tip against the parent" + elif [ "$(git rev-list --parents -n 1 HEAD | wc -w)" -ge 3 ]; then + # HEAD is the PR merged into the base branch, so its first parent + # is the commit this PR would land on. + base_sha=$(git rev-parse "HEAD^1") + else + # Manual run on a branch: compare it as-is against the base tip. + git fetch --no-tags --depth 1 origin "$BASE_REF" + base_sha=$(git rev-parse FETCH_HEAD) + echo "HEAD is not a merge commit, comparing it as-is against $BASE_REF" + fi + echo "sha=${base_sha}" >> "$GITHUB_OUTPUT" + echo "base: ${base_sha} $(git log -1 --format=%s "${base_sha}")" + echo "candidate: $(git rev-parse HEAD) $(git log -1 --format=%s HEAD)" + + # One runner per side, so the two builds really do run at the same time. + build: + name: build ${{ matrix.side }} runner + needs: resolve + runs-on: ${{ vars.USE_RUNS_ON == 'true' && format('runs-on={0},family=c8a+m8a,cpu=32,image=ubuntu24-full-x64,extras=s3-cache,disk=large,tag=datafusion', github.run_id) || 'ubuntu-latest' }} + timeout-minutes: 90 + strategy: + # No point in building one side if the other one is broken. + fail-fast: true + matrix: + side: [base, head] + steps: + - uses: runs-on/action@46910bf61b41721b0579f237e186afb35477007a # v2.3.0 + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + fetch-depth: 2 + - name: Free Disk Space (Ubuntu) + # A release build needs this on `ubuntu-latest`'s 14GB, but not on the + # RunsOn runner's `disk=large`, where it is two wasted minutes. + if: vars.USE_RUNS_ON != 'true' + uses: jlumbroso/free-disk-space@54081f138730dfa15788a46383842cd2f914a1be # v1.3.1 + - name: Install Rust + run: | + if ! command -v rustup > /dev/null; then + curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y --default-toolchain none + echo "$HOME/.cargo/bin" >> "$GITHUB_PATH" + export PATH="$HOME/.cargo/bin:$PATH" + fi + # installs the channel pinned by rust-toolchain.toml + rustup toolchain install + - name: Check out the ${{ matrix.side }} tree + env: + SIDE: ${{ matrix.side }} + BASE_SHA: ${{ needs.resolve.outputs.base_sha }} + run: | + if [ "$SIDE" = "base" ]; then + git worktree add --detach "$BENCH_ROOT/$SIDE" "$BASE_SHA" + else + git worktree add --detach "$BENCH_ROOT/$SIDE" HEAD + fi + - name: Cache the dependency build + # A cold build is fourteen minutes, mostly dependencies neither side + # changed. One shared key covers both sides; only pushes write it. + uses: Swatinem/rust-cache@6323deb102c322ba6fcbdcafc7e3dddab59af2b6 # v2.9.2 + with: + workspaces: ${{ env.BENCH_ROOT }}/${{ matrix.side }} + shared-key: benchmark-${{ env.CARGO_PROFILE }} + save-if: ${{ github.event_name == 'push' }} + - name: Build benchmark_runner + env: + SIDE: ${{ matrix.side }} + run: | + cd "$BENCH_ROOT/$SIDE" + cargo build --profile "$CARGO_PROFILE" -p datafusion-benchmarks --bin benchmark_runner + cp "target/$CARGO_PROFILE/benchmark_runner" "$RUNNER_TEMP/benchmark_runner" + - name: Upload benchmark_runner + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: benchmark_runner-${{ matrix.side }} + path: ${{ runner.temp }}/benchmark_runner + retention-days: 1 + + # Both sides are measured here, interleaved, on this one machine. + benchmark: + name: TPC-H (head vs base) + needs: [resolve, build] + runs-on: ${{ vars.USE_RUNS_ON == 'true' && format('runs-on={0},family=c8a+m8a,cpu=16,image=ubuntu24-full-x64,extras=s3-cache,disk=large,tag=datafusion', github.run_id) || 'ubuntu-latest' }} + timeout-minutes: 90 + steps: + - uses: runs-on/action@46910bf61b41721b0579f237e186afb35477007a # v2.3.0 + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + fetch-depth: 2 + - name: Configure paths + run: | + echo "DATA_DIR=$RUNNER_TEMP/bench-data" >> "$GITHUB_ENV" + echo "RESULTS_DIR=$RUNNER_TEMP/results" >> "$GITHUB_ENV" + - name: Check out both trees + env: + BASE_SHA: ${{ needs.resolve.outputs.base_sha }} + run: | + # The same paths the build jobs used, so each binary finds the + # `sql_benchmarks` directory of the tree it was built from. + git worktree add --detach "$BENCH_ROOT/base" "$BASE_SHA" + git worktree add --detach "$BENCH_ROOT/head" HEAD + - name: Download base benchmark_runner + uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 + with: + name: benchmark_runner-base + path: ${{ runner.temp }}/bin/base + - name: Download head benchmark_runner + uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 + with: + name: benchmark_runner-head + path: ${{ runner.temp }}/bin/head + - name: Check both runners see their queries + run: | + # Artifacts do not carry the executable bit, and a binary whose tree + # is missing would discover no benchmark at all -- fail here. + for side in base head; do + binary="$RUNNER_TEMP/bin/$side/benchmark_runner" + chmod +x "$binary" + if ! "$binary" --list | grep -qE '^[[:space:]]+tpch[[:space:]]'; then + echo "::error::the $side runner does not see the tpch suite at $BENCH_ROOT/$side/benchmarks" + "$binary" --list + exit 1 + fi + done + - name: Install uv + uses: astral-sh/setup-uv@c771a70e6277c0a99b617c7a806ffedaca235ff9 # v9.0.0 + - name: Generate TPC-H data + # Same generator settings as `bench.sh data tpch`, inlined because that + # path also pulls the expected answers through `docker run -it`, which + # has no TTY here. From the PyPI wheel, so no Rust toolchain is needed; + # `parquet` is a subcommand because 3.0.0 deprecated `--format`. + run: | + mkdir -p "$DATA_DIR/tpch_sf$SCALE_FACTOR" "$RESULTS_DIR/base" "$RESULTS_DIR/head" + uv tool run --from 'tpchgen-cli==3.0.0' tpchgen-cli parquet \ + --scale-factor "$SCALE_FACTOR" \ + --compression 'ZSTD(1)' \ + --parts=1 \ + --output-dir "$DATA_DIR/tpch_sf$SCALE_FACTOR" + du -sh "$DATA_DIR/tpch_sf$SCALE_FACTOR" + df -h "$DATA_DIR" + - name: Describe the machine + # GitHub withholds `vars` from fork pull requests, so `USE_RUNS_ON` + # reads as empty and they fall back to a 4-vCPU `ubuntu-latest`. Pushes + # to `main` and manual dispatches do get the 16-vCPU runner. + run: | + echo "cpus: $(nproc)" + lscpu | grep -E '^(Model name|Socket|Core|Thread|CPU\(s\)):' || true + free -h + if [ "$(nproc)" -lt 8 ]; then + echo "::warning::running on $(nproc) CPUs, not the runner asked for; expect a high noise floor. Fork pull requests cannot reach the larger one -- the run on \`main\` after the merge is authoritative." + fi + # Each side runs from its own tree and reads the one generated dataset. + - name: Benchmark both sides, interleaved + run: | + set -euo pipefail + run_side() { + local side="$1" output="$2" + cd "$BENCH_ROOT/$side/benchmarks" + "$RUNNER_TEMP/bin/$side/benchmark_runner" tpch \ + --scale-factor "$SCALE_FACTOR" \ + --format parquet \ + --iterations "$ITERATIONS" \ + --path "$DATA_DIR" \ + --output "$output" + } + # Pull the data into the page cache, which is all a warmup can carry + # between rounds -- each round is a fresh process. + echo "::group::warm the page cache" + find "$DATA_DIR" -type f -exec cat {} + > /dev/null + echo "::endgroup::" + for round in $(seq 1 "$ROUNDS"); do + # Alternate the leader, so each side pays the first-position cost + # the same number of times. + if [ $((round % 2)) -eq 1 ]; then order="base head"; else order="head base"; fi + for side in $order; do + echo "::group::round $round: $side" + # Zero-padded, because compare.py pairs the two sides' rounds in + # sorted filename order. + run_side "$side" "$(printf '%s/%s/round%02d.json' "$RESULTS_DIR" "$side" "$round")" + echo "::endgroup::" + done + done + - name: Compare + run: | + set -uo pipefail + status=0 + uv run --no-project --with rich python3 benchmarks/compare.py \ + "$RESULTS_DIR/base" \ + "$RESULTS_DIR/head" \ + --fail-threshold "$QUERY_REGRESSION" \ + --fail-total-threshold "$TOTAL_REGRESSION" \ + --fail-min-delta-ms "$MIN_DELTA_MS" \ + > "$RESULTS_DIR/comparison.txt" 2>&1 || status=$? + cat "$RESULTS_DIR/comparison.txt" + { + echo "### TPC-H SF$SCALE_FACTOR: \`head\` (${{ github.event_name == 'push' && 'the merge that just landed' || 'this PR, merged into the base branch' }}) vs \`base\` (${{ needs.resolve.outputs.base_sha }})" + echo + echo "\`$ROUNDS\` rounds of \`$ITERATIONS\` iteration(s), the two sides interleaved and their order alternated per round." + echo "A query fails the gate when the median of its per-round ratios is above \`${QUERY_REGRESSION}x\`, the regression costs at least \`${MIN_DELTA_MS}ms\`, and it is larger than the spread the base side showed against itself." + echo "The total time fails above \`${TOTAL_REGRESSION}x\` under the same noise floor." + echo "Both sides built with the \`$CARGO_PROFILE\` profile on \`$(nproc)\` CPUs." + echo + echo '```' + cat "$RESULTS_DIR/comparison.txt" + echo '```' + } >> "$GITHUB_STEP_SUMMARY" + if [ "$status" -ne 0 ]; then + echo "::error::TPC-H SF$SCALE_FACTOR got slower than the configured limits allow, see the job summary" + fi + exit "$status" + - name: Upload benchmark results + if: always() + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: tpch-comparison + path: ${{ runner.temp }}/results + retention-days: 7 diff --git a/benchmarks/README.md b/benchmarks/README.md index f357ff4da58ce..2c97777f0cac2 100644 --- a/benchmarks/README.md +++ b/benchmarks/README.md @@ -257,6 +257,67 @@ git pull Note: if `gh` is installed, you can also run `gh pr checkout $PR_NUMBER` instead of `git fetch upstream pull/$PR_NUMBER/head:pr-$PR_NUMBER` +### In CI + +The `Benchmarks` workflow (`.github/workflows/benchmark.yml`) runs TPC-H SF10 +for a candidate commit (`head`) and for the commit it sits on (`base`), on one +machine, and fails when `head` is slower than the limits allow. SF10 rather +than SF1, because SF1 queries finish in milliseconds where runner noise +dominates. + +It runs on every push to `main`, so a regression no pull request measured is +still pinned to the one merge that introduced it. On a pull request it is +opt-in, at half an hour of runner time: add the `performance` label, or start it +from the Actions tab, where the scale factor, round and iteration counts, the +regression limits and the cargo profile can be overridden. A failure on `main` +blocks nothing, so read it as a bisect already done for you. + +Three jobs: one resolves the base commit, two build a `benchmark_runner` each +(a runner per side, so the builds are simultaneous), and the last measures both +binaries. Both sides use the `release-nonlto` profile, which roughly halves the +build; dispatch with `release` for cross-crate inlining effects or for numbers +comparable with locally posted `bench.sh` results. + +The measuring job **interleaves** the sides: a pass of one, a pass of the other, +six times, alternating which leads. This matters more than any choice of +statistic -- measured in two blocks, anything that drifts between them moves one +side only and reads as a code change. The round count is even so each side leads +equally; dispatch more rounds to tighten a marginal verdict. + +Which machine the run lands on follows from the trigger. GitHub withholds `vars` +from fork pull requests, so `vars.USE_RUNS_ON` reads as empty there and the run +falls back to a 4-vCPU `ubuntu-latest`, as the rest of `rust.yml` also does; a +push to `main` or a manual dispatch gets the 16-vCPU runner. So a labelled fork +pull request is a coarse signal and the `main` run after the merge is the +measurement. On that faster machine an SF10 query that takes 300ms on four vCPUs +takes under 100ms, and a 20% regression on it falls inside the 25ms floor below, +so dispatch a smaller `min_delta_ms` when the shortest queries are the ones in +question. + +If you edit the workflow, note that `benchmark_runner` locates `sql_benchmarks` +through the `CARGO_MANIFEST_DIR` baked in at compile time, so each side's +checkout has to sit at the same absolute path in the job that builds it and the +job that runs it -- hence the fixed `/tmp` root, and the check that each binary +still sees the `tpch` suite before measuring. The comparison table goes to the +job summary, and every round's JSON plus the table are uploaded as the +`tpch-comparison` artifact. + +Runners are shared machines, so treat the numbers as a signal rather than a +measurement. A query has to clear three bars to fail the gate: + +- the **median of its per-round ratios** is above `1.20x`, so one slow pass on + either side cannot decide the verdict +- the regression costs at least **25ms**, which is what keeps an SF1 run from + failing on its short queries +- it is larger than the **spread the base side showed against itself**, since a + query whose own baseline moved 26% between rounds cannot support a 25% verdict + +The total time is gated at `1.05x` under the same noise floor. Anything that +clears the first bar but not the others is printed under "Not counted against +the gate" rather than dropped. Local runs on quiet hardware remain the way to +confirm a result, and a PR that changes the TPC-H benchmark files themselves is +measured against its own queries, which makes the comparison advisory. + ### Running Benchmarks Manually Assuming data is in the `data` directory, the `tpch` benchmark can be run with a command like this: @@ -297,6 +358,28 @@ $ cargo run --release --bin tpch -- benchmark datafusion --iterations 5 --path . ./compare.py /tmp/output_main/tpch.json /tmp/output_branch/tpch.json ``` +To use `compare.py` as a pass/fail gate (this is what CI does), pass one or both +of the regression limits. Without them it only prints the comparison and always +exits successfully. + +```shell +# exit non-zero if a single query is more than 20% slower and at least 25ms +# slower, or if the total time is more than 5% slower +./compare.py /tmp/output_main/tpch.json /tmp/output_branch/tpch.json \ + --fail-threshold 1.20 --fail-total-threshold 1.05 --fail-min-delta-ms 25 +``` + +Either path can also be a *directory* of summary files, one per measurement +round, which is how CI runs it: the gate then rules on the median of the +per-round ratios and reports how much each side varied against itself. Rounds +are paired in sorted filename order, so name them `round01.json` and so on. + +```shell +# six rounds per side, measured alternately rather than one side at a time +./compare.py /tmp/output_main /tmp/output_branch \ + --fail-threshold 1.20 --fail-total-threshold 1.05 --fail-min-delta-ms 25 +``` + This will produce output like: ``` diff --git a/benchmarks/bench.sh b/benchmarks/bench.sh index df9b7f6c94f16..2475457426246 100755 --- a/benchmarks/bench.sh +++ b/benchmarks/bench.sh @@ -668,7 +668,7 @@ data_tpch() { # check if tpchgen-cli is installed if ! command -v tpchgen-cli &> /dev/null then - echo "tpchgen-cli could not be found, please install it via 'cargo install tpchgen-cli'" + echo "tpchgen-cli could not be found, please install it via 'cargo install tpchgen-cli' (3.0 or newer)" exit 1 fi @@ -689,7 +689,7 @@ data_tpch() { echo " parquet files exist ($FILE exists)." else echo " creating parquet files using tpchgen-cli ..." - tpchgen-cli --scale-factor "${SCALE_FACTOR}" --format parquet --parquet-compression='ZSTD(1)' --parts=1 --output-dir "${TPCH_DIR}" + tpchgen-cli parquet --scale-factor "${SCALE_FACTOR}" --compression='ZSTD(1)' --parts=1 --output-dir "${TPCH_DIR}" fi return fi @@ -701,7 +701,7 @@ data_tpch() { echo " csv files exist ($FILE exists)." else echo " creating csv files using tpchgen-cli binary ..." - tpchgen-cli --scale-factor "${SCALE_FACTOR}" --format csv --parts=1 --output-dir "${TPCH_DIR}/csv" + tpchgen-cli csv --scale-factor "${SCALE_FACTOR}" --parts=1 --output-dir "${TPCH_DIR}/csv" fi return fi @@ -1308,7 +1308,7 @@ data_sort_pushdown() { TEMP_DIR="${DATA_DIR}/sort_pushdown_temp" mkdir -p "${TEMP_DIR}" "${SORT_PUSHDOWN_DIR}" - tpchgen-cli --scale-factor 1 --format parquet --parquet-compression='ZSTD(1)' --parts=3 --output-dir "${TEMP_DIR}" + tpchgen-cli parquet --scale-factor 1 --compression='ZSTD(1)' --parts=3 --output-dir "${TEMP_DIR}" # Rename: reverse alphabetical order vs key order mv "${TEMP_DIR}/lineitem/lineitem.3.parquet" "${SORT_PUSHDOWN_DIR}/a_part3.parquet" diff --git a/benchmarks/compare.py b/benchmarks/compare.py index 9ad1de980abe8..ab7f4db21b903 100755 --- a/benchmarks/compare.py +++ b/benchmarks/compare.py @@ -22,7 +22,7 @@ import json import math from dataclasses import dataclass -from typing import Dict, List, Any +from typing import Dict, List, Any, Sequence from pathlib import Path from argparse import ArgumentParser @@ -34,6 +34,34 @@ raise +def median(values: Sequence[float]) -> float: + """The median of a non-empty sequence.""" + ordered = sorted(values) + middle = len(ordered) // 2 + + if len(ordered) % 2 == 1: + return ordered[middle] + + return (ordered[middle - 1] + ordered[middle]) / 2 + + +def geometric_mean(values: Sequence[float]) -> float: + """The geometric mean of a non-empty sequence of positive numbers.""" + return math.exp(sum(math.log(value) for value in values) / len(values)) + + +def upward_spread(values: Sequence[float]) -> float: + """`(median - min) / min`: the slowdown a side already shows against itself. + + The median rather than the max, so one slow round does not set the floor. + """ + fastest = min(values) + if fastest <= 0: + return 0.0 + + return (median(values) - fastest) / fastest + + @dataclass class QueryResult: elapsed: float @@ -46,7 +74,9 @@ def load_from(cls, data: Dict[str, Any]) -> QueryResult: @dataclass class QueryRun: - query: int + # A number for benchmarks that identify queries by index, a name such as + # "tpch/Q01/sf1" for the ones run by `benchmark_runner` + query: int | str iterations: List[QueryResult] start_time: int success: bool = True @@ -60,6 +90,13 @@ def load_from(cls, data: Dict[str, Any]) -> QueryRun: success=data.get("success", True), ) + @property + def label(self) -> str: + """Row label: "Q3" for numeric query ids, the id itself for named ones.""" + query = str(self.query) + + return f"Q{query}" if query.isdigit() else query + @property def min_execution_time(self) -> float: assert len(self.iterations) >= 1 @@ -145,26 +182,150 @@ def load_from_file(cls, path: Path) -> BenchmarkRun: return cls.load_from(json.load(f)) +@dataclass +class Side: + """One side of the comparison: every round measured for it. + + A single summary file is one round; a directory is one round per file, in + sorted order. + """ + + header: str + rounds: List[BenchmarkRun] + + @classmethod + def load(cls, path: Path) -> Side: + if path.is_dir(): + paths = sorted(path.glob("*.json")) + if not paths: + raise SystemExit(f"no *.json summary files in {path}") + # A directory is named after the side it holds ("base", "pr"), + # a file is named after the run and lives in such a directory. + header = path.name + else: + paths = [path] + header = path.parent.name + + rounds = [BenchmarkRun.load_from_file(round_path) for round_path in paths] + + # Rounds are indexed by position below, so a round that measured a + # different set of queries has to be caught rather than mispaired. + queries = [query.query for query in rounds[0].queries] + for round_path, round in zip(paths[1:], rounds[1:]): + if [query.query for query in round.queries] != queries: + raise SystemExit( + f"{round_path} measured different queries than {paths[0]}" + ) + + return cls(header, rounds) + + @property + def queries(self) -> List[QueryRun]: + """The first round's queries, for labels and ordering.""" + return self.rounds[0].queries + + def merged(self, index: int) -> QueryRun: + """One query's rounds collapsed into a single run, for display.""" + runs = [round.queries[index] for round in self.rounds] + + return QueryRun( + query=runs[0].query, + iterations=[ + iteration for run in runs for iteration in run.iterations + ], + start_time=runs[0].start_time, + success=all(run.success for run in runs), + ) + + def per_round(self, index: int) -> List[float]: + """One query's fastest time in each round.""" + return [round.queries[index].min_execution_time for round in self.rounds] + + +@dataclass +class QueryComparison: + """One query's verdict, and the numbers behind it.""" + + label: str + # comparison / baseline in each round, paired by round + ratios: List[float] + # median of `ratios`: the estimate the gate rules on + ratio: float + # how much the baseline varies against itself, as a relative slowdown + noise: float + # absolute regression in ms, on the per-round medians + delta_ms: float + # the limit asked for on the command line + configured_limit: float + # the limit the ratio was actually held to: the configured one, raised to + # the noise floor when the baseline is too unstable to support it + limit: float + + @property + def regressed(self) -> bool: + return self.ratio > self.limit + + @property + def inconclusive(self) -> bool: + """Above the configured limit, but not above this query's noise.""" + return not self.regressed and self.ratio > self.configured_limit + + def summary(self) -> str: + spread = "" + if len(self.ratios) > 1: + spread = f" (rounds {min(self.ratios):.2f}-{max(self.ratios):.2f})" + + limit = f"limit {self.limit:.2f}x" + if self.limit > self.configured_limit: + limit += ( + f", raised from {self.configured_limit:.2f}x " + f"by a {self.noise:.0%} noise floor" + ) + + return ( + f"{self.label} is {self.ratio:.2f}x slower{spread}, " + f"+{self.delta_ms:.0f}ms ({limit})" + ) + + def compare( baseline_path: Path, comparison_path: Path, noise_threshold: float, detailed: bool, -) -> None: - baseline = BenchmarkRun.load_from_file(baseline_path) - comparison = BenchmarkRun.load_from_file(comparison_path) + fail_threshold: float | None = None, + fail_total_threshold: float | None = None, + min_delta_ms: float = 0.0, +) -> int: + """Print the comparison and return the process exit code. + + The exit code is non-zero only when a `fail_*_threshold` is given and the + comparison run is slower than it allows. + """ + baseline = Side.load(baseline_path) + comparison = Side.load(comparison_path) console = Console(width=200) - # use basename as the column names - baseline_header = baseline_path.parent.name - comparison_header = comparison_path.parent.name + rounds = min(len(baseline.rounds), len(comparison.rounds)) + if len(baseline.rounds) != len(comparison.rounds): + console.print( + f"[yellow]{baseline.header} has {len(baseline.rounds)} round(s) and " + f"{comparison.header} has {len(comparison.rounds)}; comparing the " + f"first {rounds} of each[/yellow]" + ) + + multi_round = rounds > 1 table = Table(show_header=True, header_style="bold magenta") table.add_column("Query", style="dim", no_wrap=True) - table.add_column(baseline_header, justify="right", style="dim", no_wrap=True) - table.add_column(comparison_header, justify="right", style="dim", no_wrap=True) + table.add_column(baseline.header, justify="right", style="dim", no_wrap=True) + table.add_column(comparison.header, justify="right", style="dim", no_wrap=True) table.add_column("Change", justify="right", style="dim", no_wrap=True) + if multi_round: + # What the gate rules on, next to the fastest-run ratio in `Change`. + table.add_column("Per-round", justify="right", style="dim", no_wrap=True) + table.add_column("Noise", justify="right", style="dim", no_wrap=True) faster_count = 0 slower_count = 0 @@ -172,30 +333,67 @@ def compare( failure_count = 0 total_baseline_time = 0 total_comparison_time = 0 - - for baseline_result, comparison_result in zip(baseline.queries, comparison.queries): + # Per-round totals, so the total is gated the same paired way a query is + baseline_totals = [0.0] * rounds + comparison_totals = [0.0] * rounds + comparisons: List[QueryComparison] = [] + new_failures: List[str] = [] + + for index, (baseline_result, comparison_result) in enumerate( + zip(baseline.queries, comparison.queries) + ): assert baseline_result.query == comparison_result.query - base_failed = not baseline_result.success - comp_failed = not comparison_result.success + baseline_merged = baseline.merged(index) + comparison_merged = comparison.merged(index) + + base_failed = not baseline_merged.success + comp_failed = not comparison_merged.success # If a query fails, its execution time is excluded from the performance comparison if base_failed or comp_failed: change_text = "incomparable" failure_count += 1 - table.add_row( - f"Q{baseline_result.query}", - "FAIL" if base_failed else baseline_result.execution_time_report(detailed)[1], - "FAIL" if comp_failed else comparison_result.execution_time_report(detailed)[1], + if comp_failed and not base_failed: + new_failures.append(baseline_merged.label) + row = [ + baseline_merged.label, + "FAIL" if base_failed else baseline_merged.execution_time_report(detailed)[1], + "FAIL" if comp_failed else comparison_merged.execution_time_report(detailed)[1], change_text, - ) + ] + table.add_row(*(row + ["", ""] if multi_round else row)) continue - baseline_value, baseline_text = baseline_result.execution_time_report(detailed) - comparison_value, comparison_text = comparison_result.execution_time_report(detailed) + baseline_value, baseline_text = baseline_merged.execution_time_report(detailed) + comparison_value, comparison_text = comparison_merged.execution_time_report(detailed) total_baseline_time += baseline_value total_comparison_time += comparison_value + baseline_rounds = baseline.per_round(index)[:rounds] + comparison_rounds = comparison.per_round(index)[:rounds] + for round_index in range(rounds): + baseline_totals[round_index] += baseline_rounds[round_index] + comparison_totals[round_index] += comparison_rounds[round_index] + + ratios = [ + comparison_round / baseline_round + for baseline_round, comparison_round in zip(baseline_rounds, comparison_rounds) + ] + noise = upward_spread(baseline_rounds) + configured_limit = fail_threshold if fail_threshold is not None else math.inf + comparisons.append( + QueryComparison( + label=baseline_merged.label, + ratios=ratios, + ratio=median(ratios), + noise=noise, + delta_ms=median(comparison_rounds) - median(baseline_rounds), + limit=max(configured_limit, 1.0 + noise), + configured_limit=configured_limit, + ) + ) + change = comparison_value / baseline_value if (1.0 - noise_threshold) <= change <= (1.0 + noise_threshold): @@ -208,12 +406,16 @@ def compare( change_text = f"{change:.2f}x slower" slower_count += 1 - table.add_row( - f"Q{baseline_result.query}", + row = [ + baseline_merged.label, baseline_text, comparison_text, change_text, - ) + ] + if multi_round: + row.append(f"{median(ratios):.2f}x ({min(ratios):.2f}-{max(ratios):.2f})") + row.append(f"±{noise:.0%}") + table.add_row(*row) console.print(table) @@ -225,15 +427,42 @@ def compare( if len(comparison.queries) - failure_count > 0: avg_comparison_time = total_comparison_time / (len(comparison.queries) - failure_count) + total_change = ( + total_comparison_time / total_baseline_time if total_baseline_time else 1.0 + ) + + total_ratios = [ + comparison_total / baseline_total + for baseline_total, comparison_total in zip(baseline_totals, comparison_totals) + if baseline_total + ] or [1.0] + total_noise = upward_spread(baseline_totals) if baseline_totals[0] else 0.0 + # Summary table summary_table = Table(show_header=True, header_style="bold magenta") summary_table.add_column("Benchmark Summary", justify="left", style="dim") summary_table.add_column("", justify="right", style="dim") - summary_table.add_row(f"Total Time ({baseline_header})", f"{total_baseline_time:.2f}ms") - summary_table.add_row(f"Total Time ({comparison_header})", f"{total_comparison_time:.2f}ms") - summary_table.add_row(f"Average Time ({baseline_header})", f"{avg_baseline_time:.2f}ms") - summary_table.add_row(f"Average Time ({comparison_header})", f"{avg_comparison_time:.2f}ms") + summary_table.add_row("Rounds", str(rounds)) + summary_table.add_row( + "Iterations per round", + str(len(baseline.queries[0].iterations)) if baseline.queries else "0", + ) + summary_table.add_row("CPUs", str(baseline.rounds[0].context.num_cpus)) + summary_table.add_row(f"Total Time ({baseline.header})", f"{total_baseline_time:.2f}ms") + summary_table.add_row(f"Total Time ({comparison.header})", f"{total_comparison_time:.2f}ms") + summary_table.add_row(f"Average Time ({baseline.header})", f"{avg_baseline_time:.2f}ms") + summary_table.add_row(f"Average Time ({comparison.header})", f"{avg_comparison_time:.2f}ms") + summary_table.add_row("Total Change", f"{total_change:.2f}x") + if multi_round: + summary_table.add_row("Total Change (per-round median)", f"{median(total_ratios):.2f}x") + summary_table.add_row("Geometric Mean of Query Ratios", f"{geometric_mean([c.ratio for c in comparisons]):.2f}x" if comparisons else "n/a") + summary_table.add_row( + f"Noise Floor ({baseline.header}, median / worst query)", + f"{median([c.noise for c in comparisons]):.1%} / {max([c.noise for c in comparisons]):.1%}" + if comparisons + else "n/a", + ) summary_table.add_row("Queries Faster", str(faster_count)) summary_table.add_row("Queries Slower", str(slower_count)) summary_table.add_row("Queries with No Change", str(no_change_count)) @@ -241,18 +470,118 @@ def compare( console.print(summary_table) + return report_regressions( + console, + baseline.header, + comparison.header, + comparisons, + new_failures, + median(total_ratios), + total_noise, + fail_threshold, + fail_total_threshold, + min_delta_ms, + ) + + +def report_regressions( + console: Console, + baseline_header: str, + comparison_header: str, + comparisons: List[QueryComparison], + new_failures: List[str], + total_ratio: float, + total_noise: float, + fail_threshold: float | None, + fail_total_threshold: float | None, + min_delta_ms: float, +) -> int: + """Report against the configured limits, returning the process exit code. + + A query fails only when its slowdown clears three bars: the configured + limit, a minimum absolute cost in milliseconds, and the noise floor the + baseline showed against itself. Clearing the first but not the others is + reported as inconclusive -- a fact about the run, not about the change. + """ + if fail_threshold is None and fail_total_threshold is None: + return 0 + + problems: List[str] = [] + notes: List[str] = [] + + if fail_threshold is not None: + for query in comparisons: + if not query.regressed: + if query.inconclusive: + notes.append( + f"{query.label} is {query.ratio:.2f}x slower, within the " + f"{query.noise:.0%} spread {baseline_header} showed against " + f"itself -- too noisy to call" + ) + continue + + if query.delta_ms < min_delta_ms: + notes.append( + f"{query.label} is {query.ratio:.2f}x slower, but only " + f"+{query.delta_ms:.0f}ms (below the {min_delta_ms:.0f}ms floor)" + ) + continue + + problems.append(query.summary()) + + if fail_total_threshold is not None: + total_limit = max(fail_total_threshold, 1.0 + total_noise) + if total_ratio > total_limit: + problems.append( + f"total time is {total_ratio:.2f}x slower (limit {total_limit:.2f}x)" + ) + elif total_ratio > fail_total_threshold: + notes.append( + f"total time is {total_ratio:.2f}x slower, within the " + f"{total_noise:.0%} spread {baseline_header} showed against itself" + ) + + # A query that only fails on one side is excluded from the timings above, + # so it would otherwise pass the gate unnoticed. + problems.extend( + f"{label} failed in {comparison_header} but not in {baseline_header}" + for label in new_failures + ) + + if notes: + console.print("Not counted against the gate:") + for note in notes: + console.print(f" - {note}", markup=False) + + if not problems: + console.print( + f"No regression: {comparison_header} is within the configured limits " + f"of {baseline_header}." + ) + return 0 + + console.print(f"Regression: {comparison_header} is slower than {baseline_header}") + for problem in problems: + console.print(f" - {problem}", markup=False) + + return 1 + + def main() -> None: parser = ArgumentParser() compare_parser = parser compare_parser.add_argument( "baseline_path", type=Path, - help="Path to the baseline summary file.", + help="Path to the baseline summary file, or to a directory holding one " + "summary file per round.", ) compare_parser.add_argument( "comparison_path", type=Path, - help="Path to the comparison summary file.", + help="Path to the comparison summary file, or to a directory holding " + "one summary file per round. Rounds are paired with the baseline's in " + "sorted filename order.", ) compare_parser.add_argument( "--noise-threshold", @@ -260,6 +589,28 @@ def main() -> None: default=0.05, help="The threshold for statistically insignificant results (+/- %5).", ) + compare_parser.add_argument( + "--fail-threshold", + type=float, + default=None, + help="Exit non-zero if any single query is slower than this ratio " + "(e.g. 1.2 for 20%% slower). Off by default.", + ) + compare_parser.add_argument( + "--fail-total-threshold", + type=float, + default=None, + help="Exit non-zero if the total time is slower than this ratio " + "(e.g. 1.05 for 5%% slower). Off by default.", + ) + compare_parser.add_argument( + "--fail-min-delta-ms", + type=float, + default=0.0, + help="Never fail on a query that got less than this many milliseconds " + "slower, however large the ratio. Keeps short queries, where a large " + "relative change is a small absolute one, out of the gate.", + ) compare_parser.add_argument( "--detailed", action=argparse.BooleanOptionalAction, @@ -269,7 +620,17 @@ def main() -> None: options = parser.parse_args() - compare(options.baseline_path, options.comparison_path, options.noise_threshold, options.detailed) + raise SystemExit( + compare( + options.baseline_path, + options.comparison_path, + options.noise_threshold, + options.detailed, + options.fail_threshold, + options.fail_total_threshold, + options.fail_min_delta_ms, + ) + )