From 3a7c64b0de99d13b9c0a44fc1f92aa17588c82ba Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 27 Jul 2026 18:17:28 +0000 Subject: [PATCH 1/2] ci(codspeed): stop manufacturing spurious benchmark changes Two CI causes of reports flagging benchmarks that the PR cannot affect. Serialised develop baselines. All develop pushes shared one concurrency group and never cancel, so they ran strictly one at a time. A burst of merges leaves later commits without a finished baseline, and CodSpeed then falls back to an older comparison base with its own footnote: "No successful run was found on develop (X), so Y was used instead. There might be some changes unrelated to this pull request in this report." That footnote appears on four of twelve open-PR reports, and it lines up with the benchmarks that flip between two fixed values in opposite directions on unrelated PRs. Give each develop push its own group so baselines still run on every commit, as the comment intends, without queueing behind each other. Develop runs can now overlap, which raises peak demand on a preemptible runner pool. CUDA walltime suite triggered by any workflow edit. The paths filter matched .github/workflows/**, so every dependency bump ran the walltime CUDA benchmarks. Eleven CI-only PRs reported cuda/bitpacked_u8/unpack/3bw[100M] improving 18-19% from an identical base, each carrying CodSpeed's own warning that walltime on standard hosted runners produces inconsistent data. Only this workflow defines those jobs, so match just this file. Non-pull_request events still always run the CUDA suite, so develop keeps full coverage. Signed-off-by: Claude Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_015ypCZPxvLuJK9FgWKF8U4P --- .github/workflows/codspeed.yml | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/.github/workflows/codspeed.yml b/.github/workflows/codspeed.yml index 80dc7c900b3..ab1c19c8bb1 100644 --- a/.github/workflows/codspeed.yml +++ b/.github/workflows/codspeed.yml @@ -2,11 +2,13 @@ name: Codspeed Benchmarking # Concurrency control: # - PRs: new commits on a feature branch will cancel in-progress (outdated) runs. -# - Push to develop: runs queue sequentially, never cancelled. This allows us to have benchmarks -# run on every commit for our benchmarks website. +# - Push to develop: every commit gets its own group, so baseline runs never cancel and never +# queue behind each other. Serialising them meant a burst of merges left later commits without +# a finished baseline, so CodSpeed fell back to an older comparison base and reported changes +# unrelated to the PR being tested. # - `workflow_dispatch`: groups by branch and queues if run on develop. concurrency: - group: ${{ github.workflow }}-${{ github.ref }} + group: ${{ github.workflow }}-${{ github.ref }}-${{ github.event_name == 'push' && github.sha || '' }} cancel-in-progress: ${{ github.ref != 'refs/heads/develop' }} on: push: @@ -40,7 +42,8 @@ jobs: filters: | cuda: - "vortex-cuda/**" - - ".github/workflows/**" + # Only this workflow defines the CUDA benchmark jobs. + - ".github/workflows/codspeed.yml" bench-codspeed: strategy: From aa146bf2fa66b6575a538b88a2e3aff6a26dc8b5 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 27 Jul 2026 18:17:45 +0000 Subject: [PATCH 2/2] bench: stop measuring harness overhead in nanosecond benchmarks One divan bench_refs iteration carries a fixed ~146 instructions of harness. Where the measured operation costs less than that, the reported figure tracks the harness rather than the operation, and a small change in harness cost swings it past CodSpeed's 10% flag threshold with the measured code untouched. Per-iteration instruction counts (callgrind, the same instrument class as CodSpeed Simulation) confirm the flagged benchmarks are exactly the overhead-dominated ones: true_count_vortex_buffer 1024: 187 Ir (78% harness) <- flagged 2048: 228 Ir (64% harness) <- flagged 16384: 802 Ir (18% harness) 65536: 2770 Ir (5% harness) The fit is linear at ~0.04 Ir/bit with a ~146 Ir intercept, and no report ever flagged 16384 or 65536. This also explains why #8861 did not settle it: dropping 128 (97% harness) just promoted 1024 to worst offender. - true_count: keep 16384 and 65536, where the popcount dominates. - slice: BitBuffer::slice only adjusts an offset, a length and a refcount, so its cost is independent of length -- every size measured exactly 125 Ir, which is why #8749 reported identical values for [1024] and [16384]. Measure one length with enough slices per iteration to dominate, varying the offset so they cannot be folded together. No black_box is needed: the slice clones an Arc, and that refcount traffic is a side effect the optimiser cannot remove. - binary_search: one search over 65536 elements is ~16 comparisons, and at ~71% harness the two implementations measured 203 vs 206 Ir -- indistinguishable, defeating the comparison the benchmark exists for. Searching 64 targets per iteration separates them: 15133 Ir for vortex against 10010 for std. varbinview_compact was left alone: compact[(4096, 90)] is 1069 Ir, only 14% harness, so its reports belong to the comparison-base problem fixed in the preceding commit rather than to harness overhead. Signed-off-by: Claude Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_015ypCZPxvLuJK9FgWKF8U4P --- vortex-array/benches/search_sorted.rs | 38 ++++++++++++---- vortex-buffer/benches/vortex_bitbuffer.rs | 54 ++++++++++++++--------- 2 files changed, 62 insertions(+), 30 deletions(-) diff --git a/vortex-array/benches/search_sorted.rs b/vortex-array/benches/search_sorted.rs index 2b5f7fccbac..370290bf1c9 100644 --- a/vortex-array/benches/search_sorted.rs +++ b/vortex-array/benches/search_sorted.rs @@ -15,27 +15,49 @@ fn main() { divan::main(); } +/// One search over 65536 elements is ~16 comparisons, small enough that the measurement is mostly +/// harness overhead and the two implementations below become indistinguishable. Search a batch per +/// iteration instead; the targets differ, so the calls cannot be folded together. +const SEARCH_TARGETS: usize = 64; + #[divan::bench] fn binary_search_std(bencher: Bencher) { - let (sorted_array, target) = fixture(); + let (sorted_array, targets) = fixture(); bencher - .with_inputs(|| (&sorted_array, &target)) - .bench_refs(|(array, target)| array.binary_search(target)); + .with_inputs(|| (&sorted_array, &targets)) + .bench_refs(|(array, targets)| { + let mut found = 0; + for target in targets.iter() { + found += array.binary_search(target).unwrap_or_else(|idx| idx); + } + found + }); } #[divan::bench] fn binary_search_vortex(bencher: Bencher) { - let (sorted_array, target) = fixture(); + let (sorted_array, targets) = fixture(); bencher - .with_inputs(|| (&sorted_array, &target)) - .bench_refs(|(array, target)| array.search_sorted(target, SearchSortedSide::Left).unwrap()); + .with_inputs(|| (&sorted_array, &targets)) + .bench_refs(|(array, targets)| { + let mut found = 0; + for target in targets.iter() { + found += array + .search_sorted(target, SearchSortedSide::Left) + .unwrap() + .to_index(); + } + found + }); } -fn fixture() -> (Vec, i32) { +fn fixture() -> (Vec, Vec) { let mut rng = StdRng::seed_from_u64(0); let range = Uniform::new(0, 65_536).unwrap(); let mut data: Vec = (0..65_536).map(|_| rng.sample(range)).collect(); data.sort(); - (data, rng.sample(range)) + let targets = (0..SEARCH_TARGETS).map(|_| rng.sample(range)).collect(); + + (data, targets) } diff --git a/vortex-buffer/benches/vortex_bitbuffer.rs b/vortex-buffer/benches/vortex_bitbuffer.rs index b9a6a6ab505..f3afeda826d 100644 --- a/vortex-buffer/benches/vortex_bitbuffer.rs +++ b/vortex-buffer/benches/vortex_bitbuffer.rs @@ -153,33 +153,43 @@ fn value_arrow_buffer(bencher: Bencher, length: usize) { }); } -#[divan::bench(args = INPUT_SIZE)] -fn slice_vortex_buffer(bencher: Bencher, length: usize) { - let buffer = BitBuffer::from_iter((0..length).map(|i| i % 2 == 0)); - bencher - .with_inputs(|| (&buffer, length / 2)) - .bench_refs(|(buffer, mid)| { - let mid = *mid; - buffer.slice(mid / 2..mid + mid / 2) - }); +/// Slicing only adjusts an offset, a length and a refcount, so its cost is independent of buffer +/// length. Measure one length, with enough slices per iteration to stay above harness overhead. +const SLICE_ITERS: usize = 64; +const SLICE_INPUT_SIZE: usize = 65_536; + +#[divan::bench] +fn slice_vortex_buffer(bencher: Bencher) { + let buffer = BitBuffer::from_iter((0..SLICE_INPUT_SIZE).map(|i| i % 2 == 0)); + let mid = SLICE_INPUT_SIZE / 2; + bencher.with_inputs(|| &buffer).bench_refs(|buffer| { + let mut total = 0; + for offset in 0..SLICE_ITERS { + total += buffer.slice(mid / 2 + offset..mid + mid / 2).len(); + } + total + }); } #[cfg(not(codspeed))] -#[divan::bench(args = INPUT_SIZE)] -fn slice_arrow_buffer(bencher: Bencher, length: usize) { - let buffer = Arrow(BooleanBuffer::from_iter((0..length).map(|i| i % 2 == 0))); - bencher - .with_inputs(|| (&buffer, length / 2)) - .bench_refs(|(buffer, mid)| { - let mid = *mid; - buffer.0.slice(mid / 2, mid / 2) - }); +#[divan::bench] +fn slice_arrow_buffer(bencher: Bencher) { + let buffer = Arrow(BooleanBuffer::from_iter( + (0..SLICE_INPUT_SIZE).map(|i| i % 2 == 0), + )); + let mid = SLICE_INPUT_SIZE / 2; + bencher.with_inputs(|| &buffer).bench_refs(|buffer| { + let mut total = 0; + for offset in 0..SLICE_ITERS { + total += buffer.0.slice(mid / 2 + offset, mid / 2).len(); + } + total + }); } -/// A 128-bit `true_count` is a popcount over two `u64` words, so the measurement is fixed -/// dispatch and harness overhead plus binary code layout rather than the count itself. Only -/// sizes where the popcount loop dominates are worth measuring. -const TRUE_COUNT_INPUT_SIZE: &[usize] = &[1024, 2048, 16_384, 65_536]; +/// Below a few thousand bits the measurement is mostly divan's fixed per-iteration overhead +/// rather than the popcount, so it moves when the count itself has not changed. +const TRUE_COUNT_INPUT_SIZE: &[usize] = &[16_384, 65_536]; #[divan::bench(args = TRUE_COUNT_INPUT_SIZE)] fn true_count_vortex_buffer(bencher: Bencher, length: usize) {