Use BooleanBuffer bitwise ops instead of arrow::compute::and#21338
Use BooleanBuffer bitwise ops instead of arrow::compute::and#21338Dandandan wants to merge 1 commit intoapache:mainfrom
Conversation
Replace `arrow::compute::and()` calls with `BooleanBuffer` bitwise operations (`&`, `&=`) where possible. This avoids allocating intermediate BooleanArrays with null handling overhead. - catalog/table.rs: accumulate filter masks with `BooleanBuffer &=` - joins/utils.rs: fold equality results with `BooleanBuffer &=` - metadata.rs: combine eq/exactness masks with `BooleanBuffer &` - correlation.rs: combine is_not_null results with `BooleanBuffer &` - approx_percentile_cont_with_weight.rs: same pattern Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
8fae761 to
401ef47
Compare
|
run benchmarks |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing bitwise-assign (401ef47) to 1e93a67 (merge-base) diff using: clickbench_partitioned File an issue against this benchmark runner |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing bitwise-assign (401ef47) to 1e93a67 (merge-base) diff using: tpch File an issue against this benchmark runner |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing bitwise-assign (401ef47) to 1e93a67 (merge-base) diff using: tpcds File an issue against this benchmark runner |
|
🤖 Benchmark completed (GKE) | trigger Instance: CPU Details (lscpu)Details
Resource Usagetpch — base (merge-base)
tpch — branch
File an issue against this benchmark runner |
|
🤖 Benchmark completed (GKE) | trigger Instance: CPU Details (lscpu)Details
Resource Usagetpcds — base (merge-base)
tpcds — branch
File an issue against this benchmark runner |
|
🤖 Benchmark completed (GKE) | trigger Instance: CPU Details (lscpu)Details
Resource Usageclickbench_partitioned — base (merge-base)
clickbench_partitioned — branch
File an issue against this benchmark runner |
|
run benchmark tpch |
| } | ||
|
|
||
| let mut combined_mask: Option<BooleanArray> = None; | ||
| let mut combined_buf: Option<BooleanBuffer> = None; |
There was a problem hiding this comment.
What do you think about implementing this API:
We could then use it for these cases 🤔
There was a problem hiding this comment.
Yeah it is a good idea!
I don't see yet any real e2e improvements so closing this one for now :)
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing bitwise-assign (401ef47) to 1e93a67 (merge-base) diff using: tpch File an issue against this benchmark runner |
|
🤖 Benchmark completed (GKE) | trigger Instance: CPU Details (lscpu)Details
Resource Usagetpch — base (merge-base)
tpch — branch
File an issue against this benchmark runner |
Which issue does this PR close?
N/A - minor optimization
Rationale for this change
arrow::compute::and()operates onBooleanArrayand includes null-handling overhead (computing output null buffer). In many cases, we can useBooleanBufferbitwise operations (&,&=) directly, which:BooleanArrays with null buffers&=(no new allocation per iteration)is_not_null()results)What changes are included in this PR?
Replace
arrow::compute::and()withBooleanBufferbitwise operations in 5 files:catalog/table.rs: Filter mask accumulation loop →BooleanBuffer &=in-placejoins/utils.rs: Equality result fold → explicit loop withBooleanBuffer &=metadata.rs: Combining eq/exactness masks →BooleanBuffer &correlation.rs: Combiningis_not_null()results →values() & values()(no nulls)approx_percentile_cont_with_weight.rs: Sameis_not_null()patternWhere inputs may have nulls (filters, eq results), null values are converted to false at the buffer level via
values() & nulls.inner()before combining.Are these changes tested?
Existing tests cover these code paths.
Are there any user-facing changes?
No.
🤖 Generated with Claude Code