Skip to content

Commit f0743ef

Browse files
committed
address
Signed-off-by: Robert Kruszewski <github@robertk.io>
1 parent 8c26832 commit f0743ef

1 file changed

Lines changed: 83 additions & 11 deletions

File tree

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

vortex-array/src/arrays/listview/compute/zip.rs

Lines changed: 83 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -96,16 +96,22 @@ impl ZipKernel for ListView {
9696
let offsets_out = offsets.spare_capacity_mut();
9797
let sizes_out = sizes.spare_capacity_mut();
9898

99-
// We matched `Mask::Values` above, so the bit buffer is materialized. Walk it as 64-bit
100-
// chunks and branchlessly blend both sides per row, letting the compiler vectorize the
101-
// inner select instead of mispredicting a data-dependent branch per element.
99+
// We matched `Mask::Values` above, so the bit buffer is materialized. `unaligned_chunks`
100+
// iterates faster than `chunks`: it exposes the byte-aligned body as a plain `&[u64]`
101+
// with no per-word reshifting, isolating any bit misalignment into a leading `prefix`
102+
// and trailing `suffix` word. We blend both sides branchlessly per row so the compiler
103+
// vectorizes the inner select instead of mispredicting a data-dependent branch.
102104
let mask_bits = mask
103105
.values()
104106
.vortex_expect("mask is Mask::Values")
105107
.bit_buffer();
106-
let chunks = mask_bits.chunks();
108+
let unaligned = mask_bits.unaligned_chunks();
109+
// The prefix word's low `lead` bits are padding; shifting them out aligns row 0 to bit 0,
110+
// after which every chunk and the suffix start cleanly on a row boundary.
111+
let lead = unaligned.lead_padding();
107112

108-
let mut select_block = |word: u64, base: usize, end: usize| {
113+
let mut select_block = |word: u64, base: usize, n: usize| {
114+
let end = base + n;
109115
// `if_false` views address the second half of the concatenated elements, so shift
110116
// their offsets by `false_shift`; sizes are taken verbatim from the chosen side.
111117
select_column(
@@ -125,14 +131,17 @@ impl ZipKernel for ListView {
125131
};
126132

127133
let mut base = 0;
128-
for word in chunks.iter() {
129-
select_block(word, base, base + 64);
134+
if let Some(prefix) = unaligned.prefix() {
135+
let n = (64 - lead).min(len);
136+
select_block(prefix >> lead, base, n);
137+
base += n;
138+
}
139+
for &word in unaligned.chunks() {
140+
select_block(word, base, 64);
130141
base += 64;
131142
}
132-
133-
let remainder = chunks.remainder_len();
134-
if remainder > 0 {
135-
select_block(chunks.remainder_bits(), base, base + remainder);
143+
if let Some(suffix) = unaligned.suffix() {
144+
select_block(suffix, base, len - base);
136145
}
137146
}
138147

@@ -454,6 +463,69 @@ mod tests {
454463
Ok(())
455464
}
456465

466+
/// A mask whose bit buffer starts at a non-byte-aligned offset (here from slicing a bool array)
467+
/// has non-zero `unaligned_chunks` lead padding, exercising the prefix word alongside the
468+
/// aligned chunk body and the suffix.
469+
#[test]
470+
fn zip_handles_offset_mask() -> VortexResult<()> {
471+
// 200 single-element lists per side: `if_true[i] = [i]`, `if_false[i] = [1000 + i]`. With a
472+
// 3-bit lead offset the mask spans more than 16 bytes, so `unaligned_chunks` exposes a
473+
// non-empty aligned `chunks` body between the prefix and suffix words.
474+
let len = 200usize;
475+
let true_elements: Vec<i32> = (0..len as i32).collect();
476+
let false_elements: Vec<i32> = (0..len as i32).map(|i| 1000 + i).collect();
477+
let offsets: Vec<u64> = (0..len as u64).collect();
478+
let sizes: Vec<u64> = vec![1; len];
479+
480+
let single_element_view = |elements: &[i32]| {
481+
list_view(
482+
elements
483+
.iter()
484+
.copied()
485+
.collect::<Buffer<i32>>()
486+
.into_array(),
487+
offsets
488+
.iter()
489+
.copied()
490+
.collect::<Buffer<u64>>()
491+
.into_array(),
492+
sizes.iter().copied().collect::<Buffer<u64>>().into_array(),
493+
Validity::NonNullable,
494+
)
495+
};
496+
let if_true = single_element_view(&true_elements);
497+
let if_false = single_element_view(&false_elements);
498+
499+
// Slice off the first `offset` bits so the mask's bit buffer keeps a sub-byte offset while
500+
// remaining `len` rows long. A non-trivial pattern straddles the prefix/body and chunk
501+
// boundaries within the sliced window.
502+
let offset = 3usize;
503+
let mask_bits: Vec<bool> = (0..offset + len)
504+
.map(|i| i.is_multiple_of(3) || i == offset + 64)
505+
.collect();
506+
let mask = BoolArray::from_iter(mask_bits.iter().copied())
507+
.into_array()
508+
.slice(offset..offset + len)?;
509+
510+
let mut ctx = LEGACY_SESSION.create_execution_ctx();
511+
let result = mask.zip(if_true, if_false)?.execute::<ArrayRef>(&mut ctx)?;
512+
assert!(result.is::<ListView>());
513+
514+
// Each row collapses to a single element: `i` when the sliced mask is set, else `1000 + i`.
515+
let expected_elements: Vec<i32> = (0..len)
516+
.map(|i| {
517+
if mask_bits[offset + i] {
518+
i as i32
519+
} else {
520+
1000 + i as i32
521+
}
522+
})
523+
.collect();
524+
let expected = single_element_view(&expected_elements);
525+
assert_arrays_eq!(result, expected, &mut ctx);
526+
Ok(())
527+
}
528+
457529
/// When an input's `elements` is already a [`ChunkedArray`], its chunks are spliced in rather
458530
/// than nesting a chunked array inside the concatenated elements.
459531
#[test]

0 commit comments

Comments
 (0)