Fix flaky CodSpeed microbenchmarks#8742
Merged
robert3005 merged 10 commits intoJul 14, 2026
Merged
Conversation
Merging this PR will not alter performance
Warning Please fix the performance issues or acknowledge them on CodSpeed. Performance Changes
Tip Investigate this regression by commenting Comparing Footnotes
|
bitwise_not_vortex_buffer_mut[128/1024/2048] was the noisiest benchmark on CodSpeed after #8490: it flipped between the same two values in both directions (+27.1%/-10.66% etc.) on roughly half of ~47 recently merged PRs, including PRs changing only docs, uv.lock, or workflow YAML. The Not impl for an owned BitBufferMut runs bitwise_unary_op_mut in place - no allocation, no memcpy. At 128-2048 bits that is a handful of word ops, so the reported number is fixed divan harness overhead plus binary code layout, both of which shift with any unrelated code change. Those sizes never measured the operation at all, so remove them outright; 16384 and 65536, where the loop dominates, were never flagged and remain. Signed-off-by: Claude <noreply@anthropic.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T6PPcdrqcNeUkfi1EGd4oC
slice_empty_vortex measured a single Buffer::slice(8..8) - a few nanoseconds of real work - so the reported ~340ns was dominated by fixed harness overhead and it flipped -14.66%/-15.83% verbatim on at least a dozen recently merged PRs that never touched vortex-buffer (docs-only, CI-YAML-only, issue-template PRs among them). Fix it the same way the file already handles cheap slices: run the operation in a tight loop (1024 empty slices per iteration, mirroring slice_tight_loop_vortex) so the slice cost dominates the measurement. Renamed to slice_empty_tight_loop_vortex to match its sibling and so CodSpeed treats it as a new benchmark rather than reporting a bogus step change on the old name. Signed-off-by: Claude <noreply@anthropic.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T6PPcdrqcNeUkfi1EGd4oC
e74ee1a to
daeb6c0
Compare
The chunked_varbinview_{canonical_into,into_canonical,opt_canonical_into,
opt_into_canonical} benches were flagged on ~20 of ~47 recently merged PRs
and chunked_bool_canonical_into[(1000, 10)] flipped ~2x (16us <-> 35us) on
four more, always between the same two values and always on PRs that never
touched canonicalization (deny.toml-only and uv.lock-only PRs included).
These benches allocate and grow builder buffers inside the timed region, so
the simulated trace runs through glibc malloc, whose code differs across
CodSpeed runner images. Routing allocation through mimalloc removes glibc
malloc from the trace entirely: the allocator is vendored and identical in
every build. The three bench files that already use 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 same 47-PR window, which is strong evidence this is the right fix. Also
what docs/developer-guide/benchmarking.md already prescribes for
allocation-heavy benchmarks.
Signed-off-by: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T6PPcdrqcNeUkfi1EGd4oC
encode_varbin[(1000, *)] flipped between ~138us and ~155-160us on about a dozen recently merged PRs (including docs-only #8737 and uv.lock-only #8732), encode_varbinview[(10000, 2)] on several more, and encode_varbinview [(10000, 4)] flipped -10.1% on an earlier revision of this very PR - which does not touch dict encoding. String dict encoding is hash-table growth plus builder-buffer growth, i.e. allocation-dominated, so the trace inherits glibc malloc's per-runner-image code differences. Route it through vendored mimalloc, as the never-flagged alloc-heavy bench suites already do. Signed-off-by: Claude <noreply@anthropic.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T6PPcdrqcNeUkfi1EGd4oC
compact[(4096, 90)] and compact_sliced[(4096, 90)] (occasionally the 16384 variants) flipped +/-10-16% between the same two values on ~8 recently merged PRs, including CI-YAML-only and FFI-only changes. At 90% buffer utilization compaction copies almost nothing, so the trace is mostly the allocation of the compacted buffers - glibc malloc code that differs per runner image. Route allocation through vendored mimalloc like the never-flagged throughput suites. Signed-off-by: Claude <noreply@anthropic.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T6PPcdrqcNeUkfi1EGd4oC
compare_int_constant reported a near-identical +/-11.1% flip on at least nine recently merged PRs spanning docs-only, JNI-packaging, file-layout, and DuckDB-binding changes, none of which touch the compare kernel. The timed region allocates the output BoolArray buffer through glibc malloc, whose code differs across CodSpeed runner images; the 65k-element compare loop itself is compute-bound and stable. Route allocation through vendored mimalloc. Signed-off-by: Claude <noreply@anthropic.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T6PPcdrqcNeUkfi1EGd4oC
eq_i64_constant showed the same ~+11% two-state flip as compare_int_constant, most conclusively on docs-only #8737. Like compare.rs, the timed region allocates the output array through glibc malloc while the kernel itself is compute-bound, so route allocation through vendored mimalloc. Signed-off-by: Claude <noreply@anthropic.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T6PPcdrqcNeUkfi1EGd4oC
rebuild_naive flipped between ~91us and ~105us (reported as -10% to -16% regressions or +15% improvements depending on the baseline) on about ten recently merged PRs that never touched vortex-zstd or ListView rebuild, including a docs-only Spark PR (#8728) and a build.rs-only CUDA PR (#8708). The benchmark cannot be stabilized: its trace is dominated by zstd's internal window/output copies (glibc memcpy resolved per runner image via ifunc), not by Vortex code. It is also of little value: the fixture is degenerate - a 4-string dictionary with all-zero offsets - and the rebuild paths it nominally covers are benchmarked properly across element types and list shapes in vortex-array/benches/listview_rebuild.rs. Remove it rather than keep a noise source; the divan dev-dependency goes with it since this was the crate's only benchmark. Signed-off-by: Claude <noreply@anthropic.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T6PPcdrqcNeUkfi1EGd4oC
daeb6c0 to
a97990f
Compare
mimalloc added as a vortex-array dev-dependency; divan removed from vortex-zstd along with its only benchmark. Signed-off-by: Claude <noreply@anthropic.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T6PPcdrqcNeUkfi1EGd4oC
Same tree as the previous commit. If the mimalloc fix is sound, this run must reproduce the previous run's values for the benchmarks it touched (e.g. chunked_varbinview_into_canonical[(1000, 10)] at 211.5us); a flip would mean the bimodality survives mimalloc and the benchmark should be removed instead. This commit can be dropped before the PR leaves draft. Signed-off-by: Claude <noreply@anthropic.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T6PPcdrqcNeUkfi1EGd4oC
4efeb0d to
1a06040
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
vortex-arraybench files:chunk_array_builder,dict_compress,varbinview_compact,compare,binary_opsbitwise_not_vortex_buffer_mut: drop the 128/1024/2048 sizes (measured only harness overhead)slice_empty_vortex: rewrite as a 1024-iteration tight loop, renamedslice_empty_tight_loop_vortexrebuild_naive(vortex-zstd): the one benchmark removed instead of fixed — its cost is dominated by zstd-internal copies (glibcifunc-resolvedmemcpy) 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 invortex-array/benches/listview_rebuild.rs. The crate's now-unuseddivandev-dependency goes with it.Which benchmarks were flaky, and the evidence
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.
bitwise_not_vortex_buffer_mut[128/1024/2048]chunked_varbinview_*×4chunked_bool_canonical_into[(1000,10)]encode_varbin,encode_varbinviewencode_varbinview[(10000,4)]also flipped on an earlier revision of this PRcompact,compact_sliced(90%-utilization args)compare_int_constanteq_i64_constantslice_empty_vortexrebuild_naive(vortex-zstd)Watch list (left alone, below the ≥3-independent-sightings bar):
copy_nullable/copy_non_nullable[65536]incast_decimal.rs,true_count_vortex_buffer[128].Root causes and why each fix matches
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.slice_tight_loop_vortex.rebuild_naive, per the justification above: unfixable at the bench level and redundant with the vortex-array ListView rebuild suite, so removed.Validation: A/A reruns and a stacked canary PR
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.✅ 1660 untouched, zero changes reported.No public API changes; benchmark-only.
https://claude.ai/code/session_01T6PPcdrqcNeUkfi1EGd4oC