feat(array): let callers choose a nested builder's chunk threshold [builders-child-stack]#8965
Conversation
`ChildBuilder` copies appended arrays shorter than 64 values instead of giving them a chunk, which is right for the nested builders that append the elements of a single list, and wrong for a caller whose appended arrays are themselves chunks worth preserving. Add `ArrayBuilder::set_min_chunk_len`, a no-op for builders without array children, so such a caller can lower the threshold — to zero to keep every chunk boundary. The threshold is read when an array is appended and applies transitively to the children of a builder's children. Signed-off-by: Claude <noreply@anthropic.com>
2e987a3 to
b4cb8fb
Compare
cc36904 to
6f3ef13
Compare
Merging this PR will regress 1 benchmark
Warning Please fix the performance issues or acknowledge them on CodSpeed. Performance Changes
Tip Investigate this regression by commenting Comparing Footnotes
|
Rationale for this change
Stacked on #8964 — review that one first.
ChildBuildercopies appended arrays shorter than 64 values instead of giving each one a chunk. That default is right for the callers it was written for: nested builders routinely append the elements of a single list, and a chunk per list would produce aChunkedArraywith more chunks than values.It is wrong for a caller whose appended arrays are already chunks whose identity is the point. The motivating one is
ChunkedArray's canonicalization, which today hand-rollsswizzle_list_chunksand friends precisely to reuse each chunk's children without copying. A filtered scan can easily produce chunks of well under 64 rows, and routing that through a builder with the default threshold would copy every child.What changes are included in this PR?
ArrayBuilder::set_min_chunk_len, defaulting to a no-op so builders without array children ignore it.StructBuilder,ListBuilder,ListViewBuilder,FixedSizeListBuilderandExtensionBuilderforward it to theirChildBuilders, which hold the threshold as a field instead of reading the const.Two semantics worth stating, both documented on the trait method:
ChildBuilder's scalar builder is itself a nested builder with children of its own, so it gets the setting too.Three tests: a threshold of zero keeps every chunk boundary at the
ChildBuilderlevel and through the publicbuilder_with_capacity, and the transitive case is checked with a list-typed child short enough to be materialized whose elements would otherwise have earned a chunk.What APIs are changed? Are there any user-facing changes?
One new
ArrayBuildertrait method with a default implementation, so external implementors are unaffected. No behaviour change for anyone who does not call it:DEFAULT_MIN_CHUNK_LENis the previous constant.Checks
cargo test -p vortex-array— 3167 lib testscargo +nightly fmt --all,cargo clippy -p vortex-array --all-targets --all-featuresGenerated by Claude Code
Stacked PR Chain: builders-child-stack
Generated by Claude Code