lichuang opened a new pull request, #12228:
URL: https://github.com/apache/arrow-rs/pull/12228
# Which issue does this PR close?
- Part of #11717
# Rationale for this change
Builder `finish` currently routes through `ArrayData` even for simple,
statically typed arrays:
```text
values + nulls
→ ArrayDataBuilder
→ ArrayData { buffers: Vec<Buffer> }
→ typed array
```
`ArrayData` needs a `Vec<Buffer>` even when the array has a single values
buffer, so every `finish` pays at least one extra heap allocation, plus
`ArrayData` construction, buffer-count checks, and `Vec` teardown. For callers
that finish many small arrays, such as DataFusion's sort-merge join finishing
two `UInt64Builder`s per chunk, this fixed overhead dominates.
# What changes are included in this PR?
Build the typed arrays directly in `finish` and `finish_cloned` for the four
builders named in the issue:
- `PrimitiveBuilder` now constructs `PrimitiveArray::new(values,
nulls).with_data_type(...)`, preserving custom timestamp/decimal data types.
- `BooleanBuilder` now constructs `BooleanArray::new(values, nulls)`
directly from its `BooleanBuffer`.
- `GenericByteBuilder` now constructs `OffsetBuffer` and `GenericByteArray`
directly, while preserving the offset reset needed for builder reuse.
- `FixedSizeBinaryBuilder` now constructs
`FixedSizeBinaryArray::new_unchecked` directly from its values, nulls, value
length, and length.
The byte and fixed-size-binary paths use unchecked constructors because
their builder invariants already guarantee valid offsets, value lengths, and
null lengths. The existing `force_validate` feature still routes these through
validation.
This PR intentionally does not change `NullBuilder`, `MapBuilder`,
`UnionBuilder`, dictionary builders, or `finish_preserve_values`; those have
different container/dynamic-type requirements and can be handled separately.
# Are these changes tested?
The existing builder test suites cover `finish`, `finish_cloned`, builder
reuse, custom primitive data types, empty arrays, null handling, and
zero-length fixed-size binary values. The full `arrow-array` test suite passes,
including doctests, and the `force_validate` configuration passes as well.
Clippy and formatting checks pass.
A local run of the microbenchmark from the issue (release build, same
benchmark code, best of seven passes over chunked arrays) showed:
| Values per array | Before | After |
|---:|---:|---:|
| 1 | 91.3 ns | 50.4 ns |
| 8 | 92.6 ns | 52.1 ns |
| 64 | 111.1 ns | 68.8 ns |
| 512 | 151.3 ns | 104.4 ns |
The benchmark is not committed; these numbers are provided as evidence for
the allocation removal. Large arrays are dominated by value filling, so the
gain is expected to be concentrated in small arrays.
# Are there any significant user-facing changes?
No public API changes. The output arrays keep the same data types, values,
nulls, and builder-reset behavior.
# AI usage disclosure
This change was prepared with AI assistance. I reviewed the implementation
and verified the existing tests, Clippy, formatting, and benchmark results
locally.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
