Performance: ListViewArray zero-copyable to ListArray optimizations (#5129)

Tracking Issue: #4699
Closes: #5053

This PR introduces a performance optimization for `ListViewArray` by
adding an `is_zero_copy_to_list` flag that tracks whether the
`ListViewArray` can be (near) constant-time converted to a `ListArray`
without copying data.

When this flag is true (indicating sorted offsets with no gaps and no
overlaps), conversions can bypass the very expensive rebuild process
(which just calls `append_scalar` in a loop).

Additionally, this PR refactors all `ListViewArray` constructor call
sites and compute operations throughout the codebase to properly
maintain this new invariant. The only time you can set this flag is via
the `new_unchecked` constructor which is unsafe, which means the caller
better be careful to know what they are doing.

Some things that are not included in this PR:
- Making conversion _actually_ zero-copy would require implementing
logic to always allocate an additional `offsets` slot for an `n+1`th
offset in `ListViewArray`. This is because `ListArray` has `n+1`
offsets, and from a plain `ListViewArray` we have to reallocate the
offsets to make space.
- `reset_offsets` right now is probably more expensive than it could be
since it uses the `sub_scalar` compute function. We know in advance that
`reset_offsets` should _never_ fail in the non-recursive case.
- Validation of this `is_zctl` flag is **SUPER** expensive, but it can
be made a bit less expensive by using a `BitBuffer` to track references
+ a flag for overlaps.
- It would probably be nice to have an `check_is_zero_copy_to_list`
function on `ListViewArray` that sets the flag if it detects it is
zero-copyable to a `ListArray`. This would be nice when we are reading
from disk or converting from an Arrow `GenericListView`.
- This PR **does not** change the on-disk format, which means we lose
this metadata when writing to disk. I think that this flag is somewhat
similar to the `is_sorted` statistic, but at the same time I feel like
it is not correct to store this as a statistic? Not sure.

---------

Signed-off-by: Connor Tsui <[email protected]>

Author: connortsui20

encodings/sparse/src/canonical.rs

@@ -931,9 +931,11 @@ mod test {
         // List 3: [2] at offset 3, size 1
         let offsets = buffer![0u32, 1, 2, 3].into_array();
         let sizes = buffer![1u32, 1, 1, 1].into_array();
-        let lists = ListViewArray::try_new(elements, offsets, sizes, Validity::AllValid)
-            .unwrap()
-            .into_array();
+        let lists = unsafe {
+            ListViewArray::new_unchecked(elements, offsets, sizes, Validity::AllValid)
+                .with_zero_copy_to_list(true)
+        }
+        .into_array();
 
         let indices = buffer![0u8, 3u8, 4u8, 5u8].into_array();
         let fill_value = Scalar::null(lists.dtype().clone());
@@ -978,9 +980,11 @@ mod test {
         let elements = buffer![1i32, 2, 1, 2, 1, 2, 1, 2].into_array();
         let offsets = buffer![0u32, 1, 2, 3, 4, 5, 6, 7].into_array();
         let sizes = buffer![1u32, 1, 1, 1, 1, 1, 1, 1].into_array();
-        let lists = ListViewArray::try_new(elements, offsets, sizes, Validity::AllValid)
-            .unwrap()
-            .into_array();
+        let lists = unsafe {
+            ListViewArray::new_unchecked(elements, offsets, sizes, Validity::AllValid)
+                .with_zero_copy_to_list(true)
+        }
+        .into_array();
 
         // Slice to get lists 2..6, which are: [1], [2], [1], [2]
         let lists = lists.slice(2..6);
@@ -1022,9 +1026,11 @@ mod test {
         let elements = buffer![1i32, 2, 1, 2].into_array();
         let offsets = buffer![0u32, 1, 2, 3].into_array();
         let sizes = buffer![1u32, 1, 1, 1].into_array();
-        let lists = ListViewArray::try_new(elements, offsets, sizes, Validity::AllValid)
-            .unwrap()
-            .into_array();
+        let lists = unsafe {
+            ListViewArray::new_unchecked(elements, offsets, sizes, Validity::AllValid)
+                .with_zero_copy_to_list(true)
+        }
+        .into_array();
 
         let indices = buffer![0u8, 3u8, 4u8, 5u8].into_array();
         let fill_value = Scalar::from(Some(vec![5i32, 6, 7, 8]));
@@ -1384,8 +1390,10 @@ mod test {
         let offsets = buffer![0u32, 3, 5].into_array();
         let sizes = buffer![3u32, 2, 4].into_array();
 
-        let list_view =
-            ListViewArray::try_new(elements.clone(), offsets, sizes, Validity::AllValid).unwrap();
+        let list_view = unsafe {
+            ListViewArray::new_unchecked(elements.clone(), offsets, sizes, Validity::AllValid)
+                .with_zero_copy_to_list(true)
+        };
 
         let list_dtype = list_view.dtype().clone();
 
@@ -1460,9 +1468,11 @@ mod test {
         let offsets = buffer![0u32, 2, 5, 6, 8].into_array();
         let sizes = buffer![2u32, 3, 1, 2, 2].into_array();
 
-        let full_listview = ListViewArray::try_new(elements, offsets, sizes, Validity::AllValid)
-            .unwrap()
-            .into_array();
+        let full_listview = unsafe {
+            ListViewArray::new_unchecked(elements, offsets, sizes, Validity::AllValid)
+                .with_zero_copy_to_list(true)
+        }
+        .into_array();
 
         // Slice to get lists 1, 2, 3 (indices 1..4)
         // This gives us lists with elements:

fuzz/src/array/mask.rs

@@ -57,13 +57,18 @@ pub fn mask_canonical_array(canonical: Canonical, mask: &Mask) -> VortexResult<A
         }
         Canonical::List(array) => {
             let new_validity = apply_mask_to_validity(array.validity(), mask);
-            ListViewArray::try_new(
-                array.elements().clone(),
-                array.offsets().clone(),
-                array.sizes().clone(),
-                new_validity,
-            )
-            .vortex_unwrap()
+
+            // SAFETY: Since we are only masking the validity and everything else comes from an
+            // already valid `ListViewArray`, all of the invariants are still upheld.
+            unsafe {
+                ListViewArray::new_unchecked(
+                    array.elements().clone(),
+                    array.offsets().clone(),
+                    array.sizes().clone(),
+                    new_validity,
+                )
+                .with_zero_copy_to_list(array.is_zero_copy_to_list())
+            }
             .into_array()
         }
         Canonical::FixedSizeList(array) => {
@@ -243,9 +248,10 @@ mod tests {
         let elements = PrimitiveArray::from_iter([1i32, 2, 3, 4, 5, 6]).into_array();
         let offsets = PrimitiveArray::from_iter([0i32, 2, 4]).into_array();
         let sizes = PrimitiveArray::from_iter([2i32, 2, 2]).into_array();
-        let array =
-            ListViewArray::try_new(elements, offsets, sizes, Nullability::NonNullable.into())
-                .unwrap();
+        let array = unsafe {
+            ListViewArray::new_unchecked(elements, offsets, sizes, Nullability::NonNullable.into())
+                .with_zero_copy_to_list(true)
+        };
 
         let mask = Mask::from_iter([false, true, false]);
 

fuzz/src/array/slice.rs

@@ -74,7 +74,14 @@ pub fn slice_canonical_array(
             // Since the list view elements can be stored out of order, we cannot slice it.
             let elements = list_array.elements().clone();
 
-            ListViewArray::try_new(elements, offsets, sizes, validity).map(|a| a.into_array())
+            // SAFETY: If the array was already zero-copyable to list, slicing the offsets and sizes
+            // only causes there to be leading and trailing garbage data, which is still
+            // zero-copyable to a `ListArray`.
+            Ok(unsafe {
+                ListViewArray::new_unchecked(elements, offsets, sizes, validity)
+                    .with_zero_copy_to_list(list_array.is_zero_copy_to_list())
+            }
+            .into_array())
         }
         DType::FixedSizeList(..) => {
             let fsl_array = array.to_fixed_size_list();

vortex-array/src/arrays/chunked/vtable/canonical.rs

@@ -5,7 +5,9 @@ use vortex_buffer::BufferMut;
 use vortex_dtype::{DType, Nullability, PType, StructFields};
 use vortex_error::VortexExpect;
 
-use crate::arrays::{ChunkedArray, ChunkedVTable, ListViewArray, PrimitiveArray, StructArray};
+use crate::arrays::{
+    ChunkedArray, ChunkedVTable, ListViewArray, ListViewRebuildMode, PrimitiveArray, StructArray,
+};
 use crate::builders::{ArrayBuilder, builder_with_capacity};
 use crate::compute::cast;
 use crate::validity::Validity;
@@ -121,6 +123,9 @@ fn swizzle_list_chunks(
 
     for chunk in chunks {
         let chunk_array = chunk.to_listview();
+        // By rebuilding as zero-copy to `List` and trimming all elements (to prevent gaps), we make
+        // the final output `ListView` also zero-copyable to `List`.
+        let chunk_array = chunk_array.rebuild(ListViewRebuildMode::MakeExact);
 
         // Add the `elements` of the current array as a new chunk.
         list_elements_chunks.push(chunk_array.elements().clone());
@@ -163,7 +168,12 @@ fn swizzle_list_chunks(
     // - Each `offset[i] + size[i]` list view is within bounds of elements array because it came
     //   from valid chunks
     // - Validity came from the outer chunked array so it must have the same length
-    unsafe { ListViewArray::new_unchecked(chunked_elements, offsets, sizes, validity) }
+    // - Since we made sure that all chunks were zero-copyable to a list above, we know that the
+    //   final concatenated output is also zero-copyable to a list.
+    unsafe {
+        ListViewArray::new_unchecked(chunked_elements, offsets, sizes, validity)
+            .with_zero_copy_to_list(true)
+    }
 }
 
 #[cfg(test)]

vortex-array/src/arrays/list/array.rs

@@ -4,7 +4,7 @@
 use std::sync::Arc;
 
 use num_traits::AsPrimitive;
-use vortex_dtype::{DType, match_each_integer_ptype, match_each_native_ptype};
+use vortex_dtype::{DType, NativePType, match_each_integer_ptype, match_each_native_ptype};
 use vortex_error::{VortexExpect, VortexResult, vortex_bail, vortex_ensure};
 
 use crate::arrays::{ListVTable, PrimitiveVTable};
@@ -191,8 +191,11 @@ impl ListArray {
 
                 vortex_ensure!(
                     max_offset
-                        <= P::try_from(elements.len())
-                            .vortex_expect("Offsets type must be able to fit elements length"),
+                        <= P::try_from(elements.len()).vortex_expect(&format!(
+                            "Offsets type {} must be able to fit elements length {}",
+                            <P as NativePType>::PTYPE,
+                            elements.len()
+                        )),
                     "Max offset {max_offset} is beyond the length of the elements array {}",
                     elements.len()
                 );
@@ -266,6 +269,11 @@ impl ListArray {
         &self.elements
     }
 
+    // TODO(connor)[ListView]: Create 2 functions `reset_offsets` and `recursive_reset_offsets`,
+    // where `reset_offsets` is infallible.
+    // Also, `reset_offsets` can be made more efficient by replacing `sub_scalar` with a match on
+    // the offset type and manual subtraction and fast path where `offsets[0] == 0`.
+
     /// Create a copy of this array by adjusting `offsets` to start at `0` and removing elements not
     /// referenced by the `offsets`.
     pub fn reset_offsets(&self, recurse: bool) -> VortexResult<Self> {