diff --git a/.github/workflows/codspeed.yml b/.github/workflows/codspeed.yml index 199cb938063..c405a4bb861 100644 --- a/.github/workflows/codspeed.yml +++ b/.github/workflows/codspeed.yml @@ -297,7 +297,7 @@ jobs: for package in meta["packages"] for target in package["targets"] if "bench" in target["kind"] - and "cpu_features]" in pathlib.Path(target["src_path"]).read_text() + and "#[vortex_bench_support::cpu_features" in pathlib.Path(target["src_path"]).read_text() ) if not tagged: raise SystemExit("no benchmark carries `#[cpu_features]`; this job has nothing to measure") diff --git a/benchmarks/bench-support/src/lib.rs b/benchmarks/bench-support/src/lib.rs index 912605c781e..988df34b529 100644 --- a/benchmarks/bench-support/src/lib.rs +++ b/benchmarks/bench-support/src/lib.rs @@ -12,10 +12,10 @@ use quote::quote; use syn::ItemFn; use syn::parse_macro_input; -/// Measure this benchmark on every walltime CPU-feature leg instead of in simulation. +/// Measure this benchmark on walltime CPU-feature legs instead of in simulation. /// -/// Takes no argument: the benchmark runs on all of them. Write it *above* `#[divan::bench]`, -/// whose arguments it fills in — the name is qualified with the leg that produced it, both as +/// Runs on every leg except those named in `exclude = ["neon", ...]`. Write it above +/// `#[divan::bench]`, whose arguments it fills in. The name is qualified with the leg, both as /// a `::` component the CI filter selects on and as a suffix on the leaf name CodSpeed /// reports under, so the legs report one series each rather than fighting over a shared name. /// `ignore` takes the benchmark out of the sharded simulation job. A plain `cargo bench` runs @@ -30,22 +30,25 @@ use syn::parse_macro_input; /// Spell it out in full rather than importing it: benchmark files are read a function at a /// time, and the path says where the behaviour comes from. /// -/// This is for code that is written once and *compiled* differently per feature set — a +/// This is for code that is written once and *compiled* differently per feature set: a /// shipped entry point that selects its kernel through `cfg(target_feature)`, or a scalar /// loop whose auto-vectorization depends on the build. A hand-written kernel for one /// instruction set extension is a different thing: it cannot run on the other legs, so it /// does not belong here. Keep those on `#[cfg(not(codspeed))]` for local A/B runs. #[proc_macro_attribute] pub fn cpu_features(attr: TokenStream, item: TokenStream) -> TokenStream { - if !attr.is_empty() { - let attr = TokenStream2::from(attr); - return syn::Error::new_spanned( - attr, - "`#[cpu_features]` takes no argument; a tagged benchmark runs on every feature-set leg", - ) - .to_compile_error() - .into(); - } + let excluded = if attr.is_empty() { + quote!(false) + } else { + let argument = parse_macro_input!(attr as syn::MetaNameValue); + if !argument.path.is_ident("exclude") { + return syn::Error::new_spanned(argument, "expected `exclude = [\"feature\", ...]`") + .to_compile_error() + .into(); + } + let features = argument.value; + quote!((#features).contains(&env!("VORTEX_BENCH_VARIANT"))) + }; let mut function = parse_macro_input!(item as ItemFn); @@ -117,7 +120,7 @@ pub fn cpu_features(attr: TokenStream, item: TokenStream) -> TokenStream { #name, env!("VORTEX_BENCH_SUFFIX"), ), - ignore = env!("VORTEX_BENCH_VARIANT") == "simulation", + ignore = env!("VORTEX_BENCH_VARIANT") == "simulation" || #excluded, ) }; diff --git a/docs/developer-guide/benchmarking.md b/docs/developer-guide/benchmarking.md index 91dd8a77b00..8fcc9d756e2 100644 --- a/docs/developer-guide/benchmarking.md +++ b/docs/developer-guide/benchmarking.md @@ -1,292 +1,235 @@ # Benchmarking -Vortex has two categories of benchmarks: microbenchmarks for individual operations, and SQL -benchmarks for end-to-end query performance. +Vortex uses microbenchmarks for individual operations and SQL benchmarks for end-to-end query +performance. ## Microbenchmarks -Microbenchmarks use the Divan framework and live in `benches/` directories within individual crates. -Every microbenchmark binary **must** use `mimalloc` as its global allocator (see -[Use `mimalloc` as the global allocator](#use-mimalloc-as-the-global-allocator)). - -Run microbenchmarks for a specific crate with: +Rust microbenchmarks use Divan and live in each crate's `benches/` directory. Run a crate's +benchmarks with: ```bash cargo bench -p ``` -## Best Practices - -### Separate setup from profiled code +### Separate setup from measured work -Always use `bencher.with_inputs(|| ...)` so fixture construction is excluded from timing: +Use `with_inputs` to construct fixtures outside the timed closure: ```rust bencher - .with_inputs(|| bench_fixture())) - .bench_refs(|(array, indices)| { - array.take(indices.to_array()).unwrap() - }); + .with_inputs(|| bench_fixture()) + .bench_refs(|(array, indices)| array.take(indices.to_array()).unwrap()); ``` -### Exclude `Drop` from measurements - -Divan measures only the closure body, **not** the `Drop` of its return value. -Structure your benchmark so that expensive drops happen via the return value or -via bench_refs inputs. - -- **Return the value** from the closure — Divan will drop it after timing stops: - - ```rust - bencher - .with_inputs(|| make_big_vec()) - .bench_values(|v| transform(v)) // drop of the result is NOT timed - ``` +Initialize one-time process state before `divan::main()`. Lazy session setup or regex initialization +inside the first measured iteration can dominate the operation, especially under simulation, which +measures one cold iteration. See `vortex-array/benches/like.rs` for an example of initializing this +state before measurement. -- **Use `bench_refs`** — the input is dropped after the entire sample loop, not per-iteration: +### Exclude destruction when it is not part of the operation - ```rust - bencher - .with_inputs(|| make_big_vec()) - .bench_refs(|v| v.sort()) // v is dropped outside the timed region - ``` +Divan drops the closure's return value after timing stops. Return the result instead of discarding +it inside the closure: -Structure your benchmark so that expensive drops happen via the return value or via `bench_refs` inputs. - -### Black-box inputs to prevent compiler optimization - -The compiler can constant-fold or eliminate work if it can prove that inputs are known at -compile time. +```rust +bencher + .with_inputs(|| make_big_vec()) + .bench_values(|v| transform(v)); +``` -Values provided through `with_inputs` are automatically black-boxed by Divan — no action -needed: +Use `bench_refs` when the operation mutates an input in place. Divan keeps the input alive until the +sample ends, so its destruction is outside the timed region: ```rust -// ✓ `array` and `indices` are automatically black-boxed by Divan bencher - .with_inputs(|| (&prebuilt_array, &prebuilt_indices)) - .bench_refs(|(array, indices)| array.take(indices.to_array()).unwrap()); + .with_inputs(|| make_big_vec()) + .bench_refs(|v| v.sort()); ``` -### Captured variables +Other values dropped inside the closure contribute to the measurement. Keep those drops only when +they are part of the cost being measured, such as an allocate-and-free benchmark. + +### Prevent the compiler from removing work -Variables captured from the surrounding scope are _not_ black-boxed. Wrap them with -`divan::black_box()` or pass them through `with_inputs` instead: +The compiler can fold or remove work when it knows the inputs. Divan black-boxes inputs from +`with_inputs` and the closure's return value to prevent that optimization. Captured values do not +receive that protection: ```rust let array = make_array(); - -// ✗ `array` is captured — the compiler may optimize based on its known contents bencher.bench(|| process(&array)); +``` + +Pass the input through `with_inputs` instead: -// ✓ Option A: pass through with_inputs +```rust +let array = make_array(); bencher .with_inputs(|| &array) .bench_refs(|array| process(array)); +``` -// ✓ Option B: explicit black_box on the capture +An explicit `black_box` also protects a captured input: + +```rust bencher.bench(|| process(divan::black_box(&array))); ``` -### Return values and manual loops - -Return values are automatically black-boxed. You only need explicit -`black_box` for side-effect-free results inside manual loops: +Inside a manual loop, black-box each result so the compiler cannot discard earlier iterations: ```rust -bencher - .with_inputs(|| (&array, SESSION.create_execution_ctx())) - .bench_refs(|(array, ctx)| { - for idx in 0..len { - divan::black_box(array.execute_scalar(idx, ctx).unwrap()); - } - }); +bencher.bench(|| { + for value in divan::black_box(&values) { + divan::black_box(process(value)); + } +}); ``` -### Use deterministic, seeded RNG +### Choose reproducible inputs -Always use `StdRng::seed_from_u64(N)` for reproducible data generation: +Use a seeded generator such as `StdRng::seed_from_u64(0)` for random data. Unseeded random data can +change the measured work between runs even when the code stays the same. -```rust -let mut rng = StdRng::seed_from_u64(0); -``` +Choose inputs that exercise useful work. A zero-byte allocation or a compaction with nothing to move +can measure mostly the benchmark harness. For tiny operations, repeat a fixed amount of work and +black-box each result. Size inputs by bytes when comparing element types of different widths. + +Keep each iteration below 1 ms so the suite remains practical to run. This is the cost of one +closure invocation, not the total time of the benchmark binary. Check the per-iteration `HEAD` +value in the CodSpeed report before merging a new benchmark. Reduce the input size, split a larger +workload into parameterized cases, or keep it local if it cannot fit this budget. + +For CPU kernels, keep the working set within the runner's L2 cache unless memory bandwidth is the +intended measurement. `vortex-array/benches/filter_fixed_width.rs` shows how sizing by bytes keeps +different element widths above the harness cost without making the widest types too slow. -### Parameterize with `args`, `consts`, and `types` +### Parameterize workloads -Use Divan's parameterization features and define parameter arrays as named constants: +Use Divan's `args`, `consts`, and `types` for distinct workloads. Keep shared parameter lists in named +constants: ```rust const NUM_INDICES: &[usize] = &[1_000, 10_000, 100_000]; const VECTOR_SIZE: &[usize] = &[16, 256, 2048, 8192]; #[divan::bench(args = NUM_INDICES, consts = VECTOR_SIZE)] -fn my_bench(bencher: Bencher, num_indices: usize) { ... } +fn my_bench(bencher: Bencher, num_indices: usize) { /* ... */ } ``` -### Keep per-iteration execution time under 1 ms - -**1 ms is the maximum, not a soft target.** Each individual iteration of the benchmarked -closure must complete in less than 1 ms. This is to keep benchmarks snappy, locally and on -CI. - -A benchmark that needs longer than that is measuring too much work at once. Shrink the -input size until a single iteration fits, split it into smaller parameterized cases, or -gate it with `#[cfg(not(codspeed))]` if it genuinely cannot be made to fit. - -The number to check against the budget is the per-iteration time, not the time the whole -benchmark binary takes. CodSpeed reports exactly that: its performance report on a pull -request lists the per-iteration time under `HEAD` for every benchmark the pull request adds -or changes, so check any new benchmark there before merging. - -### Keep per-iteration work above the harness floor - -The budget has a floor as well as a ceiling. CodSpeed runs each benchmark once and adds a fixed -cost of roughly half a microsecond of reported time around the closure. A closure that does tens -of nanoseconds of real work, such as one small allocation or a fast path that finds nothing to -do, reports mostly that floor, and the floor moves by more than 10% between runs of identical -code. Such benchmarks flag regressions on pull requests that do not touch Rust at all. - -Aim for at least a few microseconds of real work per iteration: - -- When the operation itself is tiny, repeat it a fixed number of times inside the closure and - black-box each result, as `vortex-buffer/benches/allocation.rs` does. -- Drop degenerate inputs, such as a zero-byte allocation or a compaction with nothing to move. -- Size inputs by bytes rather than element count so narrow and wide types land in the same - range, as `vortex-array/benches/filter_fixed_width.rs` does. - -Benchmarks tagged `#[cpu_features]` run on the walltime legs instead, where the floor is timer -resolution and per-iteration jitter. Give each sample of those at least tens of microseconds, -with more work per iteration or with a fixed `sample_size` (see -[How CodSpeed measures a benchmark](#how-codspeed-measures-a-benchmark)). Keep the working set -inside the L2 cache of the leg machines (1 MiB on the Graviton leg) when the benchmark is about -kernel code rather than memory bandwidth. - -### Gate CodSpeed-incompatible benchmarks - -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. - -### How CodSpeed measures a benchmark - -Divan runs the benchmark closure in _iterations_ and groups them into _samples_. The _sample size_ -is the number of iterations in one sample. Divan times each sample as a whole and reports the time -per iteration. - -CodSpeed measures a benchmark with one of two instruments. Benchmarks without `#[cpu_features]` -run in _simulation_. Benchmarks with `#[cpu_features]` run on the _walltime legs_. - -**Simulation.** [CodSpeed's CPU simulation](https://codspeed.io/docs/instruments/cpu) runs each -benchmark **exactly once** and estimates CPU cycles from the instruction trace, including cache -and memory access costs. As a result: - -- `sample_count` and `sample_size` have no effect. -- System load does not change the result, because the estimate does not come from a clock. Some - benchmarks still switch between two values on code that did not change (see - [Known unstable benchmarks](#known-unstable-benchmarks)). -- System calls are not measured. A benchmark that depends on I/O or kernel calls **must** use the - [walltime instrument](https://codspeed.io/docs/instruments/walltime) or be gated with - `#[cfg(not(codspeed))]`. The simulation job keeps mimalloc and glibc from returning freed - memory to the OS, so that allocator system calls do not land in a measured iteration. - -**Walltime legs.** A walltime leg is one CI job that measures wall-clock time on a dedicated -machine for one CPU feature set (AVX2, AVX-512 or NEON). `.github/workflows/codspeed.yml` defines -the legs. On each leg: - -- Divan takes 1000 samples (`DIVAN_SAMPLE_COUNT` in the workflow). -- Divan picks the sample size from the first sample. If the first sample takes more than 100 times - the timer precision (about 3 µs), divan keeps one iteration per sample. The first sample is cold - and slow, so most walltime benchmarks run with one iteration per sample. -- CodSpeed compares the **fastest** sample of the pull request run with the fastest sample of the - base run, not the median. - -With one iteration per sample, the fastest sample is the single fastest iteration out of 1000, so -one unusually fast iteration decides the result. A benchmark close to the 3 µs limit can also get a -different sample size in each run, and each sample size reports a different time. - -For a short walltime benchmark, set `sample_size` in `#[divan::bench]` so that one sample takes -tens of microseconds, as `vortex-buffer/benches/collect_bool.rs` and -`encodings/fastlanes/benches/bitpack_blocked.rs` do. CodSpeed then compares the average of several -iterations, and the sample size is the same in every run. Write the value as a literal: the -simulation build ignores the setting, so a named constant is unused under `--cfg codspeed`. - -Do not set `DIVAN_SAMPLE_SIZE` in the workflow instead. The variable overrides the setting of every -benchmark, and divan keeps the inputs and outputs of a whole sample alive until the sample ends. A -large global sample size changes the memory behavior of the benchmarks that allocate large -outputs. - ### Use `mimalloc` as the global allocator -Every microbenchmark binary **must** use `mimalloc` as its global allocator. This includes the -CUDA and DuckDB benchmarks. Put these lines in the main file of each benchmark target, and keep -them when you rewrite the file: +Every microbenchmark binary must use `mimalloc`, including CUDA and DuckDB benchmarks: ```rust use mimalloc::MiMalloc; + #[global_allocator] static GLOBAL: MiMalloc = MiMalloc; ``` -The cost of the system allocator depends on its state, which differs between runner images and -between runs. Allocation inside a timed region made several simulation benchmarks flip between two -values on pull requests that did not change the measured code. `mimalloc` does the same work every -time, and one allocator for every binary means that no benchmark measures a different allocator -from its neighbors. Each crate with benchmarks already has the `mimalloc` dev-dependency. - -## What Not to Do - -The mistakes below have made CodSpeed report regressions on pull requests that did not change the -measured code. Such reports teach reviewers to ignore CodSpeed, and then real regressions pass -unnoticed. When a section above explains a rule, the rule links to that section. - -- **Do not write a benchmark that does tens of nanoseconds of work, or that uses degenerate input - such as a zero-byte allocation.** See - [Keep per-iteration work above the harness floor](#keep-per-iteration-work-above-the-harness-floor). -- **Do not leave a walltime benchmark of a few microseconds to divan's sample-size tuning.** See - [How CodSpeed measures a benchmark](#how-codspeed-measures-a-benchmark). -- **Do not judge a walltime benchmark by its median alone.** CodSpeed compares the fastest sample, - so a benchmark with a stable median can still report a 12% change. See - [How CodSpeed measures a benchmark](#how-codspeed-measures-a-benchmark). -- **Do not measure code that a pull request cannot change.** See - [Keep third-party and frozen baselines out of CodSpeed](#keep-third-party-and-frozen-baselines-out-of-codspeed). -- **Do not leave one-time process setup to the first measured iteration.** In simulation, each - benchmark runs one cold iteration. Lazy statics, such as the session id interner or the state - that the first regex compilation creates, then initialize inside whichever benchmark runs - first, often with system calls. Run the operation once in `main()` before `divan::main()`, as - `vortex-array/benches/like.rs` and `vortex-array/benches/run_end_filter.rs` do. -- **Do not drop the `mimalloc` global allocator when you rewrite a benchmark file.** See - [Use `mimalloc` as the global allocator](#use-mimalloc-as-the-global-allocator). -- **Do not read a one-time change in later benchmarks as a regression after you add or remove - benchmarks.** All benchmarks of one binary run in one process and share its allocator state, so - a change to the earlier benchmarks can move the later ones once. -- **Do not acknowledge a flagged benchmark in CodSpeed without a cause.** If the pull request - cannot reach the flagged code, open the log of the CodSpeed job and compare the divan table of - the run with the table of the base run. If the median did not move, the report comes from the - fastest-sample comparison. Fix the benchmark, or add it to the list below. - -### Known unstable benchmarks - -The benchmarks below switch between a fast state and a slow state for a whole run, on code that -did not change. The cause is not known. For the NEON `threshold` and `filter` cases, a larger -iteration does not help, because the slow state also slows the work for each element, not only a -fixed cost for each call. A report on one of these benchmarks is not evidence of a regression -unless the pull request changes the code that the benchmark measures. - -| Benchmark | Instrument | Observed values | -| --- | --- | --- | -| `bitpacking_take`: `threshold[8, 64, 80]` | NEON leg | 1.4 to 2 times slower in 15% to 30% of runs | -| `bitpacking_filter`: `filter[8, 64, 80]` | NEON leg | 1.4 to 2 times slower in 15% to 30% of runs | -| `binary_ops`: `mul_u64_nonnull` | NEON leg | About 20 µs, or about 40 µs in 17% of runs | -| `row_fn_utf8_input`: `starts_with[8 and 64, VarBin]` | AVX2 and AVX-512 legs | 12% to 17% slower in 40% to 50% of runs | -| `bitpacking_take`: `isolated_runs[u32, (16, 0)]` | Simulation | 78 µs or 88 µs | -| `chunk_array_builder`: `chunked_opt_bool_canonical_into[(1000, 10)]` | Simulation | 55 µs or 65.5 µs | +Each crate with benchmarks already has the dev-dependency. A consistent allocator removes one source +of differences between binaries, but allocation can still depend on process state and earlier +benchmarks. Keep the allocator declaration when rewriting a benchmark file. + +The simulation workflow disables mimalloc purging and limits glibc heap trimming to keep allocator +system calls out of measured iterations. These settings reduce noise but do not make allocation +independent of process history. A target's simulation benchmarks share one process, so adding, +removing, or reordering earlier benchmarks can change the allocator state seen by later ones. + +## CodSpeed + +[`.github/workflows/codspeed.yml`](https://github.com/vortex-data/vortex/blob/develop/.github/workflows/codspeed.yml) +defines the measurement jobs. Rust CPU benchmarks use two instruments: + +- [CPU simulation](https://codspeed.io/docs/instruments/cpu) runs benchmarks without + `#[vortex_bench_support::cpu_features]`. It estimates cycles from one execution's instruction + trace, including cache and memory access costs. Divan's sampling settings have no effect. +- [Walltime](https://codspeed.io/docs/instruments/walltime) runs CPU feature variants on dedicated + AVX2, AVX-512, and NEON machines. It measures elapsed time, so sampling and machine state matter. + +### Give short benchmarks enough measured work + +Simulation has a fixed harness cost. A single small allocation can take only tens of nanoseconds, +so most of its reported time can come from that cost. Aim for at least a few microseconds of useful +work per iteration. `vortex-buffer/benches/allocation.rs` repeats allocate-and-drop pairs a fixed +number of times and black-boxes each result to keep the operation visible above this cost. + +For walltime, Divan groups iterations into samples and reports time per iteration. CodSpeed compares +the fastest sample, so a stable median alone does not establish a repeatable benchmark. CI collects +1000 samples, but one unusually fast sample can still decide the result. + +Divan chooses the sample size from early timing measurements. A cold first iteration can be slow +enough that Divan keeps one iteration per sample even when later iterations are much faster. With +1000 single-iteration samples, the CodSpeed result is then the single fastest iteration. A case +near Divan's sampling cutoff can also get a different sample size in each run. + +For a short walltime benchmark, set a fixed `sample_size` so each sample takes at least tens of +microseconds. This averages several iterations and avoids changes caused by Divan choosing a +different sample size between runs: + +```rust +#[vortex_bench_support::cpu_features] +#[divan::bench(args = [64, 80], sample_size = 64)] +fn filter(bencher: Bencher, selected_per_chunk: usize) { + // ... +} +``` + +For example, the AVX2 bitpacked filter varied by 9.36% across 30 processes on three matching hosts +with one iteration per sample. Setting `sample_size = 64` reduced that spread to 1.45% with the same +executable. [PR #11110](https://github.com/vortex-data/vortex/pull/11110) records the configuration +and measurements. + +Choose the sample size for the operation. Do not set a large global `DIVAN_SAMPLE_SIZE`: Divan keeps +sample inputs and outputs alive until the sample ends, so batching changes memory use for operations +that allocate. Use a literal in the attribute because simulation ignores the setting and can leave +a named constant unused. + +### Keep incompatible benchmarks out of automatic runs + +Simulation excludes system-call time. Use walltime for operations that depend on I/O or kernel calls. +Memory mapping and disabling background workers do not remove runtime polling on the calling thread. +Moving an operation entirely into memory changes what the benchmark measures. + +Use automatic reports to track maintained Vortex entry points. Keep third-party kernels and frozen +implementations available for local comparisons, gated with `#[cfg(not(codspeed))]` and without +`#[vortex_bench_support::cpu_features]`. These baselines help compare implementations locally, but +unexplained changes in their automatic results do not identify a change in maintained Vortex code. + +Use `#[cfg(not(codspeed))]` for a Rust benchmark that cannot produce useful CodSpeed measurements. +For a tagged benchmark, `#[vortex_bench_support::cpu_features(exclude = ["neon"])]` skips the named +walltime leg while keeping the other legs and local runs. When only one input case is unreliable, +filter the benchmark's argument list instead. Record the reason beside the exclusion and keep the +supporting measurements in the PR. Keep CI commands independent of individual benchmark names. + +### Investigate reports on unchanged code + +Check the exact base and head commits and the measurement instrument before attributing a change. +An old comparison base can include unrelated changes. For walltime, compare the Divan tables in the +job logs, including the minimum and the median. For simulation, inspect the callgraph for changes in +measured work, allocation, or runtime polling. + +If the median stays stable while the minimum moves, investigate sampling before treating the alert +as an operation becoming slower. If several later benchmarks shift after an earlier benchmark is +added or removed, investigate shared allocator state. Neither pattern proves the cause on its own. +Simulation avoids clock noise, but changes in allocation or runtime polling can still change the +instruction trace on unchanged code. + +Repeat the same executable in fresh processes on matching hardware to check measurement noise, then +compare independent builds to check sensitivity to code placement. Keep commands, sampling settings, +and environment fixed. One repeatable executable does not establish that unrelated source changes +cannot affect its measurements. + +Record the executable identity, base and head commits, hardware, commands, and environment with the +results. Use the instrument's metric, such as minimum sample time for walltime, rather than process +elapsed time. Check spread across processes with `max / min - 1`. Include enough repeats and hosts +to expose occasional slow states, rather than relying on several samples from one process. + +If a case remains noisy after fixing setup and sampling, exclude it from automatic runs with the +evidence in the PR. Before restoring it, demonstrate repeatability and check that a deliberate +increase in measured work remains visible. Stable results are not useful if the measurement no +longer responds to a real regression. Do not raise the regression threshold to hide noise. ## SQL Benchmarks diff --git a/encodings/fastlanes/benches/bitpacking_filter.rs b/encodings/fastlanes/benches/bitpacking_filter.rs index 6c8a283a6dd..d9d6ecac46f 100644 --- a/encodings/fastlanes/benches/bitpacking_filter.rs +++ b/encodings/fastlanes/benches/bitpacking_filter.rs @@ -69,8 +69,10 @@ fn fixture(selected_per_chunk: usize) -> (ArrayRef, Mask) { (packed, Mask::from_indices(LEN, indices)) } -#[vortex_bench_support::cpu_features] -#[divan::bench(args = [64, 80])] +// Batch short iterations to reduce variation in the fastest walltime sample. +// The NEON cases still vary on unchanged code, so keep those measurements local. +#[vortex_bench_support::cpu_features(exclude = ["neon"])] +#[divan::bench(args = [64, 80], sample_size = 64)] fn filter(bencher: Bencher, selected_per_chunk: usize) { let (packed, mask) = fixture(selected_per_chunk); diff --git a/encodings/fastlanes/benches/bitpacking_take.rs b/encodings/fastlanes/benches/bitpacking_take.rs index 4bf53292e8a..271dc955f64 100644 --- a/encodings/fastlanes/benches/bitpacking_take.rs +++ b/encodings/fastlanes/benches/bitpacking_take.rs @@ -83,8 +83,10 @@ fn threshold_fixture(selected_per_chunk: usize) -> (ArrayRef, ArrayRef) { (packed, indices) } -#[vortex_bench_support::cpu_features] -#[divan::bench(args = [64, 80])] +// Batch short iterations to reduce variation in the fastest walltime sample. +// The NEON cases still vary on unchanged code, so keep those measurements local. +#[vortex_bench_support::cpu_features(exclude = ["neon"])] +#[divan::bench(args = [64, 80], sample_size = 16)] fn threshold(bencher: Bencher, selected_per_chunk: usize) { let (packed, indices) = threshold_fixture(selected_per_chunk); diff --git a/vortex-array/benches/binary_ops.rs b/vortex-array/benches/binary_ops.rs index e688d2cc76e..8515956aef5 100644 --- a/vortex-array/benches/binary_ops.rs +++ b/vortex-array/benches/binary_ops.rs @@ -4,7 +4,7 @@ //! Benchmarks for the binary `Operator` path, over primitive, decimal, and boolean inputs. //! //! The primitive arithmetic and comparison cases carry `#[cpu_features]`, so they are -//! measured on every walltime CPU-feature leg rather than in simulation. Each is written +//! measured on walltime CPU-feature legs rather than in simulation. Each is written //! once and compiled differently per leg: the kernel underneath is a portable lane loop, and //! how wide the compiler vectorizes it is decided by the build flags, not by the source. //! @@ -237,7 +237,8 @@ fn mul_u32_nonnull(bencher: Bencher) { bench_primitive(bencher, lhs, rhs, Operator::Mul); } -#[vortex_bench_support::cpu_features] +// This case varies on unchanged code on the NEON leg. The cause is unknown. +#[vortex_bench_support::cpu_features(exclude = ["neon"])] #[divan::bench] fn mul_u64_nonnull(bencher: Bencher) { let lhs = primitive_u64_small_nonnull(1, I64_LEN).into_array(); diff --git a/vortex-array/benches/chunk_array_builder.rs b/vortex-array/benches/chunk_array_builder.rs index f259e09cac8..92cb559022f 100644 --- a/vortex-array/benches/chunk_array_builder.rs +++ b/vortex-array/benches/chunk_array_builder.rs @@ -63,7 +63,8 @@ fn chunked_bool_canonical_into(bencher: Bencher, (len, chunk_count): (usize, usi }) } -#[divan::bench(args = BENCH_ARGS)] +// The largest nullable case varies on unchanged code in simulation. The cause is unknown. +#[divan::bench(args = if cfg!(codspeed) { &[(10, 100), (100, 50)] } else { BENCH_ARGS })] fn chunked_opt_bool_canonical_into(bencher: Bencher, (len, chunk_count): (usize, usize)) { let chunk = make_opt_bool_chunks(len, chunk_count); diff --git a/vortex-array/benches/row_fn_utf8_input.rs b/vortex-array/benches/row_fn_utf8_input.rs index 792758b7719..cd559d3fb60 100644 --- a/vortex-array/benches/row_fn_utf8_input.rs +++ b/vortex-array/benches/row_fn_utf8_input.rs @@ -6,6 +6,8 @@ reason = "benchmark fixtures reduce values below 26 before narrowing" )] +use std::fmt::Display; +use std::fmt::Formatter; use std::sync::LazyLock; use divan::Bencher; @@ -63,6 +65,37 @@ enum Layout { const LAYOUTS: &[Layout] = &[Layout::VarBin, Layout::VarBinView]; +#[derive(Clone, Copy)] +struct Case { + len: usize, + layout: Layout, +} + +impl Display for Case { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + // Preserve the report names from the former const-length and layout parameters. + let layout = match self.layout { + Layout::VarBin => "VarBin", + Layout::VarBinView => "VarBinView", + }; + + write!(f, "{}, {layout}", self.len) + } +} + +fn cases() -> Vec { + VALUE_LENS + .iter() + .flat_map(|&len| LAYOUTS.iter().map(move |&layout| Case { len, layout })) + .filter(|case| { + // VarBin varies on unchanged code, except for short values on the NEON leg. + env!("VORTEX_BENCH_VARIANT") == "local" + || matches!(case.layout, Layout::VarBinView) + || (env!("VORTEX_BENCH_VARIANT") == "neon" && case.len == 8) + }) + .collect() +} + #[derive(Clone)] struct StartsWith; @@ -105,9 +138,9 @@ fn make_input(layout: Layout, len: usize) -> ArrayRef { } #[vortex_bench_support::cpu_features] -#[divan::bench(args = LAYOUTS, consts = VALUE_LENS)] -fn starts_with(bencher: Bencher, &layout: &Layout) { - let args = VecExecutionArgs::new(vec![make_input(layout, LEN)], ROWS); +#[divan::bench(args = cases())] +fn starts_with(bencher: Bencher, case: Case) { + let args = VecExecutionArgs::new(vec![make_input(case.layout, case.len)], ROWS); bencher .counter(ItemsCount::new(ROWS)) diff --git a/vortex-array/benches/take_chunked.rs b/vortex-array/benches/take_chunked.rs index 7882ddfb557..6932d8d1136 100644 --- a/vortex-array/benches/take_chunked.rs +++ b/vortex-array/benches/take_chunked.rs @@ -147,7 +147,7 @@ impl Display for Case { } } -// Chunk and index counts are sized to keep CodSpeed simulation under 1ms per case. +// Chunk and index counts cover routing costs while keeping each iteration below 1 ms. fn cases() -> Vec { let mut cases = Vec::new(); @@ -347,7 +347,10 @@ fn take_indices(case: Case) -> ArrayRef { } } -#[divan::bench(args = cases())] +// Materialization can grow mimalloc's page map through system calls. Measure its full cost. +// The x86 variants still vary between processes with larger samples. Keep those runs local. +#[vortex_bench_support::cpu_features(exclude = ["avx2", "avx512"])] +#[divan::bench(args = cases(), sample_size = 16)] fn take(bencher: Bencher, case: Case) { let values = chunked_values(case); let indices = take_indices(case); diff --git a/vortex-python/benchmark/test_scalar_at.py b/vortex-python/benchmark/test_scalar_at.py index 9d2f487d9da..977156c661e 100644 --- a/vortex-python/benchmark/test_scalar_at.py +++ b/vortex-python/benchmark/test_scalar_at.py @@ -1,6 +1,8 @@ # SPDX-License-Identifier: Apache-2.0 # SPDX-FileCopyrightText: Copyright the Vortex contributors +import os + import pyarrow as pa import pytest from pytest_benchmark.fixture import BenchmarkFixture @@ -8,13 +10,25 @@ import vortex as vx +@pytest.fixture +def scalar_vxf(request: pytest.FixtureRequest, vxf: vx.VortexFile) -> vx.VortexFile: + enabled = request.config.getoption("codspeed", default=False) or "CODSPEED_ENV" in os.environ + mode = request.config.getoption("codspeed_mode", default=None) + if mode is None and "CODSPEED_ENV" in os.environ: + mode = os.environ.get("CODSPEED_RUNNER_MODE", "simulation") + assert isinstance(request.node, pytest.Function) + if enabled and mode == "simulation" and request.node.callspec.params["vxf"] == "int": + pytest.skip("Reactor polling varies inside these scalar scans under simulation.") + return vxf + + @pytest.mark.benchmark(group="scalar_at") -def test_scan_scalar_at(benchmark: BenchmarkFixture, vxf: vx.VortexFile) -> None: +def test_scan_scalar_at(benchmark: BenchmarkFixture, scalar_vxf: vx.VortexFile) -> None: indices = vx.array([50_000]) - benchmark(lambda: pa.concat_tables(x.to_arrow_table() for x in vxf.scan(indices=indices))) + benchmark(lambda: pa.concat_tables(x.to_arrow_table() for x in scalar_vxf.scan(indices=indices))) @pytest.mark.benchmark(group="scalar_at") -def test_repeated_scan_scalar_at(benchmark: BenchmarkFixture, vxf: vx.VortexFile) -> None: - rscan = vxf.to_repeated_scan() +def test_repeated_scan_scalar_at(benchmark: BenchmarkFixture, scalar_vxf: vx.VortexFile) -> None: + rscan = scalar_vxf.to_repeated_scan() benchmark(lambda: rscan.scalar_at(50_000))