perf(parquet): avoid redundant copies in mask-backed intersection/union - #10446
Open
haohuaijin wants to merge 10 commits into
Open
perf(parquet): avoid redundant copies in mask-backed intersection/union#10446haohuaijin wants to merge 10 commits into
haohuaijin wants to merge 10 commits into
Conversation
github-actions
Bot
added
parquet
haohuaijin
force-pushed
the
perf-uneven-mask-intersect-union
branch
from
July 27, 2026 04:56
d332086 to
236925a
Compare
…ual lengths
When two mask-backed `RowSelection`s have different lengths, the bitwise
`intersection`/`union` path allocated twice: once for the bitwise result over
the common prefix, and once more for a `BooleanBufferBuilder` that appended
the prefix and then the longer side's tail, copying both.
Copy the longer mask once into a `MutableBuffer` instead and apply `&=`/`|=`
in place over the common prefix with `bit_util::apply_bitwise_binary_op`. The
tail is already in the right place, so it needs no copy at all.
Neither the mask offsets nor the prefix length are assumed to be byte aligned.
Masks reaching this path may carry a non-zero bit offset from
`BooleanBuffer::slice`, and the prefix may end mid byte. The copy keeps the
longer mask's sub-byte offset rather than re-aligning it, so it stays a plain
byte copy, and that offset is carried over to the returned buffer.
Criterion, mask-backed `RowSelection::intersection`/`union`, ~1/3 density:
before after change
intersect 3M / 2M 16.60 us 12.96 us -21.9%
union 3M / 2M 16.57 us 13.12 us -20.8%
intersect 3M / 3M-1 20.22 us 16.51 us -18.3%
union 3M / 3M-1 20.19 us 16.66 us -17.5%
intersect 100K / 1K 509 ns 278 ns -45.4%
union 100K / 1K 525 ns 270 ns -48.6%
The gain grows with the tail, which the old code copied through the builder
for no reason.
Tested by an exhaustive sweep over unaligned offset and length combinations
and a randomized fuzz test, both checked against a bit-by-bit reference.
Closes apache#10425
haohuaijin
force-pushed
the
perf-uneven-mask-intersect-union
branch
from
August 4, 2026 02:22
236925a to
ab44ada
Compare
The existing `intersection`/`union` benchmarks build their operands with `from_filters`, which is selector-backed, so they take the `RowSelector` merge path and never reach the bitwise one. Add `mask_intersection`/`mask_union` alongside them, varying two dimensions that drive the bitwise path: the ratio between operand lengths, since the longer side's tail passes through unchanged, and the bit offset both operands carry, since masks come from `BooleanBuffer::slice` and an offset that is not byte aligned puts the underlying helpers on their shifting path.
haohuaijin
marked this pull request as ready for review
August 4, 2026 13:13
Contributor
Author
…ebra
`BitAnd`/`BitOr` on `&BooleanBuffer` normalise their result to a zero bit
offset. When the operands are not byte aligned, `buffer_bin_and`'s closing
`sliced()` is a second allocation plus a shifting copy of the whole mask, and
that copy dominates the operation.
Build the result with `BooleanBuffer::from_bitwise_binary_op` instead and keep
whatever offset it produces, which is what the uneven-length path already does.
Masks reaching here come from `BooleanBuffer::slice`, so a non-zero offset is
the ordinary case, and consumers are offset-aware.
3M rows, ~1/3 density, `codegen-units=1`:
before after change
intersection equal/aligned 11.36 us 11.25 us -1.0%
intersection equal/unaligned 98.45 us 42.31 us -57.0%
union equal/aligned 11.23 us 10.56 us -6.0%
union equal/unaligned 97.86 us 42.07 us -57.0%
Aligned operands never paid for the normalisation, so they are unchanged. The
twelve uneven-length cases move within noise.
The equal-length path was previously covered only through `l & r`, so extend
the sweep with six equal-length pairs across the same offset grid and bias the
fuzz test towards equal lengths.
haohuaijin
changed the title
hhhizzz
reviewed
Aug 5, 2026
hhhizzz
left a comment
hhhizzz
left a comment
Contributor
There was a problem hiding this comment.
Thank you! Added some small suggestion.
Contributor
Author
hhhizzz
reviewed
Aug 6, 2026
Contributor
Thanks for providing the full The remaining result I find surprising is
With different mod-64 alignments, So the improvement looks real, but it does not appear to come from the redundant-copy removal described in the PR. Could you clarify whether this is a cross-crate inlining/codegen effect? One useful way to isolate it would be to benchmark |
Contributor
Author
Thanks for the suggestion — I ran the isolation, then took it further with LTO. All on
At |
Contributor
|
run benchmark mask_intersection |
Contributor
|
merging up from main to try and get a clean CI run |
|
🤖 Arrow criterion benchmark running (GKE) | trigger CPU Details (lscpu)Comparing perf-uneven-mask-intersect-union (004bf28) to 3df22cd (merge-base) diff Run configurationrun benchmark mask_intersectionBENCH_COMMAND=cargo bench --features=arrow,async,test_common,experimental,object_store --bench mask_intersection File an issue against this benchmark runner |
|
Benchmark for this request failed. Run configurationrun benchmark mask_intersectionLast 20 lines of output: Click to expandFile an issue against this benchmark runner |
Contributor
|
run benchmark row_selector |
|
🤖 Arrow criterion benchmark running (GKE) | trigger CPU Details (lscpu)Comparing perf-uneven-mask-intersect-union (004bf28) to 3df22cd (merge-base) diff Run configurationrun benchmark row_selectorBENCH_COMMAND=cargo bench --features=arrow,async,test_common,experimental,object_store --bench row_selector File an issue against this benchmark runner |
|
🤖 Arrow criterion benchmark completed (GKE) | trigger Instance: Comparing perf-uneven-mask-intersect-union (004bf28) to 3df22cd (merge-base) diff Run configurationrun benchmark row_selectorCPU Details (lscpu)Details
Resource Usagebase (merge-base)
branch
File an issue against this benchmark runner |
Contributor
Author
alamb
pushed a commit
that referenced
this pull request
Aug 7, 2026
…10574) # Which issue does this PR close? Split out of #10446 so that CI can compare both sides — the benchmark is new there, so the merge-base has nothing to compare against and the `main` column comes out empty. # Rationale for this change `row_selector.rs` already benchmarks `intersection`/`union`, but it builds the operands with `from_filters`, which is selector-backed. Those take the `RowSelector` merge path and never reach the bitwise one used when both operands are mask-backed. # What changes are included in this PR? Adds `mask_intersection`/`mask_union`, varying the two dimensions that drive the bitwise path: - the ratio between operand lengths, since unequal lengths pass the longer side's tail through unchanged - the bit offsets the operands carry, since masks come from `BooleanBuffer::slice` and whether the two share a sub-64-bit alignment decides which path the underlying helpers take Benchmark only, no library changes. # Are these changes tested? N/A — this is a benchmark. It builds and runs on `main` as-is. # Are there any user-facing changes? No.
Contributor
Contributor
|
run benchmark row_selector |
|
🤖 Arrow criterion benchmark running (GKE) | trigger CPU Details (lscpu)Comparing perf-uneven-mask-intersect-union (635c740) to 3e6bd62 (merge-base) diff Run configurationrun benchmark row_selectorBENCH_COMMAND=cargo bench --features=arrow,async,test_common,experimental,object_store --bench row_selector File an issue against this benchmark runner |
|
🤖 Arrow criterion benchmark completed (GKE) | trigger Instance: Comparing perf-uneven-mask-intersect-union (635c740) to 3e6bd62 (merge-base) diff Run configurationrun benchmark row_selectorCPU Details (lscpu)Details
Resource Usagebase (merge-base)
branch
File an issue against this benchmark runner |
Contributor
Author
|
run benchmark row_selector |
|
🤖 Arrow criterion benchmark running (GKE) | trigger CPU Details (lscpu)Comparing perf-uneven-mask-intersect-union (635c740) to 3e6bd62 (merge-base) diff Run configurationrun benchmark row_selectorBENCH_COMMAND=cargo bench --features=arrow,async,test_common,experimental,object_store --bench row_selector File an issue against this benchmark runner |
|
🤖 Arrow criterion benchmark completed (GKE) | trigger Instance: Comparing perf-uneven-mask-intersect-union (635c740) to 3e6bd62 (merge-base) diff Run configurationrun benchmark row_selectorCPU Details (lscpu)Details
Resource Usagebase (merge-base)
branch
File an issue against this benchmark runner |
Contributor
Author
|
run benchmark row_selector |
|
🤖 Arrow criterion benchmark running (GKE) | trigger CPU Details (lscpu)Comparing perf-uneven-mask-intersect-union (635c740) to 3e6bd62 (merge-base) diff Run configurationrun benchmark row_selectorBENCH_COMMAND=cargo bench --features=arrow,async,test_common,experimental,object_store --bench row_selector File an issue against this benchmark runner |
|
🤖 Arrow criterion benchmark completed (GKE) | trigger Instance: Comparing perf-uneven-mask-intersect-union (635c740) to 3e6bd62 (merge-base) diff Run configurationrun benchmark row_selectorCPU Details (lscpu)Details
Resource Usagebase (merge-base)
branch
File an issue against this benchmark runner |
Contributor
Author
|
the benchmark is basicly match this comment #10446 (comment), and the |
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.
Which issue does this PR close?
intersect_masks/union_masksfor unequal lengths with in-place bitwise ops #10425.Rationale for this change
The bitwise
intersection/unionpath, taken when both operands are mask-backed, does avoidable copying in two places:BooleanBufferBuilderthat appends that prefix and the longer side's tail.BitAnd/BitOron&BooleanBuffer, which normalise the result to a zero bit offset. That normalisation is a second allocation plus a shifting copy of the whole mask, and it happens wheneverfrom_bitwise_binary_opreturns a non-zero offset — i.e. when both operands share a non-zero sub-64-bit alignment.Masks here come from
BooleanBuffer::slice, so a non-zero offset is the ordinary case.What changes are included in this PR?
Unequal lengths now copy the longer mask once and apply
&=/|=in place over the common prefix, leaving the tail where it already is. Equal lengths build the result directly withBooleanBuffer::from_bitwise_binary_op, keeping whatever offset it produces. Both keep the sub-byte offset rather than re-aligning, and consumers are already offset-aware.Only the longer mask's own byte range is copied, so a result derived from a small slice of a large buffer does not retain the large allocation.
The uneven path is also what motivated #10444 — it reads the bits past the common prefix back out, so it relies on the in-place op leaving them alone.
Are these changes tested?
test_mask_algebra_with_offsetssweeps a 7×7 offset grid against twelve length pairs, equal and unequal;test_mask_algebra_fuzzruns 200 randomized rounds biased towards equal lengths;test_mask_algebra_does_not_retain_backing_buffercovers the allocation-retention case above. All compare against a bit-by-bit reference.cargo test -p parquet --all-featurespasses (1305 tests); clippy and fmt clean.This PR also adds
mask_intersection/mask_uniontoparquet/benches/row_selector.rs, since the existingintersection/unionbenchmarks build operands withfrom_filtersand never reach this path.3M rows, ~1/3 density. Rows are the operand length ratio, columns are the
(left, right)bit offsets the operands carry. Whether the two offsets agree mod 64 decides which pathfrom_bitwise_binary_optakes, so both are covered.Speedup (
main/ this PR), asintersection/union:(0, 0)(3, 3)(3, 67)(3, 5)equalat(0, 0)is the one case with nothing to save: the operands are already aligned, so the old code never paid for normalisation, and old and new produce identical buffers. Its confidence intervals overlap (11.3±0.21µs vs 11.3±0.28µs, 10.0±0.92µs vs 10.9±0.86µs).codegen-units=1because the default bench profile is noisy here: the shifting path is sensitive to codegen-unit partitioning, enough to move these numbers by ±10% in either direction.Are there any user-facing changes?
No public signature changes and no change to which rows a selection selects. The underlying mask layout can differ, though:
RowSelection::as_mask()is public, and where equal-lengthintersection/unionpreviously always returned a zero-offsetBooleanBuffer, the result may now carry a non-zero offset, which is observable throughvalues(),inner()andptr_eq().as_mask()does not promise a normalised layout and callers must already honourBooleanBuffer::offset(), so this should not be a breaking change.