Add BooleanBuffer::try_into_builder and BooleanBuffer::into_builder_or_clone#10397
Add BooleanBuffer::try_into_builder and BooleanBuffer::into_builder_or_clone#10397alamb wants to merge 1 commit into
BooleanBuffer::try_into_builder and BooleanBuffer::into_builder_or_clone#10397Conversation
6cb794f to
8d6bd96
Compare
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
8d6bd96 to
772a759
Compare
|
Interestingly, I couldn't find any benchmark improvements |
I ran additional The current implementation was approximately 42–45% slower than I then tested a large-buffer fast path that reuses the existing allocation only when:
This avoids allocating a new buffer and copying a large retained prefix. It produced significant improvements:
However, adding this dispatch to The large-buffer reuse optimization is promising, but I suggest investigating it separately so the existing small-input |
Which issue does this PR close?
into_buildermethods for Arrays #6430BooleanArray::take_n_true#10399I got inspired to propose this API this when reviewing this from @hhhizzz
Rationale for this change
Code that owns a
BooleanBufferand wants to mutate it currently has to copy the bits into a newBooleanBufferBuilder, even when the underlying allocation is not shared and could be reused directly.There are times downstream in DataFusion (like combining filter masks) when it would be nice to reuse an allocation when we have an owned BooleanBuffer. The current APIs typically require copying into a new buffer
What changes are included in this PR?
Add two new APIs on
BooleanBuffer:try_into_builder(self)into_builder_or_clone(self)Updates existing users to try and avoid reallocations
BooleanArray::take_n_trueto use the new APIFilterMaskAccumulatorNaming / follow up
I would like
try_into_builderto become the standard name for this pattern:try_prefix plusResultfollows Rust conventions,PrimitiveArray::into_builderis fallible despite its name.If this API looks good, a follow on PR will propose renaming
PrimitiveArray::into_buildertotry_into_builder(with a deprecated alias) and adding the correspondinginto_builder_or_clone.Are these changes tested?
Yes: new unit tests
Are there any user-facing changes?
Two new public methods on
BooleanBuffer. No changes to existing APIs.