Skip to content

Commit 6c7b88d

Browse files
connortsui20claude
andauthored
Fix the two root causes of CodSpeed flakes: substituted baselines and harness-dominated benchmarks (#9011)
## Rationale for this change Follow-up to #8861 / #8807. Two causes behind the remaining flakes. **Benchmarks measuring divan's harness, not the operation.** An iteration carries ~146 instructions of fixed harness, and only benchmarks below that were ever flagged. `true_count` was 78% / 64% harness at 1024 / 2048 bits, so those sizes are gone. `slice` is O(1) — every size measured identically at 125 Ir, which is why #8749 reported the same value for `[1024]` and `[16384]` — so it's one length, batched. `binary_search` measured 203 vs 206 Ir for the two impls, indistinguishable; batched they separate at 15,133 vs 10,010. **CI attributing other commits' changes to a PR.** Serialised develop baselines meant CodSpeed substituted an older base and said so — *"No successful run was found on `develop`…"* — on 4 of 12 open-PR reports. Now one group per commit, which does let develop runs overlap on a spot pool. Separately the CUDA filter matched `.github/workflows/**`, so 11 dependency bumps ran the walltime suite; now just `codspeed.yml`. The first report here was an instance: a YAML-only commit flagged three benchmarks that vanished once the base was correct. ## What changes are included in this PR? Three files. Checks: callgrind before and after each fix, `clippy --benches`, `+nightly fmt`, smoke runs, `yamllint --strict`. Rejected after testing: `codegen-units = 1`, and tight-looping `true_count`. Numbers are from a dev container, not a CodSpeed runner. --------- Signed-off-by: Claude <noreply@anthropic.com> Co-authored-by: Claude <noreply@anthropic.com>
1 parent 6609288 commit 6c7b88d

3 files changed

Lines changed: 69 additions & 34 deletions

File tree

.github/workflows/codspeed.yml

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,13 @@ name: Codspeed Benchmarking
22

33
# Concurrency control:
44
# - PRs: new commits on a feature branch will cancel in-progress (outdated) runs.
5-
# - Push to develop: runs queue sequentially, never cancelled. This allows us to have benchmarks
6-
# run on every commit for our benchmarks website.
5+
# - Push to develop: every commit gets its own group, so baseline runs never cancel and never
6+
# queue behind each other. Serialising them meant a burst of merges left later commits without
7+
# a finished baseline, so CodSpeed fell back to an older comparison base and reported changes
8+
# unrelated to the PR being tested.
79
# - `workflow_dispatch`: groups by branch and queues if run on develop.
810
concurrency:
9-
group: ${{ github.workflow }}-${{ github.ref }}
11+
group: ${{ github.workflow }}-${{ github.ref }}-${{ github.event_name == 'push' && github.sha || '' }}
1012
cancel-in-progress: ${{ github.ref != 'refs/heads/develop' }}
1113
on:
1214
push:
@@ -40,7 +42,8 @@ jobs:
4042
filters: |
4143
cuda:
4244
- "vortex-cuda/**"
43-
- ".github/workflows/**"
45+
# Only this workflow defines the CUDA benchmark jobs.
46+
- ".github/workflows/codspeed.yml"
4447
4548
bench-codspeed:
4649
strategy:

vortex-array/benches/search_sorted.rs

Lines changed: 30 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -15,27 +15,49 @@ fn main() {
1515
divan::main();
1616
}
1717

18+
/// One search over 65536 elements is ~16 comparisons, small enough that the measurement is mostly
19+
/// harness overhead and the two implementations below become indistinguishable. Search a batch per
20+
/// iteration instead; the targets differ, so the calls cannot be folded together.
21+
const SEARCH_TARGETS: usize = 64;
22+
1823
#[divan::bench]
1924
fn binary_search_std(bencher: Bencher) {
20-
let (sorted_array, target) = fixture();
25+
let (sorted_array, targets) = fixture();
2126
bencher
22-
.with_inputs(|| (&sorted_array, &target))
23-
.bench_refs(|(array, target)| array.binary_search(target));
27+
.with_inputs(|| (&sorted_array, &targets))
28+
.bench_refs(|(array, targets)| {
29+
let mut found = 0;
30+
for target in targets.iter() {
31+
found += array.binary_search(target).unwrap_or_else(|idx| idx);
32+
}
33+
found
34+
});
2435
}
2536

2637
#[divan::bench]
2738
fn binary_search_vortex(bencher: Bencher) {
28-
let (sorted_array, target) = fixture();
39+
let (sorted_array, targets) = fixture();
2940
bencher
30-
.with_inputs(|| (&sorted_array, &target))
31-
.bench_refs(|(array, target)| array.search_sorted(target, SearchSortedSide::Left).unwrap());
41+
.with_inputs(|| (&sorted_array, &targets))
42+
.bench_refs(|(array, targets)| {
43+
let mut found = 0;
44+
for target in targets.iter() {
45+
found += array
46+
.search_sorted(target, SearchSortedSide::Left)
47+
.unwrap()
48+
.to_index();
49+
}
50+
found
51+
});
3252
}
3353

34-
fn fixture() -> (Vec<i32>, i32) {
54+
fn fixture() -> (Vec<i32>, Vec<i32>) {
3555
let mut rng = StdRng::seed_from_u64(0);
3656
let range = Uniform::new(0, 65_536).unwrap();
3757
let mut data: Vec<i32> = (0..65_536).map(|_| rng.sample(range)).collect();
3858
data.sort();
3959

40-
(data, rng.sample(range))
60+
let targets = (0..SEARCH_TARGETS).map(|_| rng.sample(range)).collect();
61+
62+
(data, targets)
4163
}

vortex-buffer/benches/vortex_bitbuffer.rs

Lines changed: 32 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -153,33 +153,43 @@ fn value_arrow_buffer(bencher: Bencher, length: usize) {
153153
});
154154
}
155155

156-
#[divan::bench(args = INPUT_SIZE)]
157-
fn slice_vortex_buffer(bencher: Bencher, length: usize) {
158-
let buffer = BitBuffer::from_iter((0..length).map(|i| i % 2 == 0));
159-
bencher
160-
.with_inputs(|| (&buffer, length / 2))
161-
.bench_refs(|(buffer, mid)| {
162-
let mid = *mid;
163-
buffer.slice(mid / 2..mid + mid / 2)
164-
});
156+
/// Slicing only adjusts an offset, a length and a refcount, so its cost is independent of buffer
157+
/// length. Measure one length, with enough slices per iteration to stay above harness overhead.
158+
const SLICE_ITERS: usize = 64;
159+
const SLICE_INPUT_SIZE: usize = 65_536;
160+
161+
#[divan::bench]
162+
fn slice_vortex_buffer(bencher: Bencher) {
163+
let buffer = BitBuffer::from_iter((0..SLICE_INPUT_SIZE).map(|i| i % 2 == 0));
164+
let mid = SLICE_INPUT_SIZE / 2;
165+
bencher.with_inputs(|| &buffer).bench_refs(|buffer| {
166+
let mut total = 0;
167+
for offset in 0..SLICE_ITERS {
168+
total += buffer.slice(mid / 2 + offset..mid + mid / 2).len();
169+
}
170+
total
171+
});
165172
}
166173

167174
#[cfg(not(codspeed))]
168-
#[divan::bench(args = INPUT_SIZE)]
169-
fn slice_arrow_buffer(bencher: Bencher, length: usize) {
170-
let buffer = Arrow(BooleanBuffer::from_iter((0..length).map(|i| i % 2 == 0)));
171-
bencher
172-
.with_inputs(|| (&buffer, length / 2))
173-
.bench_refs(|(buffer, mid)| {
174-
let mid = *mid;
175-
buffer.0.slice(mid / 2, mid / 2)
176-
});
175+
#[divan::bench]
176+
fn slice_arrow_buffer(bencher: Bencher) {
177+
let buffer = Arrow(BooleanBuffer::from_iter(
178+
(0..SLICE_INPUT_SIZE).map(|i| i % 2 == 0),
179+
));
180+
let mid = SLICE_INPUT_SIZE / 2;
181+
bencher.with_inputs(|| &buffer).bench_refs(|buffer| {
182+
let mut total = 0;
183+
for offset in 0..SLICE_ITERS {
184+
total += buffer.0.slice(mid / 2 + offset, mid / 2).len();
185+
}
186+
total
187+
});
177188
}
178189

179-
/// A 128-bit `true_count` is a popcount over two `u64` words, so the measurement is fixed
180-
/// dispatch and harness overhead plus binary code layout rather than the count itself. Only
181-
/// sizes where the popcount loop dominates are worth measuring.
182-
const TRUE_COUNT_INPUT_SIZE: &[usize] = &[1024, 2048, 16_384, 65_536];
190+
/// Below a few thousand bits the measurement is mostly divan's fixed per-iteration overhead
191+
/// rather than the popcount, so it moves when the count itself has not changed.
192+
const TRUE_COUNT_INPUT_SIZE: &[usize] = &[16_384, 65_536];
183193

184194
#[divan::bench(args = TRUE_COUNT_INPUT_SIZE)]
185195
fn true_count_vortex_buffer(bencher: Bencher, length: usize) {

0 commit comments

Comments
 (0)