feat(array): builders skip child canonicalization and append nested values in bulk [builders-child-stack] - #9064
Open
robert3005 wants to merge 13 commits into
Open
feat(array): builders skip child canonicalization and append nested values in bulk [builders-child-stack]#9064robert3005 wants to merge 13 commits into
robert3005 wants to merge 13 commits into
Conversation
This was referenced Jul 29, 2026
Closed
Merging this PR will regress 6 benchmarks
Warning Please fix the performance issues or acknowledge them on CodSpeed. Performance ChangesTip Investigate this regression by commenting Comparing 
Footnotes
|
robert3005
force-pushed
the
claude/builders-bulk-loop-callers-9ze0t6
branch
from
July 29, 2026 14:19
b622d0f to
3304731
Compare
robert3005
force-pushed
the
claude/builders-bulk-loop-callers-9ze0t6
branch
2 times, most recently
from
July 31, 2026 15:19
118384a to
022be8b
Compare
Base automatically changed from
claude/listview-builder-keep-overlaps-9ze0t6
to
develop
July 31, 2026 15:28
robert3005
force-pushed
the
claude/builders-bulk-loop-callers-9ze0t6
branch
from
July 31, 2026 15:29
022be8b to
10d22d5
Compare
Contributor
Author
robert3005
force-pushed
the
claude/builders-bulk-loop-callers-9ze0t6
branch
3 times, most recently
from
July 31, 2026 19:19
24b26b3 to
ade8dbe
Compare
Contributor
Author
Contributor
Author
|
Ideally we can review the whole stack. The change merged partially causes some regressions |
robert3005
force-pushed
the
claude/builders-bulk-loop-callers-9ze0t6
branch
2 times, most recently
from
August 7, 2026 10:40
043d08a to
e570927
Compare
robert3005
changed the title
robert3005
force-pushed
the
claude/builders-bulk-loop-callers-9ze0t6
branch
from
August 7, 2026 13:10
57d0640 to
626df0f
Compare
joseph-isaacs
added
the
action/bench-all
github-actions
Bot
removed
the
action/bench-all
Contributor
Polar Signals Profiling ResultsLatest Run
Powered by Polar Signals Cloud |
Contributor
Benchmarks: String Encoding 📖vortex / vortex-file-compressed / ms (0.997x ➖, 0↑ 0↓)
vortex / vortex-file-compressed / % (1.000x ➖, 0↑ 0↓)
|
Contributor
Benchmarks: Random Access 📖Verdict: No clear signal (low confidence) How to read Verdict and Engines
random-access / vortex-file-compressed / ns (0.999x ➖, 0↑ 0↓)
random-access / parquet / ns (1.002x ➖, 0↑ 0↓)
random-access / lance / ns (1.003x ➖, 0↑ 0↓)
|
… a time Four callers walked a builder one list at a time where a whole run of them was available up front. `Sparse` canonicalization filled the gaps between patches by appending the fill value once per row, and appended the patches themselves one list at a time. A gap is a run of one value, so it goes in as a single `ConstantArray`: canonicalizing a constant list array points every view at one copy of the value, so a gap now costs the fill value's elements once however many rows it covers. Appending a 10,000-row gap of a three-element fill produced 30,000 elements; it now produces 3. Patches landing on consecutive rows go in as one slice of the patch array, so the builder sees an append per gap rather than one per patch. The patch values are flattened once up front instead. That is what lets a run be sliced out of them, and it also means a null patch carries a zero-size view rather than the elements the old code skipped, so the appended run holds exactly the elements the patches reference. `ListBuilder::append_listview_array` sliced the elements array and appended the slice once per list, which is what its `ListViewBuilder` twin stopped doing. `ListArray` offsets can only describe contiguous, in-order lists, so flatten the incoming views to that layout - a no-op when they are laid out that way already - and then append the referenced elements in one go, walking only the metadata to rebase the offsets. Signed-off-by: Claude <noreply@anthropic.com> Signed-off-by: Robert Kruszewski <robert@spiraldb.com> Signed-off-by: Robert Kruszewski <github@robertk.io> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Nested builders used to push every appended child array through `append_to_builder`, which decoded it into the child's canonical builder. That work is wasted: `Canonical` only promises a canonical *top level*, so struct fields, list elements and extension storage are free to stay compressed. Introduce `ChildBuilder`, which accumulates a child as a `Vec<ArrayRef>` of chunks plus a scalar builder for the values that cannot come from an array, and stitches them into a `ChunkedArray` on `finish` when more than one chunk accumulated. `StructBuilder`, `ListBuilder`, `ListViewBuilder`, `FixedSizeListBuilder` and `ExtensionBuilder` now hold their children this way. However short the appended array, it becomes a chunk. Deciding on the caller's behalf that its values are cheaper copied than referenced would be guessing at a boundary only the caller can see, and a caller that wants them copied has `append_scalar`. A child is therefore chunked on exactly the boundaries it was appended on. Signed-off-by: Claude <noreply@anthropic.com> Signed-off-by: Robert Kruszewski <robert@spiraldb.com> Signed-off-by: Robert Kruszewski <github@robertk.io> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A nested builder learns about validity from two sources: one row at a time as scalars are appended, and a whole array's worth at a time as arrays are. Only the first needs a null buffer, but `LazyBitBufferBuilder` treated both the same, so every appended array had its validity executed into a `Mask` and its bits copied. `ValidityBuilder` keeps a whole array's validity as a run and concatenates the runs at the end, the way `Validity::concat` already does for `StructArray::try_concat`. `AllValid` and `AllInvalid` runs cost nothing, array-backed runs are bool arrays that are already built, and a builder that only ever saw uniform validity still answers from its nullability rather than producing a bool array. However few values a run covers, it is kept as it arrived, so a builder's validity is split on exactly the boundaries its children are. `StructBuilder`, `ListBuilder`, `ListViewBuilder` and `FixedSizeListBuilder` use it; the leaf builders keep `LazyBitBufferBuilder`. Signed-off-by: Claude <noreply@anthropic.com> Signed-off-by: Robert Kruszewski <robert@spiraldb.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`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-bulk-loop-callers-9ze0t6
branch
from
August 7, 2026 14:36
626df0f to
64c3464
Compare
Contributor
Author
|
Where sparse canonicalization flips to a regression The crossover is governed by patch density (i.e. fill-run length), not array size. All medians below are at len = 65 536, list_size = 4; stride means one patch every stride rows, so fill runs are stride − 1 rows long. |
Contributor
Benchmarks: FineWeb NVMe 📖Verdict: No clear signal (low confidence) How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (1.021x ➖, 0↑ 2↓)
datafusion / vortex-compact / ns (0.992x ➖, 0↑ 0↓)
datafusion / parquet / ns (0.986x ➖, 1↑ 0↓)
duckdb / vortex-file-compressed / ns (1.036x ➖, 1↑ 2↓)
duckdb / vortex-compact / ns (0.982x ➖, 0↑ 0↓)
duckdb / parquet / ns (1.019x ➖, 0↑ 0↓)
File Size Changes (2 files changed, +0.0% overall, 1↑ 1↓)
Totals:
|
Contributor
Benchmarks: TPC-DS SF=1 on NVME 📖Verdict: No clear signal (low confidence) How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (0.998x ➖, 1↑ 2↓)
datafusion / vortex-compact / ns (0.996x ➖, 0↑ 1↓)
datafusion / parquet / ns (1.002x ➖, 1↑ 2↓)
duckdb / vortex-file-compressed / ns (1.008x ➖, 3↑ 6↓)
duckdb / vortex-compact / ns (0.999x ➖, 1↑ 5↓)
duckdb / parquet / ns (0.994x ➖, 4↑ 2↓)
No file size changes detected. |
Contributor
Benchmarks: Compression 📖vortex / vortex-file-compressed / ns (1.056x ➖, 1↑ 8↓)
vortex / vortex-file-compressed / bytes (1.001x ➖, 0↑ 0↓)
vortex / vortex-file-compressed / ratio (1.036x ➖, 1↑ 8↓)
vortex / parquet / ns (1.003x ➖, 0↑ 0↓)
vortex / parquet / bytes (1.000x ➖, 0↑ 0↓)
|
Contributor
Benchmarks: FineWeb S3 📖Verdict: No clear signal (environment too noisy confidence) How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (1.111x ➖, 0↑ 0↓)
datafusion / vortex-compact / ns (1.071x ➖, 0↑ 1↓)
datafusion / parquet / ns (1.016x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed / ns (1.050x ➖, 0↑ 0↓)
duckdb / vortex-compact / ns (0.973x ➖, 0↑ 0↓)
duckdb / parquet / ns (1.522x ❌, 0↑ 2↓)
|
Contributor
Benchmarks: TPC-H SF=10 on NVME 📖Verdict: No clear signal (low confidence) How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (0.996x ➖, 0↑ 0↓)
datafusion / vortex-compact / ns (0.996x ➖, 0↑ 0↓)
datafusion / parquet / ns (0.987x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed / ns (1.005x ➖, 0↑ 0↓)
duckdb / vortex-compact / ns (1.007x ➖, 0↑ 1↓)
duckdb / parquet / ns (1.018x ➖, 0↑ 1↓)
No file size changes detected. |
Contributor
Benchmarks: PolarSignals Profiling 📖Vortex (geomean): 0.980x ➖ datafusion / vortex-file-compressed / ns (0.980x ➖, 1↑ 0↓)
No file size changes detected. |
Contributor
Benchmarks: Clickbench Sorted on NVME 📖Verdict: No clear signal (low confidence) How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (0.987x ➖, 1↑ 0↓)
datafusion / vortex-compact / ns (1.016x ➖, 0↑ 1↓)
datafusion / parquet / ns (0.955x ➖, 3↑ 0↓)
duckdb / vortex-file-compressed / ns (1.022x ➖, 0↑ 1↓)
duckdb / vortex-compact / ns (0.996x ➖, 0↑ 0↓)
duckdb / parquet / ns (0.989x ➖, 0↑ 0↓)
File Size Changes (200 files changed, +0.0% overall, 102↑ 98↓)
Totals:
|
Contributor
Benchmarks: Clickbench on NVME 📖Verdict: No clear signal (low confidence) How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (0.974x ➖, 3↑ 1↓)
datafusion / vortex-compact / ns (0.998x ➖, 0↑ 0↓)
datafusion / parquet / ns (0.998x ➖, 0↑ 1↓)
duckdb / vortex-file-compressed / ns (1.000x ➖, 3↑ 3↓)
duckdb / vortex-compact / ns (1.022x ➖, 1↑ 4↓)
duckdb / parquet / ns (0.997x ➖, 0↑ 0↓)
File Size Changes (56 files changed, -0.0% overall, 18↑ 38↓)
Totals:
|
Contributor
Benchmarks: Appian on NVME 📖Verdict: No clear signal (low confidence) How to read Verdict and Engines
datafusion / vortex-compact / ns (1.004x ➖, 0↑ 0↓)
datafusion / parquet / ns (0.991x ➖, 0↑ 0↓)
duckdb / vortex-compact / ns (0.990x ➖, 0↑ 0↓)
duckdb / parquet / ns (1.002x ➖, 0↑ 0↓)
File Size Changes (12 files changed, -63.8% overall, 0↑ 12↓)
Totals:
|
Contributor
Benchmarks: Statistical and Population Genetics 📖Verdict: No clear signal (low confidence) How to read Verdict and Engines
duckdb / vortex-file-compressed / ns (1.008x ➖, 1↑ 1↓)
duckdb / vortex-compact / ns (1.083x ➖, 0↑ 4↓)
duckdb / parquet / ns (0.995x ➖, 0↑ 0↓)
File Size Changes (2 files changed, -0.0% overall, 0↑ 2↓)
Totals:
|
Contributor
Benchmarks: TPC-H SF=1 on S3 📖Verdict: No clear signal (low confidence) How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (0.870x ➖, 2↑ 0↓)
datafusion / vortex-compact / ns (0.925x ➖, 0↑ 0↓)
datafusion / parquet / ns (0.964x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed / ns (1.033x ➖, 0↑ 2↓)
duckdb / vortex-compact / ns (0.975x ➖, 1↑ 0↓)
duckdb / parquet / ns (0.970x ➖, 0↑ 0↓)
|
Contributor
Benchmarks: TPC-H SF=10 on S3 📖Verdict: No clear signal (environment too noisy confidence) How to read Verdict and Engines
datafusion / vortex-compact / ns (0.891x ➖, 1↑ 0↓)
datafusion / parquet / ns (0.828x ➖, 3↑ 0↓)
duckdb / vortex-compact / ns (1.030x ➖, 0↑ 0↓)
duckdb / parquet / ns (0.965x ➖, 0↑ 0↓)
|
Contributor
Benchmarks: TPC-H SF=1 on NVME 📖Verdict: No clear signal (low confidence) How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (1.005x ➖, 0↑ 0↓)
datafusion / vortex-compact / ns (1.001x ➖, 0↑ 0↓)
datafusion / parquet / ns (0.997x ➖, 0↑ 1↓)
duckdb / vortex-file-compressed / ns (0.994x ➖, 0↑ 1↓)
duckdb / vortex-compact / ns (1.006x ➖, 0↑ 1↓)
duckdb / parquet / ns (1.004x ➖, 0↑ 0↓)
No file size changes detected. |
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
3 participants
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.
Collapses the first four PRs of the builders-child-stack into one (#8964, #8966, and #9131 were closed and folded in here):