Skip to content

Commit 5abaf98

Browse files
connortsui20claude
andauthored
Fix flaky CodSpeed microbenchmarks (#8742)
A small pool of microbenchmarks flips ±10-35% between two fixed values on unrelated PRs (including docs-only and lockfile-only changes), spamming every CodSpeed report. This PR fixes them (and removes only 1 benchmark). No `#[cfg(codspeed)]` gating; CI and local runs stay identical. One commit per benchmark. ## Changes - **mimalloc as global allocator** in the 5 flaky `vortex-array` bench files: `chunk_array_builder`, `dict_compress`, `varbinview_compact`, `compare`, `binary_ops` - **`bitwise_not_vortex_buffer_mut`**: drop the 128/1024/2048 sizes (measured only harness overhead) - **`slice_empty_vortex`**: rewrite as a 1024-iteration tight loop, renamed `slice_empty_tight_loop_vortex` - **`rebuild_naive` (vortex-zstd)**: the one benchmark removed instead of fixed — its cost is dominated by zstd-internal copies (glibc `ifunc`-resolved `memcpy`) that no bench-level change can stabilize, its fixture is degenerate (a 4-string dictionary with all-zero offsets), and ListView rebuild is already benchmarked across element types and list shapes in `vortex-array/benches/listview_rebuild.rs`. The crate's now-unused `divan` dev-dependency goes with it. <details> <summary>Which benchmarks were flaky, and the evidence</summary> Identified by reading the CodSpeed comments on the ~47 PRs merged since June 25 (post-#8490). The tell: the same benchmark flipping between the same two values, in both directions, on PRs that can't have affected it — including deny.toml-only (#8712, #8716), uv.lock-only (#8732), docs-only (#8737, #8728, #8685), and CI-YAML-only (#8660, #8683) changes. | Benchmark | Evidence | |---|---| | `bitwise_not_vortex_buffer_mut[128/1024/2048]` | ~half of all PRs — worst offender | | `chunked_varbinview_*` ×4 | ~20 PRs, both directions | | `chunked_bool_canonical_into[(1000,10)]` | ~2× flips (16µs ↔ 35µs) on 4 PRs | | `encode_varbin`, `encode_varbinview` | ~12 PRs; `encode_varbinview[(10000,4)]` also flipped on an earlier revision of this PR | | `compact`, `compact_sliced` (90%-utilization args) | ~8 PRs | | `compare_int_constant` | ±11.1% verbatim on ≥9 PRs | | `eq_i64_constant` | same ±11% signature incl. docs-only PR | | `slice_empty_vortex` | -14.66% verbatim on ~13 PRs | | `rebuild_naive` (vortex-zstd) | ~10 PRs, both directions | Watch list (left alone, below the ≥3-independent-sightings bar): `copy_nullable`/`copy_non_nullable[65536]` in `cast_decimal.rs`, `true_count_vortex_buffer[128]`. </details> <details> <summary>Root causes and why each fix matches</summary> - **Allocation in the timed region** → glibc malloc's code differs across CodSpeed runner images, so alloc-heavy benchmarks trace differently for byte-identical Vortex code. Vendored mimalloc removes glibc malloc from the trace. Empirical support: the only three bench files already using mimalloc (`single_encoding_throughput`, `common_encoding_tree_throughput`, `row_encode`) are the most alloc-heavy suites in the repo and were never flagged once in the 47-PR window. - **Sub-microsecond work** → the measurement is fixed harness overhead plus binary code layout, which shifts with any unrelated change. Fix by making the operation dominate: the in-place NOT (no alloc, no copy) keeps only sizes where the loop dominates; the empty slice runs 1024× per iteration, mirroring the neighboring `slice_tight_loop_vortex`. - **Environment-bound and low-signal** → `rebuild_naive`, per the justification above: unfixable at the bench level and redundant with the vortex-array ListView rebuild suite, so removed. </details> <details> <summary>Validation: A/A reruns and a stacked canary PR</summary> - **Expected one-time step changes on this PR**: swapping the allocator changes the trace of every benchmark in the touched files, so this PR's report shows a few ±10-20% level shifts (including on never-flaky `encode_primitives`, which just shares a file). These need a one-time acknowledgment on the CodSpeed dashboard; after merge every PR compares mimalloc-vs-mimalloc. - **A/A reruns** (same bench binaries measured three times, on separate runners and commits): every value reproduced exactly — 211.5µs, 137.1µs, 14.6µs, 26.3µs — with zero new flags across 1656 benchmarks. - **Canary #8743** (a #8681-style cold-string change stacked on this branch — the class of change that used to collect five false flags): **Performance Gate Passed**, `✅ 1660 untouched`, zero changes reported. </details> No public API changes; benchmark-only. https://claude.ai/code/session_01T6PPcdrqcNeUkfi1EGd4oC --------- Signed-off-by: Claude <noreply@anthropic.com> Co-authored-by: Claude <noreply@anthropic.com>
1 parent 2f18a4c commit 5abaf98

11 files changed

Lines changed: 37 additions & 67 deletions

File tree

Cargo.lock

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

encodings/zstd/Cargo.toml

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,10 +34,5 @@ vortex-session = { workspace = true }
3434
zstd = { workspace = true }
3535

3636
[dev-dependencies]
37-
divan = { workspace = true }
3837
rstest = { workspace = true }
3938
vortex-array = { workspace = true, features = ["_test-harness"] }
40-
41-
[[bench]]
42-
name = "listview_rebuild"
43-
harness = false

encodings/zstd/benches/listview_rebuild.rs

Lines changed: 0 additions & 58 deletions
This file was deleted.

vortex-array/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@ serde = ["dep:serde", "vortex-buffer/serde", "vortex-mask/serde"]
8585
divan = { workspace = true }
8686
futures = { workspace = true, features = ["executor"] }
8787
insta = { workspace = true }
88+
mimalloc = { workspace = true }
8889
rand_distr = { workspace = true }
8990
rstest = { workspace = true }
9091
serde_json = { workspace = true }

vortex-array/benches/binary_ops.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ use std::sync::LazyLock;
1111

1212
use divan::Bencher;
1313
use divan::counter::ItemsCount;
14+
use mimalloc::MiMalloc;
1415
use vortex_array::ArrayRef;
1516
use vortex_array::Executable;
1617
use vortex_array::IntoArray;
@@ -23,6 +24,9 @@ use vortex_array::builtins::ArrayBuiltins;
2324
use vortex_array::scalar_fn::fns::operators::Operator;
2425
use vortex_session::VortexSession;
2526

27+
#[global_allocator]
28+
static GLOBAL: MiMalloc = MiMalloc;
29+
2630
fn main() {
2731
LazyLock::force(&SESSION);
2832
divan::main();

vortex-array/benches/chunk_array_builder.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
use std::sync::LazyLock;
55

66
use divan::Bencher;
7+
use mimalloc::MiMalloc;
78
use rand::RngExt;
89
use rand::SeedableRng;
910
use rand::prelude::StdRng;
@@ -22,6 +23,9 @@ use vortex_array::dtype::DType;
2223
use vortex_error::VortexExpect;
2324
use vortex_session::VortexSession;
2425

26+
#[global_allocator]
27+
static GLOBAL: MiMalloc = MiMalloc;
28+
2529
fn main() {
2630
LazyLock::force(&SESSION);
2731
divan::main();

vortex-array/benches/compare.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
#![expect(clippy::unwrap_used)]
55

66
use divan::Bencher;
7+
use mimalloc::MiMalloc;
78
use rand::RngExt;
89
use rand::SeedableRng;
910
use rand::distr::Uniform;
@@ -25,6 +26,9 @@ use vortex_array::scalar_fn::fns::operators::Operator;
2526
use vortex_array::validity::Validity;
2627
use vortex_buffer::Buffer;
2728

29+
#[global_allocator]
30+
static GLOBAL: MiMalloc = MiMalloc;
31+
2832
fn main() {
2933
divan::main();
3034
}

vortex-array/benches/dict_compress.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
use std::sync::LazyLock;
77

88
use divan::Bencher;
9+
use mimalloc::MiMalloc;
910
use rand::distr::Distribution;
1011
use rand::distr::StandardUniform;
1112
use vortex_array::Canonical;
@@ -20,6 +21,9 @@ use vortex_array::builders::dict::dict_encode;
2021
use vortex_array::dtype::NativePType;
2122
use vortex_session::VortexSession;
2223

24+
#[global_allocator]
25+
static GLOBAL: MiMalloc = MiMalloc;
26+
2327
fn main() {
2428
LazyLock::force(&SESSION);
2529
divan::main();

vortex-array/benches/varbinview_compact.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
use std::sync::LazyLock;
55

66
use divan::Bencher;
7+
use mimalloc::MiMalloc;
78
use rand::RngExt;
89
use rand::SeedableRng;
910
use rand::rngs::StdRng;
@@ -19,6 +20,9 @@ use vortex_buffer::Buffer;
1920
use vortex_error::VortexExpect;
2021
use vortex_session::VortexSession;
2122

23+
#[global_allocator]
24+
static GLOBAL: MiMalloc = MiMalloc;
25+
2226
fn main() {
2327
divan::main();
2428
}

vortex-buffer/benches/vortex_bitbuffer.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -246,7 +246,12 @@ fn bitwise_not_vortex_buffer(bencher: Bencher, length: usize) {
246246
.bench_values(|buffer| !&buffer);
247247
}
248248

249-
#[divan::bench(args = INPUT_SIZE)]
249+
/// The in-place NOT on an owned `BitBufferMut` performs no allocation and no copy, so below a
250+
/// few thousand bits the measurement is fixed harness overhead and binary code layout rather
251+
/// than the loop itself. Only the sizes where the loop dominates are worth measuring.
252+
const NOT_MUT_INPUT_SIZE: &[usize] = &[16_384, 65_536];
253+
254+
#[divan::bench(args = NOT_MUT_INPUT_SIZE)]
250255
fn bitwise_not_vortex_buffer_mut(bencher: Bencher, length: usize) {
251256
bencher
252257
.with_inputs(|| BitBufferMut::from_iter((0..length).map(|i| i % 2 == 0)))

0 commit comments

Comments
 (0)