From 0b9bec142dcd3e40b983d304be0d716155839514 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 20 Sep 2026 16:14:51 +0000 Subject: [PATCH] Keep third-party and frozen baselines out of CodSpeed `arrow_checked_add_u32` runs the arrow-rs checked add next to the Vortex lane kernel, and `words_gather_scalar` runs a frozen copy of the previous scalar `collect_bool` loop. Neither can regress because of a pull request, so a change in their number is never actionable, and on the walltime legs they were the three noisiest series in the suite: the arrow benchmark flipped on 64 of 97 pull requests on the neon leg alone. Both stay in the tree for local `cargo bench` comparisons. They lose `#[cpu_features]`, which takes them off the walltime legs, and gain `#[cfg(not(codspeed))]`, which keeps them out of the simulation build. Signed-off-by: Claude --- docs/developer-guide/benchmarking.md | 9 +++++++++ vortex-buffer/benches/collect_bool.rs | 16 ++++++++++------ vortex-compute/benches/lane_kernels.rs | 19 ++++++++++++++----- 3 files changed, 33 insertions(+), 11 deletions(-) diff --git a/docs/developer-guide/benchmarking.md b/docs/developer-guide/benchmarking.md index abe3f109bed..0e2b59d0067 100644 --- a/docs/developer-guide/benchmarking.md +++ b/docs/developer-guide/benchmarking.md @@ -159,6 +159,15 @@ benchmark is about kernel code rather than memory bandwidth. Use `#[cfg(not(codspeed))]` for benchmarks that are incompatible with CodSpeed. +### Keep third-party and frozen baselines out of CodSpeed + +A benchmark of code that Vortex does not own, such as an arrow-rs kernel over the same data, or +of a frozen copy of an old Vortex implementation, cannot regress because of a pull request, so a +change in its number is never actionable. On the walltime legs these baselines were among the +noisiest series in the suite. Keep them for local `cargo bench` comparisons, but gate them with +`#[cfg(not(codspeed))]` and leave them untagged, as `vortex-compute/benches/lane_kernels.rs` and +`vortex-buffer/benches/collect_bool.rs` do. + ### CodSpeed's single-run model CI benchmarks run under [CodSpeed's CPU simulation](https://codspeed.io/docs/instruments/cpu), diff --git a/vortex-buffer/benches/collect_bool.rs b/vortex-buffer/benches/collect_bool.rs index f5fc14f3518..083f608d6ba 100644 --- a/vortex-buffer/benches/collect_bool.rs +++ b/vortex-buffer/benches/collect_bool.rs @@ -13,11 +13,13 @@ //! - `collect_bool_*` / `from_bool_slice`: the public entry points end to end, with a //! boolean-gather predicate and a `u32` comparison predicate. //! -//! `words_gather_dispatch` and `words_gather_scalar` carry `#[cpu_features]`, so they are -//! measured on every walltime CPU-feature leg rather than in simulation. Both are written -//! once and compiled differently per leg: the shipped entry point picks its pack kernel -//! through `cfg(target_feature)`, and how well the scalar loop auto-vectorizes depends on -//! the build. Comparing them across legs is the point. +//! `words_gather_dispatch` carries `#[cpu_features]`, so it is measured on every walltime +//! CPU-feature leg rather than in simulation: the shipped entry point is written once and +//! picks its pack kernel through `cfg(target_feature)`, so each leg measures a different +//! build of it. `words_gather_scalar`, the frozen copy of the previous scalar loop, is +//! compiled only outside CodSpeed: its code never changes, so a change in its walltime is +//! never actionable, and it flipped by up to 15% between runs of identical code. Run +//! `cargo bench` locally to compare the two under one set of build flags. //! //! The hand-written per-kernel benchmarks are not tagged. Each one needs an instruction set //! extension the other legs do not build for, so they stay out of CodSpeed entirely and @@ -27,6 +29,7 @@ use divan::Bencher; use vortex_buffer::BitBuffer; +#[cfg(not(codspeed))] use vortex_buffer::collect_bool_word_scalar; #[cfg(not(codspeed))] use vortex_buffer::pack_bool_word_swar; @@ -123,7 +126,7 @@ fn words_gather_dispatch(bencher: Bencher, len: usize) { }); } -#[vortex_bench_support::cpu_features] +#[cfg(not(codspeed))] #[divan::bench(args = GATHER_INPUT_SIZE)] fn words_gather_scalar(bencher: Bencher, len: usize) { bench_words_gather(bencher, len, |words, len, bools| { @@ -180,6 +183,7 @@ fn words_gather_neon(bencher: Bencher, len: usize) { /// Faithful copy of the previous scalar-only `collect_bool_words` word loop, used as the /// baseline for the end-to-end comparison. +#[cfg(not(codspeed))] fn collect_bool_words_old(words: &mut [u64], len: usize, mut f: impl FnMut(usize) -> bool) { let full = len / 64; let remainder = len % 64; diff --git a/vortex-compute/benches/lane_kernels.rs b/vortex-compute/benches/lane_kernels.rs index 281646ada67..0b1f704d020 100644 --- a/vortex-compute/benches/lane_kernels.rs +++ b/vortex-compute/benches/lane_kernels.rs @@ -13,10 +13,13 @@ //! equivalent arrow-rs kernel over the same data shape, so the divan report //! lines up side-by-side. //! -//! The checked-add pair carries `#[cpu_features]`, so both are measured on every -//! walltime CPU-feature leg rather than in simulation: they are one lane loop -//! compiled differently per leg, and comparing them against arrow-rs is only -//! meaningful under the same build flags. The cast benches stay in simulation. +//! `lanezip_checked_add_u32` carries `#[cpu_features]`, so it is measured on every +//! walltime CPU-feature leg rather than in simulation: it is one lane loop compiled +//! differently per leg. Its arrow-rs sibling is compiled only outside CodSpeed. The +//! arrow kernel is not Vortex code, so a change in its walltime is never actionable, +//! and it flipped by up to 67% between runs of identical code. Run `cargo bench` +//! locally to compare the pair under the same build flags. The cast benches stay in +//! simulation. #![expect(clippy::unwrap_used)] #![expect(clippy::clone_on_ref_ptr)] @@ -24,10 +27,12 @@ use std::mem::MaybeUninit; use std::sync::Arc; +#[cfg(not(codspeed))] use arrow_arith::numeric::add; use arrow_array::ArrayRef as ArrowArrayRef; use arrow_array::Int32Array; use arrow_array::UInt16Array; +#[cfg(not(codspeed))] use arrow_array::UInt32Array; use arrow_array::UInt64Array; use arrow_buffer::NullBuffer; @@ -289,7 +294,9 @@ struct AddFixture { rhs_mask: BitBuffer, /// Plain `Vec` mirrors of the validity masks — used to build the arrow /// `NullBuffer`s for the baseline bench. + #[cfg(not(codspeed))] lhs_valid: Vec, + #[cfg(not(codspeed))] rhs_valid: Vec, } @@ -333,7 +340,9 @@ fn add_fixture(n: usize) -> AddFixture { rhs, lhs_mask, rhs_mask, + #[cfg(not(codspeed))] lhs_valid, + #[cfg(not(codspeed))] rhs_valid, } } @@ -361,7 +370,7 @@ fn lanezip_checked_add_u32(bencher: Bencher, n: usize) { }); } -#[vortex_bench_support::cpu_features] +#[cfg(not(codspeed))] #[divan::bench(args = ADD_SIZES)] fn arrow_checked_add_u32(bencher: Bencher, n: usize) { let f = add_fixture(n);