feat(array): builders skip child canonicalization and append nested values in bulk [builders-child-stack] - #9064
Merged
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↓)
|
`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. |
onursatici
reviewed
Aug 19, 2026
robert3005
added
changelog/break
onursatici
approved these changes
Aug 20, 2026
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
4 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):
With this change all builders lose set_validity method as it's no longer used in the codebase and there are always better ways of expressing the desired operation