Skip to content

Commit 7cf53cb

Browse files
Add documentation links to benchmark suite PR comments (#8815)
## Rationale for this change It's hard to know what each CI benchmark suite actually measures or where it is defined. Every suite should ship a short explainer doc, and the recurring `# Benchmarks: <suite>` PR comments should link to it. ## What changes are included in this PR? 1. **A markdown explainer per benchmark suite**, living next to the suite definition: `vortex-bench/sql/{tpch,tpcds,appian,vortex}/README.md`, `vortex-bench/sql/{clickbench,fineweb,statpopgen,polarsignals,spatialbench,gharchive,public-bi}.md`, and `benchmarks/{random-access,compress}-bench/README.md`. Each covers what the suite measures, the dataset, the CI variants, and how to run it locally. 2. **The Rust benchmark code is the source of truth for the link.** `Benchmark::doc_path()` is a required trait method; the runner stamps it as a `doc` field on every results.json row (the standalone `random-access-bench`/`compress-bench` binaries do the same via a `DOC_PATH` const). 3. **`scripts/compare-benchmark-jsons.py` renders the comment title** as `# Benchmarks: <name> [📖](<doc>)` from that field, replacing the `echo` lines in `bench-pr.yml`/`sql-benchmarks.yml`. Per review feedback, the link pins the results' commit SHA rather than `develop`, so it resolves before the PR merges and stays valid afterwards. ## What APIs are changed? Are there any user-facing changes? `vortex_bench::Benchmark` gains a required `doc_path()` method (new suites must ship a doc), and `print_measurements_json`/`export_results` take the doc path. results.json rows carry a new `doc` field. Benchmark PR comments now start with a 📖 link to the suite's explainer. --------- Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk>
1 parent cc6803a commit 7cf53cb

33 files changed

Lines changed: 380 additions & 12 deletions

File tree

.github/workflows/bench-pr.yml

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -113,10 +113,8 @@ jobs:
113113
python3 scripts/s3-download.py s3://vortex-ci-benchmark-results/data.json.gz data.json.gz --no-sign-request
114114
gzip -d -c data.json.gz > base.json
115115
116-
echo '# Benchmarks: ${{ matrix.benchmark.name }}' > comment.md
117-
echo '' >> comment.md
118116
uv run --no-project scripts/compare-benchmark-jsons.py base.json results.json "${{ matrix.benchmark.name }}" \
119-
>> comment.md
117+
> comment.md
120118
cat comment.md >> $GITHUB_STEP_SUMMARY
121119
122120
- name: Comment PR

.github/workflows/sql-benchmarks.yml

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -678,10 +678,8 @@ jobs:
678678
python3 scripts/s3-download.py s3://vortex-ci-benchmark-results/data.json.gz data.json.gz --no-sign-request
679679
gzip -d -c data.json.gz > base.json
680680
681-
echo "# Benchmarks: ${{ matrix.name }}" > comment.md
682-
echo '' >> comment.md
683681
uv run --no-project scripts/compare-benchmark-jsons.py base.json results.json "${{ matrix.name }}" \
684-
>> comment.md
682+
> comment.md
685683
cat comment.md >> "$GITHUB_STEP_SUMMARY"
686684
687685
- name: Comment PR
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
# Compression benchmark
2+
3+
Measures compression and decompression throughput, plus resulting file sizes, for Vortex
4+
versus Parquet (and optionally Lance) across a range of datasets: NYC taxi data, several
5+
[Public BI](https://github.com/cwida/public_bi_benchmark) tables (Arade, Bimbo,
6+
CMSprovider, Euro2016, Food, HashTags), TPC-H `l_comment` variants, and synthetic nested
7+
data. This is the workload behind the `Compression` PR comment.
8+
9+
See [`src/main.rs`](./src/main.rs) for the dataset list and CLI flags (`--formats`,
10+
`--datasets`, `--ops compress,decompress`).
11+
12+
## Running locally
13+
14+
```bash
15+
cargo run -p compress-bench --profile release_debug
16+
```

benchmarks/compress-bench/src/main.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,9 @@ fn get_compressor(format: Format) -> Box<dyn Compressor> {
113113
/// The benchmark ID used for output path.
114114
const BENCHMARK_ID: &str = "compress";
115115

116+
/// Repo-relative path of the suite explainer linked from CI benchmark PR comments.
117+
const DOC_PATH: &str = "benchmarks/compress-bench/README.md";
118+
116119
async fn run_compress(
117120
iterations: usize,
118121
datasets_filter: Option<Regex>,
@@ -212,8 +215,8 @@ async fn run_compress(
212215
)
213216
}
214217
DisplayFormat::GhJson => {
215-
print_measurements_json(&mut writer, measurements.timings)?;
216-
print_measurements_json(&mut writer, measurements.ratios)
218+
print_measurements_json(&mut writer, measurements.timings, DOC_PATH)?;
219+
print_measurements_json(&mut writer, measurements.ratios, DOC_PATH)
217220
}
218221
}
219222
}
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
# Random Access benchmark
2+
3+
Measures point-lookup latency: fetching individual rows by index from a file, rather than
4+
scanning it. This is the workload behind the `Random Access` PR comment.
5+
6+
Two access patterns are generated with a fixed seed (see [`src/main.rs`](./src/main.rs)):
7+
8+
- **correlated**: several clusters of consecutive indices scattered across the dataset,
9+
simulating lookups with spatial locality;
10+
- **uniform**: indices drawn from a Poisson process spread uniformly across the dataset,
11+
simulating lookups with no locality.
12+
13+
Each pattern runs over four datasets (`taxi`, `feature-vectors`, `nested-lists`,
14+
`nested-structs`) in Parquet, Lance, and Vortex, both with a cached open file handle and
15+
reopening the file per lookup. CI drives the full matrix via
16+
[`scripts/random-access-split.py`](../../scripts/random-access-split.py).
17+
18+
## Running locally
19+
20+
```bash
21+
cargo run -p random-access-bench --profile release_debug --features lance
22+
```

benchmarks/random-access-bench/src/lib.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -282,6 +282,9 @@ async fn open_accessor(
282282
/// The benchmark ID used for output path.
283283
const BENCHMARK_ID: &str = "random-access";
284284

285+
/// Repo-relative path of the suite explainer linked from CI benchmark PR comments.
286+
const DOC_PATH: &str = "benchmarks/random-access-bench/README.md";
287+
285288
/// Fixed indices used by the original taxi benchmark (preserved for historical continuity).
286289
const FIXED_TAXI_INDICES: [u64; 6] = [10, 11, 12, 13, 100_000, 3_000_000];
287290

@@ -409,7 +412,7 @@ pub async fn run(config: RunConfig) -> Result<()> {
409412
}
410413
DisplayFormat::GhJson => {
411414
let timings: Vec<TimingMeasurement> = runs.into_iter().map(|r| r.timing).collect();
412-
print_measurements_json(&mut writer, timings)?;
415+
print_measurements_json(&mut writer, timings, DOC_PATH)?;
413416
}
414417
}
415418

scripts/compare-benchmark-jsons.py

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
# SPDX-FileCopyrightText: Copyright the Vortex contributors
1313

1414
import math
15+
import os
1516
import re
1617
import sys
1718
from dataclasses import dataclass
@@ -763,6 +764,27 @@ def format_within_engine_summary(analyses: dict[str, dict[str, Any]]) -> str | N
763764
return " · ".join(summaries)
764765

765766

767+
def format_title(benchmark_name: str, pr: pd.DataFrame) -> str:
768+
"""Render the comment title, linking the suite explainer doc emitted by the benchmark binary.
769+
770+
The doc path is a repo-relative markdown path carried on the PR result rows (the `doc`
771+
field, populated from `Benchmark::doc_path` in Rust), so the benchmark code is the single
772+
source of truth for where each suite is documented. The link pins the PR's own commit so
773+
it resolves before the PR merges and stays valid afterwards.
774+
"""
775+
776+
title = f"# Benchmarks: {benchmark_name}" if benchmark_name else "# Benchmarks"
777+
if "doc" in pr.columns:
778+
docs = pr["doc"].dropna().unique()
779+
if len(docs) > 0:
780+
server_url = os.environ.get("GITHUB_SERVER_URL", "https://github.com")
781+
repository = os.environ.get("GITHUB_REPOSITORY", "vortex-data/vortex")
782+
commits = pr["commit_id"].dropna().unique() if "commit_id" in pr.columns else []
783+
ref = commits[0] if len(commits) > 0 else "develop"
784+
title += f" [\N{OPEN BOOK}]({server_url}/{repository}/blob/{ref}/{docs[0]})"
785+
return title
786+
787+
766788
def format_report_help() -> str:
767789
"""Render explanatory markdown for the benchmark report headline fields."""
768790

@@ -823,6 +845,7 @@ def main() -> None:
823845
benchmark_name = sys.argv[3] if len(sys.argv) > 3 else ""
824846

825847
pr = pd.read_json(sys.argv[2], lines=True)
848+
title = format_title(benchmark_name, pr)
826849
base = read_latest_baseline_rows(sys.argv[1], pr)
827850

828851
base_commit_id = set(base["commit_id"].unique())
@@ -896,6 +919,8 @@ def main() -> None:
896919
shifts += f" · Median polish {format_ratio_change(float(np.exp(polish.overall)))}"
897920
summary_fields.append(f"**Shifts**: {shifts}")
898921

922+
print(title)
923+
print("")
899924
print("<br>".join(summary_fields))
900925
print("")
901926
print(format_report_help())

vortex-bench/REUSE.toml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,12 @@ path = "**/*.csv"
1010
SPDX-FileCopyrightText = "Copyright the Vortex contributors"
1111
SPDX-License-Identifier = "Apache-2.0"
1212

13+
# Benchmark suite explainer docs linked from CI PR comments.
14+
[[annotations]]
15+
path = "sql/**/*.md"
16+
SPDX-FileCopyrightText = "Copyright the Vortex contributors"
17+
SPDX-License-Identifier = "CC-BY-4.0"
18+
1319
# `insta` snapshot files do not allow leading comment lines.
1420
[[annotations]]
1521
path = "src/snapshots/**.snap"

vortex-bench/sql/appian/README.md

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
# Appian benchmark
2+
3+
Mirrors the queries from DuckDB's in-tree
4+
[`appian_benchmarks`](https://github.com/duckdb/duckdb/tree/main/benchmark/appian_benchmarks)
5+
suite — a real-world business-application workload (customers, orders, addresses) with
6+
selective filters and joins over camelCase-named columns.
7+
8+
The eight queries live in this directory (`q1.sql` ... `q8.sql`). Upstream ships the data
9+
as a single `.duckdb` blob (~593 MB); the harness downloads it once and projects each
10+
table into Parquet with lowercased column names so that DataFusion and DuckDB resolve the
11+
verbatim queries identically — see [`src/appian`](../../src/appian) for the details.
12+
13+
## CI variant
14+
15+
CI runs this suite from local NVMe as the `Appian on NVME` PR comment, comparing
16+
DataFusion and DuckDB over Parquet and Vortex files (plus a native DuckDB baseline).
17+
18+
## Running locally
19+
20+
```bash
21+
vx-bench run appian --engine datafusion,duckdb --format parquet,vortex
22+
```

vortex-bench/sql/clickbench.md

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
# ClickBench benchmark
2+
3+
[ClickBench](https://github.com/ClickHouse/ClickBench) is ClickHouse's web-analytics
4+
benchmark: 43 queries over a single wide `hits` table (~100M rows of real-ish traffic
5+
data). It is heavy on aggregations, `GROUP BY`s over high-cardinality columns, and
6+
selective string filters, and is the main "wide table scan" workload in CI.
7+
8+
The queries live in [`clickbench_queries.sql`](./clickbench_queries.sql) (one query per
9+
line, numbered from Q0 in file order). The harness lives in
10+
[`src/clickbench`](../src/clickbench).
11+
12+
## CI variant
13+
14+
CI runs this suite from local NVMe as the `Clickbench on NVME` PR comment, comparing
15+
DataFusion and DuckDB over Parquet, Vortex, and vortex-compact files (plus a native
16+
DuckDB baseline).
17+
18+
## Sorted variant
19+
20+
`Clickbench Sorted on NVME` runs the same table sorted by event time, split into 100
21+
shards whose filenames are shuffled so engines cannot rely on file order, exercising sort
22+
pushdown and zone-map-style pruning on a subset of the queries. See
23+
[`ClickBenchSortedBenchmark`](../src/clickbench/benchmark.rs).
24+
25+
## Running locally
26+
27+
```bash
28+
vx-bench run clickbench --engine datafusion,duckdb --format parquet,vortex
29+
vx-bench run clickbench-sorted --engine datafusion,duckdb --format parquet,vortex
30+
```

0 commit comments

Comments
 (0)