perf(sequence): build single-match bitmap directly - #9262
Conversation
| } | ||
|
|
||
| const NUM_VALUES: u64 = 100_000; | ||
| const SEQUENCE_COMPARE_LENGTHS: [usize; 3] = [1_000, 100_000, 1_000_000]; |
There was a problem hiding this comment.
Switched both comparison cases to the existing NUM_VALUES workload in b7f33d0.
There was a problem hiding this comment.
please read the benchmarking docs
There was a problem hiding this comment.
Read docs/developer-guide/benchmarking.md and updated both cases to pass the length and match index through with_inputs, removing the captured values and std::hint::black_box. The focused run measured 19.99 microseconds median for from_iter and 87.3 nanoseconds for the single-set path, both below the 1 ms per-iteration limit.
| let mut buffer = BitBufferMut::new_unset(lhs.len()); | ||
| buffer.set(set_idx); | ||
| let buffer = buffer.freeze(); |
b7f33d0 to
e1d3874
Compare
Merging this PR will degrade performance by 12.1%
Warning Please fix the performance issues or acknowledge them on CodSpeed. Performance Changes
Tip Investigate this regression by commenting Comparing Footnotes
|
|
I dug into the CodSpeed report and pushed the focused follow-up through |
Signed-off-by: ahmad <ahmadalgaidy@hotmail.com>
Signed-off-by: ahmad <ahmadalgaidy@hotmail.com>
This reverts commit e1d3874. Signed-off-by: ahmad <ahmadalgaidy@hotmail.com>
Signed-off-by: ahmad <ahmadalgaidy@hotmail.com>
Signed-off-by: ahmad <ahmadalgaidy@hotmail.com>
cdcd3bf to
f325e4d
Compare
Summary
The sequence equality kernel knows that a match bitmap contains exactly one set bit, but it currently evaluates a predicate for every row to build that bitmap. This change allocates an unset
BitBufferMut, sets the known match index, and freezes it. The PR stays limited to the production comparison path after removing unrelated benchmark-harness edits.Testing
rustfmtonencodings/sequence/src/compute/compare.rsgit diff --checkcargo test -p vortex-sequence --lib --no-default-features compute::compare::testswas attempted twice but could not complete because the shallow checkout needs the uncachedtpchgenGit dependency.AI assistance
This PR was prepared with agentic AI assistance. I inspected the issue and surrounding implementation, kept the change scoped to the requested path, and recorded the local validation limitation above.
Fixes #9092