Skip to content

Commit 86c3568

Browse files
authored
Filter indices without an iterator for Bool (#7176)
## Summary Noticed this difference as part of #6740 that changed some of the inlining around `collect_bool`, making the closure not inlined. The comment over `filter_indices` used to be on a different function, and the iterator and generics seems wasteful. This seems to be about 30µs faster than develop pre #6470 for most affected benchmarks, which is about 3% (probably within the noise, but the affect seems very consistent). Signed-off-by: Adam Gutglick <adam@spiraldb.com>
1 parent ce9f13d commit 86c3568

1 file changed

Lines changed: 12 additions & 22 deletions

File tree

  • vortex-array/src/arrays/bool/compute

vortex-array/src/arrays/bool/compute/filter.rs

Lines changed: 12 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -28,11 +28,7 @@ impl FilterReduce for Bool {
2828
.vortex_expect("AllTrue and AllFalse are handled by filter fn");
2929

3030
let buffer = match mask_values.threshold_iter(FILTER_SLICES_DENSITY_THRESHOLD) {
31-
MaskIter::Indices(indices) => filter_indices(
32-
&array.to_bit_buffer(),
33-
mask.true_count(),
34-
indices.iter().copied(),
35-
),
31+
MaskIter::Indices(indices) => filter_indices(&array.to_bit_buffer(), indices),
3632
MaskIter::Slices(slices) => filter_slices(
3733
&array.to_bit_buffer(),
3834
mask.true_count(),
@@ -44,24 +40,18 @@ impl FilterReduce for Bool {
4440
}
4541
}
4642

47-
/// Select indices from a boolean buffer.
48-
/// NOTE: it was benchmarked to be faster using collect_bool to index into a slice than to
49-
/// pass the indices as an iterator of usize. So we keep this alternate implementation.
50-
pub fn filter_indices(
51-
bools: &BitBuffer,
52-
indices_len: usize,
53-
mut indices: impl Iterator<Item = usize>,
54-
) -> BitBuffer {
43+
fn filter_indices(bools: &BitBuffer, indices: &[usize]) -> BitBuffer {
5544
let buffer = bools.inner().as_ref();
56-
BitBuffer::collect_bool(indices_len, |_idx| {
57-
let idx = indices
58-
.next()
59-
.vortex_expect("iterator is guaranteed to be within the length of the array.");
60-
get_bit(buffer, bools.offset() + idx)
45+
let offset = bools.offset();
46+
BitBuffer::collect_bool(indices.len(), |idx| {
47+
// Safety:
48+
// We iterate over the slice's length.
49+
let idx = unsafe { indices.get_unchecked(idx) } + offset;
50+
get_bit(buffer, idx)
6151
})
6252
}
6353

64-
pub fn filter_slices(
54+
fn filter_slices(
6555
buffer: &BitBuffer,
6656
indices_len: usize,
6757
slices: impl Iterator<Item = (usize, usize)>,
@@ -76,13 +66,13 @@ pub fn filter_slices(
7666

7767
#[cfg(test)]
7868
mod test {
69+
7970
use itertools::Itertools;
8071
use vortex_mask::Mask;
8172

73+
use super::*;
8274
use crate::IntoArray;
8375
use crate::arrays::BoolArray;
84-
use crate::arrays::bool::compute::filter::filter_indices;
85-
use crate::arrays::bool::compute::filter::filter_slices;
8676
use crate::assert_arrays_eq;
8777
use crate::compute::conformance::filter::test_filter_conformance;
8878

@@ -107,7 +97,7 @@ mod test {
10797
fn filter_bool_by_index_test() {
10898
let arr = BoolArray::from_iter([true, true, false]);
10999

110-
let filtered = filter_indices(&arr.to_bit_buffer(), 2, [0, 2].into_iter());
100+
let filtered = filter_indices(&arr.to_bit_buffer(), &[0, 2]);
111101
assert_eq!(vec![true, false], filtered.iter().collect_vec())
112102
}
113103

0 commit comments

Comments
 (0)