Skip to content

Commit 049b7e3

Browse files
evertlammertsevertlammerts
committed
more benchmarking
1 parent 4f24ed7 commit 049b7e3

20 files changed

Lines changed: 1177 additions & 170 deletions

‎.github/workflows/codspeed.yml‎

Lines changed: 99 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,42 @@
1-
# Perf-regression benchmarks via CodSpeed in instruction-count (simulation) mode: deterministic, so the whole
2-
# suite is gate-able (no walltime noise, no gated/informational split).
1+
# Perf-regression benchmarks: instruction-count (Callgrind) gating against a COMMITTED baseline.
32
#
4-
# TOKENLESS: the token is only for uploading to the CodSpeed dashboard. Without it the action still runs every
5-
# benchmark and reports counts in the job log. For the hosted gate later, create a CodSpeed project and rely on
6-
# the OIDC id-token permission below (public repo), or add a CODSPEED_TOKEN secret and pass token: to the action.
3+
# NO CodSpeed account/dashboard/token/runner. pytest-codspeed's hooks call callgrind_dump_stats_at(<uri>) per
4+
# benchmark, so a self-hosted `valgrind --tool=callgrind` run writes one dump per benchmark, headed by
5+
# `desc: Trigger: Client Request: <uri>` with the count on `totals:` (events: Ir). benchmarks/compare_baseline.py
6+
# parses those dumps and diffs each benchmark against benchmarks/baseline.json (the committed instruction-count
7+
# baseline). Counts are near-deterministic under Callgrind with PYTHONHASHSEED pinned (~0.1% noise observed;
8+
# often bit-identical), so a 5% default gate threshold sits far above noise. Validated on a Linux+valgrind box.
79
#
8-
# Not yet run in CI; the build mirrors the dev build (CLAUDE.md) and will need a shakeout. Valgrind is slow
9-
# (~20-50x); trim the largest-N benchmarks if the suite is too slow.
10+
# TRIGGERS: nightly `schedule` + manual `workflow_dispatch`. No pull_request/push (and no `paths:` -- neither
11+
# schedule nor dispatch honors it). A dispatch on a feature branch compares that branch's benchmark counts vs the
12+
# baseline.json committed on the branch (i.e. main's baseline), answering "did my branch regress vs main".
13+
#
14+
# MODES (workflow_dispatch input `regen`):
15+
# regen=false (default) -> COMPARE: run + diff vs baseline.json, print a report. REPORT-ONLY for now (never
16+
# fails the job); flip compare_baseline.py to --enforce once trusted.
17+
# regen=true -> REGENERATE: run + write a fresh baseline.json (per-bench counts + provenance meta +
18+
# Option-B binding fractions/auto-move) and upload it as an artifact to commit
19+
# deliberately. Bump benchmarks/requirements-bench.txt in a separate commit FIRST if
20+
# the pins should change, then regen so the baseline matches the committed pins.
21+
#
22+
# The concurrency module is EXCLUDED from the Callgrind sweep: Callgrind serializes threads, so its signal
23+
# (wall-clock GIL contention) is meaningless here; it stays a local walltime tool.
24+
#
25+
# MEMORY MODE (a second Callgrind sweep for O(rows) produce peak-RSS) is DESIGNED but DEFERRED -- see PLAN.md.
26+
#
27+
# Valgrind is slow (~20-50x); timeout-minutes is a conservative guess -- calibrate after the first CI run.
1028

1129
name: Benchmarks
1230

1331
on:
14-
pull_request:
15-
push:
16-
branches: [main]
32+
schedule:
33+
- cron: "0 3 * * *" # nightly at 03:00 UTC
1734
workflow_dispatch:
35+
inputs:
36+
regen:
37+
description: "Regenerate benchmarks/baseline.json (upload as artifact) instead of comparing"
38+
type: boolean
39+
default: false
1840

1941
concurrency:
2042
group: codspeed-${{ github.ref }}
@@ -23,45 +45,101 @@ concurrency:
2345
jobs:
2446
benchmarks:
2547
runs-on: ubuntu-latest
48+
timeout-minutes: 90 # measured: ~25 min Callgrind sweep at BENCH_SCALE=10 (12-core Linux) + cold build ~10 min; margin for CI
2649
permissions:
2750
contents: read
28-
id-token: write # enables tokenless (OIDC) upload once a CodSpeed project is linked; harmless otherwise
51+
env:
52+
PYTHONHASHSEED: "0" # pin hash randomization so dict/struct paths give stable instruction counts (INFRA-6)
53+
CODSPEED_ENV: "1" # activates pytest-codspeed's instrument hooks (the callgrind_dump_stats_at markers)
54+
# env-gated row counts (INFRA-4): shrink the O(rows)/per-row-object benchmarks so the Callgrind sweep fits
55+
# under timeout-minutes. Local runs leave this unset -> full N. Recorded in baseline.json meta.bench_scale;
56+
# a baseline is only comparable to a run at the SAME scale. Calibrated on a 12-core Linux+valgrind box:
57+
# BENCH_SCALE=10 -> ~25 min full sweep, and the Option-B move-list matches full-N (fractions shift slightly
58+
# but stay the same side of the cutoff). Most benches floor at 20k rows (_scale.FLOOR), still row-dominated.
59+
BENCH_SCALE: "10"
2960
steps:
3061
- uses: actions/checkout@v4
3162
with:
3263
submodules: recursive # the DuckDB engine submodule is needed to build
3364
fetch-depth: 0 # setuptools_scm needs history for version detection
3465

66+
- name: Resolve DuckDB submodule SHA
67+
id: duckdb_sha
68+
# used for the sccache key AND passed to compare_baseline.py for the engine-bump guard
69+
run: echo "sha=$(git rev-parse HEAD:external/duckdb)" >> "$GITHUB_OUTPUT"
70+
3571
- name: Install uv
3672
uses: astral-sh/setup-uv@v5
3773
with:
3874
python-version: "3.13"
3975

76+
- name: Install valgrind
77+
run: sudo apt-get update && sudo apt-get install -y valgrind
78+
4079
- name: Cache sccache
4180
uses: actions/cache@v4
4281
with:
4382
path: ~/.cache/sccache
44-
key: sccache-codspeed-${{ hashFiles('external/duckdb') }}
83+
key: sccache-codspeed-${{ steps.duckdb_sha.outputs.sha }}
4584
restore-keys: sccache-codspeed-
4685

4786
- name: Install sccache
4887
run: |
4988
curl -fsSL https://github.com/mozilla/sccache/releases/download/v0.8.2/sccache-v0.8.2-x86_64-unknown-linux-musl.tar.gz \
5089
| tar -xz --strip-components=1 -C /usr/local/bin sccache-v0.8.2-x86_64-unknown-linux-musl/sccache
5190
52-
- name: Build the extension (release) + benchmark deps
91+
- name: Build the extension (release) + pinned benchmark deps
5392
env:
5493
CMAKE_C_COMPILER_LAUNCHER: sccache
5594
CMAKE_CXX_COMPILER_LAUNCHER: sccache
5695
run: |
96+
# step 1: build deps only (needed for --no-build-isolation), no project
5797
uv sync --only-group build --no-install-project -p 3.13
58-
uv sync --no-build-isolation --no-editable --reinstall -p 3.13
59-
# benchmark deps: keep these pinned in lockstep with any baseline you compare against, so the only
60-
# cross-run delta is the binding (numpy/pandas/pyarrow/polars/pytz + the codspeed plugin).
61-
uv pip install pytest pytest-codspeed numpy pandas pyarrow polars pytz
98+
# step 2: build+install the project (release) + build group, WITHOUT the heavy default `dev` group
99+
# (torch/tensorflow/pyspark). uv.lock is gitignored, so it is deliberately NOT relied on for bench deps.
100+
uv sync --no-build-isolation --no-editable --reinstall --no-default-groups --group build -p 3.13
101+
# step 3: install the FROZEN, committed bench pins (exact ==). Regenerated deliberately with the baseline
102+
# (source list: pyproject [dependency-groups] bench), so the only cross-run delta is the binding.
103+
uv pip install -r benchmarks/requirements-bench.txt
104+
105+
- name: Collect gate node-ids
106+
# the gate/informational split (conftest markers) classifies which benchmarks are gate-able; regen uses it
107+
run: uv run --no-sync pytest benchmarks/ -m gate --collect-only -q -o addopts= -p no:cacheprovider \
108+
| grep '::' > gate_list.txt || true
109+
110+
- name: Run benchmarks under Callgrind (per-benchmark instruction counts)
111+
# ONE sweep over all gate+informational benchmarks EXCEPT the concurrency module (Callgrind serializes
112+
# threads -> its wall-clock signal is meaningless and it is expensive). Each benchmark emits a callgrind
113+
# dump keyed by its uri. The pytest-codspeed hooks obj-skip libpython, so counts are clean.
114+
run: |
115+
mkdir -p profiles
116+
CODSPEED_PROFILE_FOLDER="$PWD/profiles" valgrind --tool=callgrind --instr-atstart=no \
117+
--callgrind-out-file="$PWD/profiles/cg.%p.%n" \
118+
uv run --no-sync pytest benchmarks/ \
119+
--ignore=benchmarks/test_concurrency_perf.py \
120+
-m "gate or informational" --codspeed -o addopts= -p no:cacheprovider
121+
122+
- name: Compare against committed baseline (report-only)
123+
if: ${{ !inputs.regen }}
124+
# report-only for now: prints the per-benchmark delta table and NEVER fails the job. Add --enforce here
125+
# once trusted to fail on a gate regression (informational benches never fail).
126+
run: |
127+
uv run --no-sync python benchmarks/compare_baseline.py compare \
128+
--profiles profiles --baseline benchmarks/baseline.json \
129+
--submodule-sha "${{ steps.duckdb_sha.outputs.sha }}" \
130+
--pins benchmarks/requirements-bench.txt
131+
132+
- name: Regenerate baseline (upload artifact to commit deliberately)
133+
if: ${{ inputs.regen }}
134+
run: |
135+
uv run --no-sync python benchmarks/compare_baseline.py regen \
136+
--profiles profiles --out benchmarks/baseline.json --gate-list gate_list.txt \
137+
--git-commit "${{ github.sha }}" --submodule-sha "${{ steps.duckdb_sha.outputs.sha }}" \
138+
--pins benchmarks/requirements-bench.txt
62139
63-
- name: Run benchmarks (instruction-count)
64-
uses: CodSpeedHQ/action@v4
140+
- name: Upload regenerated baseline
141+
if: ${{ inputs.regen }}
142+
uses: actions/upload-artifact@v4
65143
with:
66-
mode: simulation
67-
run: uv run pytest benchmarks/ --codspeed -o addopts= -p no:cacheprovider
144+
name: baseline-update
145+
path: benchmarks/baseline.json

‎benchmarks/PLAN.md‎

Lines changed: 49 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -122,20 +122,58 @@ benchmarks/
122122
```
123123
One module per binding subsystem so a CodSpeed report points at one src/ area. torch/tf go in produce_numpy (wrap FetchNumpyInternal); polars stays in arrow (wraps FetchArrowTable).
124124

125+
> **Note (reconciled to the implemented model).** The prose below originally described a per-PR CodSpeed
126+
> commit-diff gate. That is NOT how the suite works now. The implemented model is: **nightly `schedule` +
127+
> manual `workflow_dispatch`** (no per-PR trigger, no CodSpeed account/token/runner), a **self-hosted
128+
> `valgrind --tool=callgrind`** sweep that emits one dump per benchmark, and **`compare_baseline.py`** diffing
129+
> those counts against a **committed `benchmarks/baseline.json`**. See `.github/workflows/codspeed.yml`.
130+
125131
### Walltime vs instruction-count
126132

127133
- **Local A/B (macOS arm64): walltime only** (no Valgrind), `--codspeed-mode=walltime`.
128-
- **CI gate: instruction-count / simulation (Linux + Callgrind)**, deterministic — gate PRs with this.
129-
130-
Instruction-count is ideal AND should gate the GIL-held single-threaded overhead paths: fetchone loop, fetchall/fetchmany, native UDF per-call, native values() ingest, analyzer bind, all per-element converters (FromValue, TransformPythonValue, NumpyScan object/string, ArrayWrapper fill). The historical fetchall regression would be caught cleanly here.
131-
132-
Noisy under instruction-count — keep walltime-only, informational, do NOT hard-gate:
133-
- to_arrow_table / pl() on materialized results: PromoteMaterializedToArrow re-runs the query parallel with GIL released (`pyresult.cpp:450-477`).
134-
- Large 1M+ SELECT sum() ingest reads: engine parallel aggregate dominates.
135-
- read_csv/parquet/json: engine + I/O dominated.
136-
- GIL-per-chunk streaming (FetchNextRaw, to_record_batch_reader drain).
137-
138-
Gate tactic: pair each large-throughput scenario with a small/1-row variant (e.g. fetchall range(1_000_000) walltime + fetchall range(2048) instruction-count gate) so binding fixed-cost is measured noise-free.
134+
- **CI: instruction-count via self-hosted Callgrind (Linux)**, near-deterministic (~0.1% noise with
135+
`PYTHONHASHSEED=0`; often bit-identical) — compared against the committed baseline, **report-only** for now
136+
(flip `compare_baseline.py` to `--enforce` when trusted).
137+
138+
### Marker split + committed-baseline gate (INFRA-1 / Phase-3)
139+
140+
- Every benchmark carries exactly one of `@pytest.mark.gate` / `@pytest.mark.informational` (registered in
141+
`conftest.py`). **gate** = binding-dominated, instruction-count-meaningful (fetchone loop, fetchall/fetchmany,
142+
df()/fetchnumpy, native UDF per-call, native values()/executemany ingest, analyzer bind, per-element
143+
converters). **informational** = engine/library/streaming-diluted, reported but never gated
144+
(`to_arrow_table`/`pl()`/`to_pandas` GIL-released re-runs; registered-frame `SELECT sum()` reads;
145+
streaming drains; the concurrency module).
146+
- **Engine floors + Option-B (MEAS-1).** `test_engine_control_perf.py` measures `SELECT sum(...) FROM range(N)`
147+
with no Python egress — the engine floor. At baseline **regen**, each mapped numeric-produce gate's binding
148+
fraction `= 1 - floor_Ir/bench_Ir` is computed; a gate below the ~25% cutoff is **auto-moved to
149+
informational** (a threshold on an engine-diluted total is not meaningful) and the fraction is stored in
150+
`baseline.json` for audit. MEAS-1 showed OUT-row fetch and UDFs are ~all binding (stay gate); numeric
151+
produce (`df()`/`fetchnumpy`) is a bulk memcpy of ~engine magnitude (auto-move candidate).
152+
- **Small-N gates are compile+fetch fixed-cost**, not pure fetch (MEAS-1: ~60% compile+engine at `range(2048)`).
153+
- **Engine-bump guard.** `compare_baseline.py` compares the committed submodule SHA against the baseline's; if
154+
they differ, engine-inclusive deltas may reflect the engine bump, so gate deltas are not enforced (regen the
155+
baseline for the new engine).
156+
- **Reproducibility.** `benchmarks/requirements-bench.txt` (frozen `==` pins, from the `[dependency-groups]
157+
bench` list) + `benchmarks/baseline.json` are the co-regenerated pair; CI installs the frozen pins (NOT the
158+
gitignored `uv.lock`), so the only cross-run delta is the binding.
159+
160+
Still **informational / do NOT gate** (engine/parallel/IO/library dominated):
161+
- to_arrow_table / pl() on materialized results (PromoteMaterializedToArrow re-runs GIL-released).
162+
- registered-frame `SELECT sum()` ingest reads (engine aggregate dominates).
163+
- read_csv/parquet/json; GIL-per-chunk streaming drains.
164+
165+
### New coverage dimensions (beyond the converter surface)
166+
167+
- **Concurrency/GIL** (`test_concurrency_perf.py`, informational/walltime): threads {1,4,8} over a **multi-batch**
168+
arrow scan / pandas scan / native + arrow UDF. EXCLUDED from the Callgrind sweep (Callgrind serializes threads
169+
→ its wall-clock contention signal is meaningless there); it is a local walltime tool.
170+
- **Sustained-leak guard** (`tests/fast/test_binding_pressure_leak.py`): a plain psutil RSS + object-count
171+
ratio test (not a codspeed benchmark) for the object-pinning paths (register/unregister, UDF create/run/remove,
172+
executemany). Runs in the normal test suite.
173+
- **Memory mode (DEFERRED).** A second Callgrind sweep (`--codspeed-mode=memory`) over the O(rows) produce paths
174+
for peak-RSS, feeding the same baseline model, is DESIGNED but not implemented this round (roughly doubles the
175+
CI cost; nightly-only when added). The `test_mem_df_with_nulls` tracemalloc guard stays as a local signal until
176+
then (convert it to an A/B delta when memory mode lands).
139177

140178
### Two code-grounded gotchas
141179
- **OUT-col null benchmarks need REAL DuckDB nulls** (`CASE WHEN ... THEN NULL`): the masked-array branch only triggers on an actually-invalid validity bit (`array_wrapper.cpp:396-404,736`); a no-null column silently takes the cheap `std::move` path and measures the wrong thing.

‎benchmarks/_scale.py‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
"""Env-gated row-count scaling for the benchmark suite (INFRA-4).
2+
3+
Callgrind is 20-50x, and the O(rows) / per-row-object benchmarks at full N make the CI sweep too slow. `scaled(n)`
4+
shrinks those row counts ONLY when an explicit `BENCH_SCALE=<divisor>` env var is set (which the CI Callgrind
5+
sweep sets). Unset -> full N, so LOCAL walltime A/B keeps the large N unchanged.
6+
7+
CRITICAL: a gate benchmark and the engine-control floor it is compared against (the FLOOR_MAP pairs in
8+
compare_baseline.py) share the same base N, so routing BOTH through `scaled()` keeps them at an identical scaled
9+
N -- the Option-B binding_fraction stays valid. Scaling ONLY reduces row counts; it must never change the data
10+
patterns the benchmarks depend on (real NULLs, mixed ASCII+non-ASCII+null, LIMIT-no-ORDER-BY, warm-before-measure).
11+
12+
A floor keeps a scaled benchmark row-dominated (well above the range(2048) fixed-cost probes), so per-element
13+
work still dominates and the fraction/signal stay meaningful. The small-N `*_gate` probes are NOT routed through
14+
this (they are already fast and are the fixed-cost baseline).
15+
"""
16+
17+
from __future__ import annotations
18+
19+
import os
20+
21+
FLOOR = 20_000 # a scaled bench never drops below this (stays row-dominated, ~10x the range(2048) probes)
22+
23+
24+
def bench_scale() -> int:
25+
"""Return the divisor from `BENCH_SCALE` (>=1); 1 (no scaling) if unset/invalid."""
26+
v = os.environ.get("BENCH_SCALE")
27+
if not v:
28+
return 1
29+
try:
30+
return max(int(v), 1)
31+
except ValueError:
32+
return 1
33+
34+
35+
def scaled(n: int) -> int:
36+
"""Return `n` at full scale, or `max(n // BENCH_SCALE, min(n, FLOOR))` when scaling is enabled."""
37+
d = bench_scale()
38+
if d <= 1:
39+
return n
40+
return max(n // d, min(n, FLOOR))

0 commit comments

Comments
 (0)