Skip to content

Commit 177d99b

Browse files
committed
Redesign cardinality benchmark + add tokenless CodSpeed CI
Cardinality: the ORDER BY ... LIMIT n sweep let the engine top-N sort dominate and swamp the per-row conversion signal (numbers were non-monotone). Replace it with a pre-materialized fixed source + plain LIMIT n (no sort): the scan early-stops at n rows, so rows-to-Python conversion is the dominant n-varying cost and the slope is monotone; the A/B delta at each n isolates the binding. CI: .github/workflows/codspeed.yml runs the suite under CodSpeed simulation (instruction-count) mode on Linux, tokenless (no dashboard upload; enable the hosted gate later via a CodSpeed project + OIDC/token). Instruction counts are deterministic for every benchmark, so no gated/informational split is needed. Not yet run in CI; the build steps mirror the dev build and need a shakeout.
1 parent 159757d commit 177d99b

2 files changed

Lines changed: 100 additions & 23 deletions

File tree

.github/workflows/codspeed.yml

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
# Performance-regression benchmarks via CodSpeed, in deterministic instruction-count (simulation) mode.
2+
#
3+
# TOKENLESS FOR NOW: the CodSpeed action's token is only needed to UPLOAD results to the CodSpeed
4+
# dashboard (the hosted PR-gate). Without it the action still RUNS every benchmark under Valgrind and
5+
# reports the instruction counts in the job log. To turn on the hosted regression gate later: create a
6+
# CodSpeed project for the repo and either (public repo) rely on the OIDC `id-token: write` permission
7+
# below, or add a `CODSPEED_TOKEN` repo secret and pass `token: ${{ secrets.CODSPEED_TOKEN }}` to the action.
8+
#
9+
# Why simulation (instruction-count) and not walltime: instruction counts are deterministic even for the
10+
# multi-threaded / engine-heavy paths (Valgrind serializes and counts), so the whole suite is gate-able and
11+
# there is no need to split "gated vs informational" the way noisy local walltime required. Instruction count
12+
# is exactly the signal that would have caught the LIST/ARRAY df() regression cleanly.
13+
#
14+
# NOTE: this workflow has not been run in CI yet; the build steps mirror the documented dev build (CLAUDE.md)
15+
# and will likely need a shakeout run. Valgrind is slow (~20-50x); if the full suite is too slow, trim the
16+
# largest-N benchmarks or run a curated subset.
17+
18+
name: Benchmarks
19+
20+
on:
21+
pull_request:
22+
push:
23+
branches: [main]
24+
workflow_dispatch:
25+
26+
concurrency:
27+
group: codspeed-${{ github.ref }}
28+
cancel-in-progress: true
29+
30+
jobs:
31+
benchmarks:
32+
runs-on: ubuntu-latest
33+
permissions:
34+
contents: read
35+
id-token: write # enables tokenless (OIDC) upload once a CodSpeed project is linked; harmless otherwise
36+
steps:
37+
- uses: actions/checkout@v4
38+
with:
39+
submodules: recursive # the DuckDB engine submodule is needed to build
40+
fetch-depth: 0 # setuptools_scm needs history for version detection
41+
42+
- name: Install uv
43+
uses: astral-sh/setup-uv@v5
44+
with:
45+
python-version: "3.13"
46+
47+
- name: Cache sccache
48+
uses: actions/cache@v4
49+
with:
50+
path: ~/.cache/sccache
51+
key: sccache-codspeed-${{ hashFiles('external/duckdb') }}
52+
restore-keys: sccache-codspeed-
53+
54+
- name: Install sccache
55+
run: |
56+
curl -fsSL https://github.com/mozilla/sccache/releases/download/v0.8.2/sccache-v0.8.2-x86_64-unknown-linux-musl.tar.gz \
57+
| tar -xz --strip-components=1 -C /usr/local/bin sccache-v0.8.2-x86_64-unknown-linux-musl/sccache
58+
59+
- name: Build the extension (release) + benchmark deps
60+
env:
61+
CMAKE_C_COMPILER_LAUNCHER: sccache
62+
CMAKE_CXX_COMPILER_LAUNCHER: sccache
63+
run: |
64+
uv sync --only-group build --no-install-project -p 3.13
65+
uv sync --no-build-isolation --no-editable --reinstall -p 3.13
66+
# benchmark deps: keep these pinned in lockstep with any baseline you compare against, so the only
67+
# cross-run delta is the binding (numpy/pandas/pyarrow/polars/pytz + the codspeed plugin).
68+
uv pip install pytest pytest-codspeed numpy pandas pyarrow polars pytz
69+
70+
- name: Run benchmarks (instruction-count)
71+
uses: CodSpeedHQ/action@v4
72+
with:
73+
mode: simulation
74+
run: uv run pytest benchmarks/ --codspeed -o addopts= -p no:cacheprovider

benchmarks/test_cardinality_perf.py

Lines changed: 26 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,47 +1,50 @@
1-
"""Standalone CodSpeed benchmark module: the RESULT-CARDINALITY (top-N) sweep — NOT integrated (not in
2-
pyproject, not in CI, not committed). Run under each build's interpreter and compare:
1+
"""Standalone CodSpeed benchmark: the RESULT-CARDINALITY (rows-to-Python) sweep. Run under each build:
32
43
M=/Users/evert/projects/duckdb-python/main/.venv-release/bin/python
54
C=/Users/evert/projects/duckdb-python/wt-codspeed/.venv-release/bin/python
65
cd /Users/evert/projects/duckdb-python/wt-codspeed
76
$M -m pytest benchmarks/test_cardinality_perf.py --codspeed --codspeed-mode=walltime -o addopts= -p no:cacheprovider
87
$C -m pytest benchmarks/test_cardinality_perf.py --codspeed --codspeed-mode=walltime -o addopts= -p no:cacheprovider
98
10-
WHY THIS MODULE (adopted from iqmo-org/bareduckdb): hold the SOURCE fixed and sweep only the number of rows
11-
materialized to Python via ORDER BY ... LIMIT n for n in {100, 1k, 10k, 100k}, through fetchall / df /
12-
to_arrow_table. The engine cost (scan the fixed SRC + top-N heap) stays ~constant, so the walltime delta
13-
across n is dominated by the per-row binding conversion -> a clean per-row slope. The n=100 end is the
14-
noise-free overhead regime (the natural instruction-count-gate point); the n=100k end is throughput.
15-
16-
A clean monotone slope (and ~parity slope between the two builds) is the signal we report; a build whose slope
17-
is steeper has a per-row conversion regression. Source held constant rules out scan-cost as the confound (a
18-
cleaner axis than varying range(), which also changes scan cost).
19-
20-
numpy/pandas/pyarrow are pinned to the SAME versions in both .venv-release, so the A/B delta is purely the binding.
9+
REDESIGN NOTE: the first version swept `ORDER BY a DESC LIMIT n` over a fixed source. That was wrong:
10+
the engine's full top-N SORT (~3-14ms, itself variable) dominated and swamped the per-row conversion
11+
signal, and the numbers came out non-monotone. This version pre-materializes the fixed source table ONCE
12+
and sweeps `SELECT * FROM src LIMIT n` with NO ORDER BY: a plain LIMIT early-stops the scan at n rows, so
13+
the engine cost is cheap and monotone in n, and the rows-to-Python CONVERSION is the dominant n-varying
14+
cost. That gives a clean, monotone per-row slope; the A/B delta at each n isolates the binding, and a build
15+
whose slope is steeper has a per-row conversion regression. n=100 is the overhead regime (the natural
16+
instruction-count-gate point); n=100_000 is throughput.
17+
18+
3 columns (BIGINT, DOUBLE, VARCHAR) so per-row conversion is non-trivial. numpy/pandas/pyarrow are pinned to
19+
the SAME versions in both .venv-release, so the A/B delta is purely the binding.
2120
"""
2221

2322
import duckdb
2423
import pytest
2524

26-
SRC = 200_000 # fixed source size -> constant engine scan + top-N across all n
25+
SRC_ROWS = 200_000
2726
LIMITS = [100, 1_000, 10_000, 100_000]
2827

29-
# 3 columns (BIGINT, DOUBLE, VARCHAR) so the per-row conversion is non-trivial; source is a fixed inline
30-
# subquery (no table state) and ORDER BY forces a full scan + top-N of the same SRC rows every time.
31-
_SRC_SUBQ = f"(SELECT i::BIGINT AS a, (i * 1.5)::DOUBLE AS b, ('s_' || i) AS s FROM range({SRC}) t(i))"
32-
33-
34-
def _query(n):
35-
return f"SELECT a, b, s FROM {_SRC_SUBQ} ORDER BY a DESC LIMIT {n}"
3628

37-
38-
@pytest.fixture
29+
@pytest.fixture(scope="module")
3930
def con():
31+
# Fixed source materialized ONCE (module-scoped): building it per test would add noise, and it must be
32+
# identical across the n sweep. `SELECT * FROM src LIMIT n` then reads only the first n rows.
4033
c = duckdb.connect()
34+
c.execute(
35+
"CREATE TABLE src AS "
36+
f"SELECT i::BIGINT AS a, (i * 1.5)::DOUBLE AS b, ('s_' || i) AS s FROM range({SRC_ROWS}) t(i)"
37+
)
4138
yield c
4239
c.close()
4340

4441

42+
def _query(n):
43+
# No ORDER BY: a plain LIMIT early-stops the scan at n rows -> engine cost cheap and monotone in n, so the
44+
# per-row binding conversion dominates the n-varying signal (unlike the old ORDER BY top-N sort).
45+
return f"SELECT a, b, s FROM src LIMIT {n}"
46+
47+
4548
@pytest.mark.parametrize("n", LIMITS)
4649
def test_limit_fetchall(benchmark, con, n):
4750
q = _query(n)

0 commit comments

Comments
 (0)