Append constant runs and sparse patches without slicing or canonicalizing [builders-child-stack] - #9131
Closed
robert3005 wants to merge 5 commits into
Closed
Conversation
Hooray! CodSpeed harness just leveled up!The base and head of this comparison were measured with different runner settings, so their benchmark values are not directly comparable. What changed between base and head:
Re-run the base with the same settings to get a valid performance comparison. Comparing 
Footnotes |
robert3005
force-pushed
the
claude/constant-fast-paths-9ze0t6
branch
2 times, most recently
from
August 1, 2026 10:19
9918f2f to
b53d487
Compare
robert3005
force-pushed
the
claude/builders-lazy-validity-9ze0t6
branch
from
August 5, 2026 12:51
5e51643 to
421b6d4
Compare
robert3005
force-pushed
the
claude/constant-fast-paths-9ze0t6
branch
from
August 5, 2026 12:53
b53d487 to
fdd975e
Compare
`Constant::append_to_builder` canonicalized every dtype it had no fast path for. For a list that builds a whole `ListViewArray` only for `append_listview_array` to rebuild it and cast its offsets and sizes back to the builder's types - fixed cost per appended run, paid by every caller that covers a run of rows with one repeated list. A list builder can record the run from the scalar alone. `ListViewBuilder` stores the elements once and points every view at them; `ListBuilder` repeats them, because its offsets can only describe contiguous, in-order lists. Dispatch to both through `match_each_list_builder!` and leave every other builder on the canonical path. Signed-off-by: Robert Kruszewski <robert@spiraldb.com>
…_array Trimming through `rebuild(ListViewRebuildMode::TrimElements)` subtracts the window start from every offset with a compute kernel, only for the rebase in `append_listview_array` to add this builder's elements base straight back on: two passes, one of them through the compute stack, for one addition per offset. Casting the sizes to the builder's type is another kernel for what is a copy. Compute the referenced window here instead. An exact source covers it back to back, so its first and last view bound it, and only some other layout has to be searched for. Slice the elements to that window and rebase in the single pass that was already walking the offsets. The sizes go straight into the builder's `uninit_range` - as a `copy_from_slice` when they already have its type, and a typed conversion loop when they do not. Signed-off-by: Robert Kruszewski <robert@spiraldb.com>
Slicing is not the constant-time operation its doc comment claims: `ArrayRef::slice` ends in `.optimize()`, so every slice pays a full optimizer pass. Slicing a `ListView` is four of them, because slicing the array slices its offsets, its sizes and its elements in turn. A profile of `canonicalize_sparse_list[(512, 7, 4)]` put 42% of the benchmark in `optimizer::try_optimize` under `ArrayRef::slice`. So take each patch's elements instead of slicing runs of patches out of the patch array: `list_elements_at` slices `elements` alone, which is one optimizer pass over a primitive array rather than four over a list view. The same reasoning applies to the fixed-size-list path, which sliced its elements and its validity per run. Gaps keep their bulk append, but reach it without a `ConstantArray`: the fill's elements are materialized once, up front, and every gap points its rows at that one array through `append_array_as_repeated_list`. The fill's elements are now stored once for the whole result rather than once per gap, and a gap costs nothing per row it covers. `canonicalize_sparse_list` medians, against develop: (512, 7, 4) 24.1 us -> 22.5 us (1024, 17, 8) 33.4 us -> 18.7 us (4096, 8, 4) 173.3 us -> 119.6 us (4096, 64, 4) 99.9 us -> 17.3 us (8192, 1024, 4) 180.6 us -> 5.6 us Signed-off-by: Robert Kruszewski <robert@spiraldb.com>
… directly `Constant::append_to_builder` had fast paths for the flat dtypes and fell back to canonicalizing everything else, which builds the whole run as an array only to copy it into the builder. The nested dtypes with a builder can all skip that, and two of them can skip the values entirely: - a constant struct is a constant array per field, so each field's builder takes one as a chunk and the fields stay constant-encoded; - an extension array is its storage wearing a dtype, so a constant one is a constant storage array; - a fixed-size list cannot share one copy of its elements between rows the way a list view can, since its elements sit back to back. A null value still needs only placeholders, and a value whose elements are all the same scalar still tiles to a constant array. Otherwise the tile is copied in per row - one copy of each element, where canonicalizing first made two. That last case is why `ChildBuilder` grows `append_array_values`: appending the same tiny array over and over is the one case where a chunk per append costs more than copying the values, and only the caller can see it. `canonicalize_sparse_fixed_size_list` medians: (512, 7, 4) 79.0 us -> 55.0 us (1024, 17, 8) 139.5 us -> 63.9 us (8192, 1024, 4) 375.9 us -> 142.3 us `Union` is the only dtype left on the fallback, and it has no builder yet. Signed-off-by: Robert Kruszewski <robert@spiraldb.com>
`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`. A fixed-size list holds its elements back to back, so it cannot point a run of rows at one shared range the way a list view can - but it need not copy the tile per row either. The run goes in as a `ChunkedArray` of `n` clones of the tile, which costs `n` reference bumps and no element data at all. The child keeps that whole run as a single chunk: unpacking it would spill a chunk per row into the child's chunk list, which every later append and the final `finish` walk, and that list is what makes the difference - unpacking measured 36.0 us against 26.0 us on `(512, 7, 4)`. Elements that are all the same scalar do better still, collapsing to a single constant chunk however many rows they cover. `canonicalize_sparse_fixed_size_list` medians, against develop: (512, 7, 4) 21.1 us -> 26.0 us (was 79.0 us) (1024, 17, 8) 29.1 us -> 22.7 us (was 139.5 us) (8192, 1024, 4) 150.1 us -> 34.5 us (was 375.9 us) Signed-off-by: Robert Kruszewski <robert@spiraldb.com>
robert3005
force-pushed
the
claude/builders-lazy-validity-9ze0t6
branch
from
August 7, 2026 10:40
421b6d4 to
b24a656
Compare
robert3005
force-pushed
the
claude/constant-fast-paths-9ze0t6
branch
from
August 7, 2026 10:40
fdd975e to
e250d3a
Compare
Contributor
Author
This was referenced Aug 7, 2026
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is an alternative way of building sparse arrays. Since everything is lazy we can construct fills and other values as arrays and tile them in the builder.
The question is do we want to unwrap the tiled arrays when appending. They produce one extra level of nesting but remove a overhead from the builder which we don't know if we will need to do or not, not quite sure which way is ultimately better.