fix benchmarks and add workflow

Author: evertlammerts

.github/workflows/codspeed.yml

@@ -1,19 +1,12 @@
-# Performance-regression benchmarks via CodSpeed, in deterministic instruction-count (simulation) mode.
+# Perf-regression benchmarks via CodSpeed in instruction-count (simulation) mode: deterministic, so the whole
+# suite is gate-able (no walltime noise, no gated/informational split).
 #
-# TOKENLESS FOR NOW: the CodSpeed action's token is only needed to UPLOAD results to the CodSpeed
-# dashboard (the hosted PR-gate). Without it the action still RUNS every benchmark under Valgrind and
-# reports the instruction counts in the job log. To turn on the hosted regression gate later: create a
-# CodSpeed project for the repo and either (public repo) rely on the OIDC `id-token: write` permission
-# below, or add a `CODSPEED_TOKEN` repo secret and pass `token: ${{ secrets.CODSPEED_TOKEN }}` to the action.
+# TOKENLESS: the token is only for uploading to the CodSpeed dashboard. Without it the action still runs every
+# benchmark and reports counts in the job log. For the hosted gate later, create a CodSpeed project and rely on
+# the OIDC id-token permission below (public repo), or add a CODSPEED_TOKEN secret and pass token: to the action.
 #
-# Why simulation (instruction-count) and not walltime: instruction counts are deterministic even for the
-# multi-threaded / engine-heavy paths (Valgrind serializes and counts), so the whole suite is gate-able and
-# there is no need to split "gated vs informational" the way noisy local walltime required. Instruction count
-# is exactly the signal that would have caught the LIST/ARRAY df() regression cleanly.
-#
-# NOTE: this workflow has not been run in CI yet; the build steps mirror the documented dev build (CLAUDE.md)
-# and will likely need a shakeout run. Valgrind is slow (~20-50x); if the full suite is too slow, trim the
-# largest-N benchmarks or run a curated subset.
+# Not yet run in CI; the build mirrors the dev build (CLAUDE.md) and will need a shakeout. Valgrind is slow
+# (~20-50x); trim the largest-N benchmarks if the suite is too slow.
 
 name: Benchmarks
 

benchmarks/test_arrow_perf.py

@@ -1,41 +1,46 @@
-"""Standalone CodSpeed benchmark module for the Arrow read/write binding paths — NOT integrated
-(not in pyproject, not in CI, not committed). Run under each build's interpreter and compare:
-
-  M=/Users/evert/projects/duckdb-python/main/.venv-release/bin/python
-  C=/Users/evert/projects/duckdb-python/wt-cutover/.venv-release/bin/python
-  cd /Users/evert/projects/duckdb-python/wt-cutover
-  $M -m pytest benchmarks/test_arrow_perf.py --codspeed --codspeed-mode=walltime -o addopts=
-  $C -m pytest benchmarks/test_arrow_perf.py --codspeed --codspeed-mode=walltime -o addopts=
-
-DESIGN — the data must be FULLY MOVED, not lazily wrapped, or the benchmark measures nothing:
-  * READ (arrow -> duckdb): the duckdb ENGINE must scan every value. We aggregate over the actual
-    columns (sum/length), NOT count(*) -- count(*) is answered from arrow metadata without touching data.
-  * WRITE (duckdb -> arrow): the CONSUMER must materialize everything.
-      - to_arrow_table() / pl() are EAGER (the full table / polars DataFrame is built).
-      - to_arrow_reader() is LAZY -- duckdb only produces a batch when it is pulled -- so we iterate the
-        whole stream to actually exercise and consume the write path.
-
-pyarrow/polars are pinned to the SAME version in both .venv-release, so the A/B delta is purely the binding.
+"""CodSpeed benchmark: Arrow read/write paths. Standalone, not in CI.
+
+A/B: run under each build, compare (data libs pinned identically, so the delta is the binding):
+  cd /Users/evert/projects/duckdb-python/wt-codspeed
+  for P in ../main/.venv-release/bin/python .venv-release/bin/python; do \
+    $P -m pytest benchmarks/test_arrow_perf.py \
+    --codspeed --codspeed-mode=walltime -o addopts= -p no:cacheprovider; \
+  done
+
+Data must be fully moved or nothing is measured: READ aggregates over real columns (sum/length, not count(*),
+which arrow answers from metadata); WRITE materializes the result (to_arrow_reader is lazy, so it is drained).
 """
 
-import duckdb
+from __future__ import annotations
+
+from typing import TYPE_CHECKING
+
 import pyarrow as pa
 import pytest
 
+import duckdb
+
+if TYPE_CHECKING:
+    from collections.abc import Iterator
+
+    from pytest_codspeed import BenchmarkFixture
+
 N = 500_000
 WRITE_Q_NUM = "SELECT i::BIGINT AS a, (i * 1.5)::DOUBLE AS b FROM range(500000) t(i)"
 WRITE_Q_STR = "SELECT ('str_value_' || i) AS s FROM range(500000) t(i)"
 
 
 @pytest.fixture
-def con():
+def con() -> Iterator[duckdb.DuckDBPyConnection]:
+    """Yield a fresh connection, closed on teardown."""
     c = duckdb.connect()
     yield c
     c.close()
 
 
 @pytest.fixture(scope="module")
-def arrow_numeric():
+def arrow_numeric() -> pa.Table:
+    """Return a two-column numeric arrow table."""
     return pa.table(
         {
             "a": pa.array(range(N), type=pa.int64()),
@@ -45,12 +50,14 @@ def arrow_numeric():
 
 
 @pytest.fixture(scope="module")
-def arrow_string():
+def arrow_string() -> pa.Table:
+    """Return a single-column string arrow table."""
     return pa.table({"s": pa.array([f"str_value_{i}" for i in range(N)], type=pa.string())})
 
 
 @pytest.fixture(scope="module")
-def arrow_numeric_batches(arrow_numeric):
+def arrow_numeric_batches(arrow_numeric: pa.Table) -> tuple[pa.Schema, list[pa.RecordBatch]]:
+    """Return the schema and record batches for the numeric table."""
     # RecordBatches are immutable/re-readable, so a fresh reader can be built from them every round
     return arrow_numeric.schema, arrow_numeric.to_batches(max_chunksize=50_000)
 
@@ -60,12 +67,16 @@ def arrow_numeric_batches(arrow_numeric):
 # --------------------------------------------------------------------------- #
 
 
-def test_read_arrow_numeric(benchmark, con, arrow_numeric):
+def test_read_arrow_numeric(
+    benchmark: BenchmarkFixture, con: duckdb.DuckDBPyConnection, arrow_numeric: pa.Table
+) -> None:
+    """Benchmark scanning a numeric arrow table."""
     con.register("t_num", arrow_numeric)
     benchmark(lambda: con.execute("SELECT sum(a), sum(b) FROM t_num").fetchall())
 
 
-def test_read_arrow_string(benchmark, con, arrow_string):
+def test_read_arrow_string(benchmark: BenchmarkFixture, con: duckdb.DuckDBPyConnection, arrow_string: pa.Table) -> None:
+    """Benchmark scanning a string arrow table."""
     con.register("t_str", arrow_string)
     benchmark(lambda: con.execute("SELECT count(s), sum(length(s)) FROM t_str").fetchall())
 
@@ -75,10 +86,15 @@ def test_read_arrow_string(benchmark, con, arrow_string):
 # full scan of every value.
 
 
-def test_read_arrow_reader_numeric(benchmark, con, arrow_numeric_batches):
+def test_read_arrow_reader_numeric(
+    benchmark: BenchmarkFixture,
+    con: duckdb.DuckDBPyConnection,
+    arrow_numeric_batches: tuple[pa.Schema, list[pa.RecordBatch]],
+) -> None:
+    """Benchmark scanning a streaming record-batch reader."""
     schema, batches = arrow_numeric_batches
 
-    def run():
+    def run() -> list:
         reader = pa.RecordBatchReader.from_batches(schema, iter(batches))
         con.register("t_rdr", reader)
         return con.execute("SELECT sum(a), sum(b) FROM t_rdr").fetchall()
@@ -92,16 +108,20 @@ def run():
 # --------------------------------------------------------------------------- #
 
 
-def test_write_arrow_table_numeric(benchmark, con):
+def test_write_arrow_table_numeric(benchmark: BenchmarkFixture, con: duckdb.DuckDBPyConnection) -> None:
+    """Benchmark materializing a numeric result to an arrow table."""
     benchmark(lambda: con.sql(WRITE_Q_NUM).to_arrow_table())
 
 
-def test_write_arrow_table_string(benchmark, con):
+def test_write_arrow_table_string(benchmark: BenchmarkFixture, con: duckdb.DuckDBPyConnection) -> None:
+    """Benchmark materializing a string result to an arrow table."""
     benchmark(lambda: con.sql(WRITE_Q_STR).to_arrow_table())
 
 
-def test_write_arrow_reader_consumed(benchmark, con):
-    def run():
+def test_write_arrow_reader_consumed(benchmark: BenchmarkFixture, con: duckdb.DuckDBPyConnection) -> None:
+    """Benchmark draining a lazy arrow record-batch reader."""
+
+    def run() -> int:
         reader = con.sql(WRITE_Q_NUM).to_arrow_reader(100_000)
         rows = 0
         for batch in reader:  # drain the lazy stream so duckdb actually produces every batch
@@ -111,9 +131,11 @@ def run():
     benchmark(run)
 
 
-def test_write_polars_numeric(benchmark, con):
+def test_write_polars_numeric(benchmark: BenchmarkFixture, con: duckdb.DuckDBPyConnection) -> None:
+    """Benchmark materializing a numeric result to a polars frame."""
     benchmark(lambda: con.sql(WRITE_Q_NUM).pl())
 
 
-def test_write_polars_string(benchmark, con):
+def test_write_polars_string(benchmark: BenchmarkFixture, con: duckdb.DuckDBPyConnection) -> None:
+    """Benchmark materializing a string result to a polars frame."""
     benchmark(lambda: con.sql(WRITE_Q_STR).pl())

benchmarks/test_cardinality_perf.py

@@ -1,33 +1,38 @@
-"""Standalone CodSpeed benchmark: the RESULT-CARDINALITY (rows-to-Python) sweep. Run under each build:
+"""CodSpeed benchmark: the result-cardinality (rows-to-Python) sweep. Standalone, not in CI.
 
-  M=/Users/evert/projects/duckdb-python/main/.venv-release/bin/python
-  C=/Users/evert/projects/duckdb-python/wt-codspeed/.venv-release/bin/python
+A/B: run under each build, compare (data libs pinned identically, so the delta is the binding):
   cd /Users/evert/projects/duckdb-python/wt-codspeed
-  $M -m pytest benchmarks/test_cardinality_perf.py --codspeed --codspeed-mode=walltime -o addopts= -p no:cacheprovider
-  $C -m pytest benchmarks/test_cardinality_perf.py --codspeed --codspeed-mode=walltime -o addopts= -p no:cacheprovider
-
-REDESIGN NOTE: the first version swept `ORDER BY a DESC LIMIT n` over a fixed source. That was wrong:
-the engine's full top-N SORT (~3-14ms, itself variable) dominated and swamped the per-row conversion
-signal, and the numbers came out non-monotone. This version pre-materializes the fixed source table ONCE
-and sweeps `SELECT * FROM src LIMIT n` with NO ORDER BY: a plain LIMIT early-stops the scan at n rows, so
-the engine cost is cheap and monotone in n, and the rows-to-Python CONVERSION is the dominant n-varying
-cost. That gives a clean, monotone per-row slope; the A/B delta at each n isolates the binding, and a build
-whose slope is steeper has a per-row conversion regression. n=100 is the overhead regime (the natural
-instruction-count-gate point); n=100_000 is throughput.
-
-3 columns (BIGINT, DOUBLE, VARCHAR) so per-row conversion is non-trivial. numpy/pandas/pyarrow are pinned to
-the SAME versions in both .venv-release, so the A/B delta is purely the binding.
+  for P in ../main/.venv-release/bin/python .venv-release/bin/python; do \
+    $P -m pytest benchmarks/test_cardinality_perf.py \
+    --codspeed --codspeed-mode=walltime -o addopts= -p no:cacheprovider; \
+  done
+
+Sweeps `SELECT * FROM src LIMIT n` (no ORDER BY) over a pre-materialized 3-column source: a plain LIMIT
+early-stops the scan, so the per-row conversion dominates and the slope is monotone in n. A steeper slope on
+one build is a per-row conversion regression. n=100 is the overhead regime, n=100_000 is throughput.
+(An earlier ORDER BY version was dropped: the top-N sort swamped the signal.)
 """
 
-import duckdb
+from __future__ import annotations
+
+from typing import TYPE_CHECKING
+
 import pytest
 
+import duckdb
+
+if TYPE_CHECKING:
+    from collections.abc import Iterator
+
+    from pytest_codspeed import BenchmarkFixture
+
 SRC_ROWS = 200_000
 LIMITS = [100, 1_000, 10_000, 100_000]
 
 
 @pytest.fixture(scope="module")
-def con():
+def con() -> Iterator[duckdb.DuckDBPyConnection]:
+    """Yield a connection over a once-materialized source table."""
     # Fixed source materialized ONCE (module-scoped): building it per test would add noise, and it must be
     # identical across the n sweep. `SELECT * FROM src LIMIT n` then reads only the first n rows.
     c = duckdb.connect()
@@ -39,28 +44,31 @@ def con():
     c.close()
 
 
-def _query(n):
+def _query(n: int) -> str:
     # No ORDER BY: a plain LIMIT early-stops the scan at n rows -> engine cost cheap and monotone in n, so the
     # per-row binding conversion dominates the n-varying signal (unlike the old ORDER BY top-N sort).
     return f"SELECT a, b, s FROM src LIMIT {n}"
 
 
 @pytest.mark.parametrize("n", LIMITS)
-def test_limit_fetchall(benchmark, con, n):
+def test_limit_fetchall(benchmark: BenchmarkFixture, con: duckdb.DuckDBPyConnection, n: int) -> None:
+    """Benchmark fetchall over a LIMIT n sweep."""
     q = _query(n)
     con.execute(q).fetchall()  # warm
     benchmark(lambda: con.execute(q).fetchall())
 
 
 @pytest.mark.parametrize("n", LIMITS)
-def test_limit_df(benchmark, con, n):
+def test_limit_df(benchmark: BenchmarkFixture, con: duckdb.DuckDBPyConnection, n: int) -> None:
+    """Benchmark df() over a LIMIT n sweep."""
     q = _query(n)
     con.sql(q).df()  # warm
     benchmark(lambda: con.sql(q).df())
 
 
 @pytest.mark.parametrize("n", LIMITS)
-def test_limit_to_arrow(benchmark, con, n):
+def test_limit_to_arrow(benchmark: BenchmarkFixture, con: duckdb.DuckDBPyConnection, n: int) -> None:
+    """Benchmark to_arrow_table() over a LIMIT n sweep."""
     q = _query(n)
     con.sql(q).to_arrow_table()  # warm
     benchmark(lambda: con.sql(q).to_arrow_table())