Skip to content

Add BooleanBuffer::try_into_builder and BooleanBuffer::into_builder_or_clone#10397

Draft
alamb wants to merge 1 commit into
apache:mainfrom
alamb:alamb/boolean-buffer-try-into-builder
Draft

Add BooleanBuffer::try_into_builder and BooleanBuffer::into_builder_or_clone#10397
alamb wants to merge 1 commit into
apache:mainfrom
alamb:alamb/boolean-buffer-try-into-builder

Conversation

@alamb

@alamb alamb commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

I got inspired to propose this API this when reviewing this from @hhhizzz

Rationale for this change

Code that owns a BooleanBuffer and wants to mutate it currently has to copy the bits into a new BooleanBufferBuilder, 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:

  1. try_into_builder(self)
  2. into_builder_or_clone(self)

Updates existing users to try and avoid reallocations

  • BooleanArray::take_n_true to use the new API
  • FilterMaskAccumulator

Naming / follow up

I would like try_into_builder to become the standard name for this pattern:

  • the try_ prefix plus Result follows Rust conventions,
  • the existing PrimitiveArray::into_builder is fallible despite its name.

If this API looks good, a follow on PR will propose renaming PrimitiveArray::into_builder to try_into_builder (with a deprecated alias) and adding the corresponding into_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.

@github-actions github-actions Bot added the arrow Changes to the arrow crate label Jul 20, 2026
@alamb
alamb force-pushed the alamb/boolean-buffer-try-into-builder branch from 6cb794f to 8d6bd96 Compare July 20, 2026 20:56
@github-actions github-actions Bot added the parquet Changes to the parquet crate label Jul 20, 2026
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@alamb
alamb force-pushed the alamb/boolean-buffer-try-into-builder branch from 8d6bd96 to 772a759 Compare July 20, 2026 21:03
@alamb

alamb commented Jul 20, 2026

Copy link
Copy Markdown
Contributor Author

Interestingly, I couldn't find any benchmark improvements

@hhhizzz

hhhizzz commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Interestingly, I couldn't find any benchmark improvements

I ran additional take_n_true benchmarks on my server using five alternating rounds pinned to a single CPU.

The current implementation was approximately 42–45% slower than main on the existing 8K-bit benchmarks.

I then tested a large-buffer fast path that reuses the existing allocation only when:

  • the retained prefix is at least 64 KiB;
  • the BooleanBuffer is unsliced; and
  • the allocation is uniquely owned.

This avoids allocating a new buffer and copying a large retained prefix. It produced significant improvements:

  • 1M bits, 50% cutoff: 24.15% faster
  • 1M bits, 99% cutoff: 25.50% faster
  • 16M bits, 50% cutoff: 27.30% faster
  • 16M bits, 99% cutoff: 30.26% faster

However, adding this dispatch to take_n_true still caused a repeatable 2.7–4.7% regression on the existing 8K workload due to changes in generated code, even though that workload did not enter the reuse path.

The large-buffer reuse optimization is promising, but I suggest investigating it separately so the existing small-input take_n_true path remains unchanged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

arrow Changes to the arrow crate parquet Changes to the parquet crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants