Repository navigation
Fix walltime benchmark flakes and document what not to do - #10808
Conversation
CodSpeed compares the fastest walltime sample, and divan runs most walltime benchmarks with one iteration per sample. Pin `sample_size` on `bitpack_blocked_compress`, whose fastest iteration flipped between two values while its median stayed within 1%, and on `words_gather_dispatch`, where divan picked 1, 2 or 4 iterations per sample from run to run and each choice reported its own time. Restore the `mimalloc` allocator in `bitpacking_filter.rs`, which a rewrite dropped. The benchmarking guide now describes how CodSpeed measures walltime benchmarks, corrects the claim that `sample_size` has no effect (true only in simulation), adds a "What Not to Do" section, and lists the benchmarks that switch between two states for reasons not yet known. Claude-Session: https://claude.ai/code/session_01KF3pMFx7U5ngk8BzQQFS2W Signed-off-by: Claude <[email protected]>
Merging this PR will regress 2 benchmarks4 warnings, 1 note
Warning Please fix the performance issues or acknowledge them on CodSpeed. Performance Changes
Tip Investigate this regression by commenting Comparing Footnotes
|
Define iterations, samples and sample size before the guide relies on them, record why a global `DIVAN_SAMPLE_SIZE` was rejected, and turn the "What Not to Do" items into rules that link to the section that owns each explanation. Every microbenchmark binary now must use `mimalloc`, stated in the microbenchmark introduction and in its own section. The two `sample_size` comments now keep only the local reason and the effect of changing the value, and link the guide and the measurements. Claude-Session: https://claude.ai/code/session_01KF3pMFx7U5ngk8BzQQFS2W Signed-off-by: Claude <[email protected]>
Short walltime benchmarks can report changes on unchanged code because CodSpeed compares the fastest sample. Batch the bitpacked filter and take samples, and keep unreliable cases local. Exclusions live beside the benchmark definitions, using `#[cpu_features(exclude = ["neon"])]` for whole CPU variants and source-level selection for individual inputs. CI commands stay generic. Move the chunked-take family to walltime with 16 iterations per sample so it measures the operation's allocator and syscall costs. Keep its x86 variants local after repeatability checks failed; all 27 NEON cases remain in CI. The guide retains practical examples and measurement guidance without an exclusion catalog or a repeatability tool. <details> <summary>Measurement evidence</summary> Spread is `max / min - 1` across independent processes, with 1000 Divan samples per process and ten processes on each of three matching hosts. | Case | Before | Fixed sampling | | --- | --- | --- | | AVX2 `filter[80]`, sample size 64 | 9.36% | 1.45% | | AVX-512 `threshold[80]`, sample size 16 | 8.26% | 1.69% | Across independent base and head AVX2 builds, the fixed filter's worst spread was 4.66% over 60 processes. AVX-512 filter cases stayed within 1.78%. The excluded NEON string case still varied by 6.38%. Measurements used existing CI executables with matching sample-size overrides from [run 41449104113](https://github.com/vortex-data/vortex/actions/runs/41449104113) (`44bdefff6e69`) and [run 41442182591](https://github.com/vortex-data/vortex/actions/runs/41442182591) (`7c9bb58e39fe`), on three `c7i.metal-24xl` hosts for x86 and three `c7g.metal` hosts for NEON. These measure repeatability, not kernel speedups. Other Rust exclusions follow unexplained variation recorded in [#10808](#10808) and repeated on the workflow-only [#11095](#11095 (comment)). The two Python integer scalar scans include variable calling-thread reactor polling, visible in [#11102](#11102), even with memory mapping and background workers disabled. The chunked-take warning on `dfb32c5`, compared with `d8e7467`, came from mimalloc lazily allocating page-map metadata. The affected `small_m/shuffled/primitive/nonnull/chunks=2048/indices=16` case made one `mmap` and one `prctl` call totaling 23.3 µs; the base trace made neither. An untimed warm-up removed the syscall warning but produced eight regression alerts with unchanged kernel code under allocation-excluded simulation. Walltime measures elapsed time without that allocation exclusion and establishes separate per-CPU baselines. Walltime repeats used the CI executables from `29920deef0`, 1000 samples per process, and ten independent processes on each of three matching hosts per architecture. All 27 NEON cases stayed within 4.35% spread; the originally flagged case stayed within 2.88%. The worst x86 spreads were 11.04% for AVX2 and 11.20% for AVX-512. Increasing the sample size to 64 on two troublesome x86 cases still produced 12.15% and 13.38% spread, so those CPU variants remain local through the macro exclusion. </details> Focused native and simulation Clippy checks passed. Test-mode runs confirmed the local, AVX2, and NEON string case selection and the macro's NEON exclusion. Pinned-nightly formatting, Ruff, ty, YAML linting, and patch whitespace checks passed. All 66 CI checks passed on `29920deef0`, including CodSpeed analysis with zero syscall warnings. The final x86 exclusion passed focused native and simulation Clippy checks and test-mode checks for both excluded variants. CI is rerunning for that final change. Local Git hooks were bypassed. AI assistance: Codex helped investigate, implement and prepare this PR. --------- Signed-off-by: Connor Tsui <[email protected]>
Summary
CodSpeed reported a regression on 49 of 68 recent PRs that changed no benchmarked Rust code. On the walltime legs it compares the fastest sample, and divan runs most benchmarks there with one iteration per sample, so one fast iteration out of 1000 can decide the result. Expect a one-time change in the two pinned series on this PR's own report.
Changes
A fixed
sample_sizemakes each sample the average of several iterations and stops divan from picking a different sample size per run. Those were the two causes behindbitpack_blocked_compress(flagged on 148 PRs) andwords_gather_dispatch(58).bitpacking_filter.rsgets back themimallocallocator that a rewrite dropped, and the guide now makesmimallocmandatory and lists what not to do, so the same flakes stop coming back.The NEON
threshold/filter,mul_u64_nonnullandstarts_with[VarBin]benchmarks also switch between two states for a whole run, but the cause is not known and a larger iteration does not help. The guide lists them as known unstable instead of changing them.Evidence and checks
fastestcolumn in 108 of 122 checkable cases, and the median in 9.bitpack_blocked_compress(AVX2): the median stayed within 1% over 100 runs. The fastest iteration was near 6.7 µs in 29 runs and near 7.55 µs in the rest.words_gather_dispatch(NEON): divan picked one iteration per sample in 42 runs (1.98 to 2.01 µs) and two in 57 runs (1.80 to 1.82 µs).Checks run:
cargo clippy --locked -p vortex-buffer -p vortex-fastlanes --benches --features vortex-fastlanes/_test-harness -- -D warningsRUSTFLAGS="--cfg codspeed -D warnings" cargo checkon the same targets. The touched files are clean.canonicalize_bench.rs(untouched) has unused-import errors under this configuration.cargo +nightly-2026-09-10 fmt --check,typos,git diff --checkNot run: the CodSpeed jobs (they run on this PR) and the Sphinx docs build.
https://claude.ai/code/session_01KF3pMFx7U5ngk8BzQQFS2W
Generated by Claude Code