perf(sparse): build the fixed-size-list fill tile once, not once per gap

`append_fill` handed the fill scalar to `Constant::append_to_builder` per gap,
which materialized the scalar's elements into an array every time. Hoist that
out of the loop the way the list path already does: the fill's elements become
an array once, up front, and every gap tiles that same array through
`FixedSizeListBuilder::append_array_as_repeated_list`.

Elements that are all the same scalar stay a constant array, so tiling them
over a gap costs nothing at all however many rows it covers. Anything else is
copied in per row, because a fixed-size list holds its elements back to back
and cannot point its rows at one shared copy.

`canonicalize_sparse_fixed_size_list` medians, against develop:

  (512, 7, 4)      21.1 us -> 25.6 us   (was 55.0 us)
  (1024, 17, 8)    29.1 us -> 32.9 us   (was 63.9 us)
  (8192, 1024, 4) 150.1 us -> 140.1 us  (was 142.3 us)

What is left of the gap against develop is the per-row copy: each row pays its
own append for `list_size` elements, where tiling a run could double the region
already written instead.

Signed-off-by: Robert Kruszewski <robert@spiraldb.com>

Author: robert3005

encodings/sparse/src/canonical.rs

@@ -368,6 +368,12 @@ fn execute_sparse_fixed_size_list_inner<I: IntegerPType>(
         nullability,
         array_len,
     );
+    // The fill's elements become an array once, up front, so that a gap does not rebuild them.
+    // They are tiled per row rather than shared - a fixed-size list holds its elements back to
+    // back - unless they are all the same scalar, in which case the tile stays constant-encoded
+    // and the tiling costs nothing.
+    let fill_elements = fixed_size_list_fill_tile(fill_value.as_list(), list_size);
+
     // One mask for the whole patch array rather than a validity lookup per patch.
     let patch_validity = values
         .validity()
@@ -384,7 +390,12 @@ fn execute_sparse_fixed_size_list_inner<I: IntegerPType>(
         let sparse_idx = sparse_idx
             .to_usize()
             .vortex_expect("patch index must fit in usize");
-        append_fill(&mut builder, fill_value, sparse_idx - next_index, ctx);
+        append_fixed_size_list_fill(
+            &mut builder,
+            fill_elements.as_ref(),
+            sparse_idx - next_index,
+            ctx,
+        );
 
         // Take each patch's elements rather than slicing the patch array itself: slicing a
         // `FixedSizeList` slices its elements and its validity, and every one of those slices pays
@@ -404,38 +415,56 @@ fn execute_sparse_fixed_size_list_inner<I: IntegerPType>(
     }
 
     // Fill remaining positions after last patch.
-    append_fill(&mut builder, fill_value, array_len - next_index, ctx);
+    append_fixed_size_list_fill(
+        &mut builder,
+        fill_elements.as_ref(),
+        array_len - next_index,
+        ctx,
+    );
 
     builder.finish_into_fixed_size_list()
 }
 
-/// Appends the run of `count` fill values that covers the gap before the next patch.
+/// Materializes the elements a fixed-size-list fill value covers each of its rows with, or `None`
+/// if the fill is null.
 ///
-/// The run goes in as a single [`ConstantArray`] rather than one appended value per row. For a
-/// list dtype that is the difference between storing the fill value once per gap and once per row:
-/// canonicalizing a constant list array points every view at one copy of the value, and the
-/// builder keeps that layout.
-fn append_fill(
-    builder: &mut dyn ArrayBuilder,
-    fill_value: &Scalar,
+/// Elements that are all the same scalar stay a constant array, so tiling them over a gap costs
+/// nothing however many rows it covers.
+fn fixed_size_list_fill_tile(fill: ListScalar, list_size: u32) -> Option<ArrayRef> {
+    let elements = fill.elements()?;
+
+    Some(match elements.iter().all_equal_value() {
+        Ok(uniform) => ConstantArray::new(uniform.clone(), list_size as usize).into_array(),
+        Err(_) => {
+            let mut builder = builder_with_capacity(fill.element_dtype(), elements.len());
+            for element in &elements {
+                builder
+                    .append_scalar(element)
+                    .vortex_expect("fixed-size-list element scalar was invalid");
+            }
+            builder.finish()
+        }
+    })
+}
+
+/// Appends the run of `count` fill lists that covers the gap before the next patch.
+fn append_fixed_size_list_fill(
+    builder: &mut FixedSizeListBuilder,
+    fill_elements: Option<&ArrayRef>,
     count: usize,
     ctx: &mut ExecutionCtx,
 ) {
     if count == 0 {
         return;
     }
 
-    if fill_value.is_null() {
-        // A null fill has no elements to share, and the builder can record the nulls without
-        // going through an array at all.
-        builder.append_nulls(count);
-        return;
+    match fill_elements {
+        Some(fill_elements) => builder
+            .append_array_as_repeated_list(fill_elements, count, ctx)
+            .vortex_expect("Failed to append sparse fixed-size-list fill value"),
+        // A null fill has no elements of its own, only the placeholders the builder writes.
+        None => builder.append_nulls(count),
     }
-
-    ConstantArray::new(fill_value.clone(), count)
-        .into_array()
-        .append_to_builder(builder, ctx)
-        .vortex_expect("Failed to append sparse fill value");
 }
 
 fn execute_sparse_bools(

vortex-array/src/builders/fixed_size_list.rs

@@ -107,19 +107,63 @@ impl FixedSizeListBuilder {
         Ok(())
     }
 
-    /// Appends the same fixed-size list `value` `n` times.
+    /// Appends `array` as `n` identical non-null lists.
     ///
-    /// A fixed-size list array holds its elements back to back, so a run of `n` identical lists is
-    /// the value's elements tiled `n` times - there is no layout that lets the rows share one copy
-    /// the way a list view's can. Two cases avoid paying for the tiling anyway:
+    /// A fixed-size list array holds its elements back to back, so `n` identical lists are the
+    /// array's elements tiled `n` times - there is no layout that lets the rows share one copy the
+    /// way a list view's can, so the tile is copied in per row. A constant tile is the exception:
+    /// tiling it gives another constant array, so it goes in as a single chunk and no elements are
+    /// materialized at all.
     ///
-    /// - a null value needs no elements at all, only placeholders, which
-    ///   [`append_nulls`](ArrayBuilder::append_nulls) writes in bulk;
-    /// - a value whose elements are all the same scalar tiles to a constant array, which goes in as
-    ///   a single chunk.
+    /// A caller with a run of appends to make should hand over the same `array` each time rather
+    /// than rebuild it, which is the whole reason this takes an array instead of a scalar.
+    pub fn append_array_as_repeated_list(
+        &mut self,
+        array: &ArrayRef,
+        n: usize,
+        ctx: &mut ExecutionCtx,
+    ) -> VortexResult<()> {
+        vortex_ensure!(
+            array.dtype() == self.element_dtype(),
+            "Array dtype {:?} does not match list element dtype {:?}",
+            array.dtype(),
+            self.element_dtype()
+        );
+        vortex_ensure!(
+            array.len() == self.list_size() as usize,
+            "Array length {} does not match fixed list size {}",
+            array.len(),
+            self.list_size()
+        );
+
+        if n == 0 {
+            return Ok(());
+        }
+
+        let tiled_len = n * array.len();
+        match array.as_constant() {
+            Some(element) => {
+                let tiled = ConstantArray::new(element, tiled_len).into_array();
+                self.elements_builder.append_array(&tiled, ctx)?;
+            }
+            None => {
+                self.elements_builder.reserve_exact(tiled_len);
+                for _ in 0..n {
+                    self.elements_builder.append_array_values(array, ctx)?;
+                }
+            }
+        }
+        self.nulls.append_n_non_nulls(n);
+
+        Ok(())
+    }
+
+    /// Appends the same fixed-size list `value` `n` times.
     ///
-    /// Otherwise the tile is copied in per row. That is one copy of each element, where
-    /// canonicalizing the run first would build the whole tiled array and then copy it again.
+    /// This materializes the value's elements and hands them to
+    /// [`append_array_as_repeated_list`](Self::append_array_as_repeated_list) - as a constant array
+    /// when they are all the same scalar, so that the tiling costs nothing. A caller with a run of
+    /// appends to make should materialize the elements once itself and call that directly.
     pub(crate) fn append_constant(
         &mut self,
         value: ListScalar,
@@ -139,36 +183,26 @@ impl FixedSizeListBuilder {
             return Ok(());
         };
 
+        let list_size = self.list_size() as usize;
         vortex_ensure!(
-            elements.len() == self.list_size() as usize,
+            elements.len() == list_size,
             "Scalar list length {} does not match fixed list size {}",
             elements.len(),
-            self.list_size()
+            list_size
         );
 
-        let tiled_len = n * elements.len();
-        if let Ok(uniform) = elements.iter().all_equal_value() {
-            let tile = ConstantArray::new(uniform.clone(), tiled_len).into_array();
-            self.elements_builder.append_array(&tile, ctx)?;
-            self.nulls.append_n_non_nulls(n);
-            return Ok(());
-        }
-
-        let tile = {
-            let mut tile_builder = builder_with_capacity(self.element_dtype(), elements.len());
-            for element in &elements {
-                tile_builder.append_scalar(element)?;
+        let tile = match elements.iter().all_equal_value() {
+            Ok(uniform) => ConstantArray::new(uniform.clone(), list_size).into_array(),
+            Err(_) => {
+                let mut tile_builder = builder_with_capacity(self.element_dtype(), list_size);
+                for element in &elements {
+                    tile_builder.append_scalar(element)?;
+                }
+                tile_builder.finish()
             }
-            tile_builder.finish()
         };
 
-        self.elements_builder.reserve_exact(tiled_len);
-        for _ in 0..n {
-            self.elements_builder.append_array_values(&tile, ctx)?;
-        }
-        self.nulls.append_n_non_nulls(n);
-
-        Ok(())
+        self.append_array_as_repeated_list(&tile, n, ctx)
     }
 
     /// Appends the values of a canonical [`FixedSizeListArray`] to the builder, recursing into the