FilterExec : expose push_batch_with_filter on LimitedBatchCoalescer - #25503
Rich-T-kid wants to merge 4 commits into
Conversation
|
run benchmarks |
Arrow's BatchCoalescer already has push_batch_with_filter, which writes surviving rows directly into the coalescer buffer in a single pass. LimitedBatchCoalescer was not exposing it, forcing FilterExec to call filter_record_batch first (materialising a new allocation) and then push_batch (a second copy). Wire push_batch_with_filter through LimitedBatchCoalescer with correct fetch-limit accounting, and update FilterExec to use it, eliminating the intermediate allocation on the hot filter path.
|
run benchmarks |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing rich-T-kid/expose-push-batch-with-filter (d418c19) to 6e4d9ba (merge-base) diff Run configurationrun benchmark tpchResults will be posted here when complete File an issue against this benchmark runner |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing rich-T-kid/expose-push-batch-with-filter (d418c19) to 6e4d9ba (merge-base) diff Run configurationrun benchmark tpcdsResults will be posted here when complete File an issue against this benchmark runner |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing rich-T-kid/expose-push-batch-with-filter (d418c19) to 6e4d9ba (merge-base) diff Run configurationrun benchmark clickbench_partitionedResults will be posted here when complete File an issue against this benchmark runner |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing rich-T-kid/expose-push-batch-with-filter (d418c19) to 6e4d9ba (merge-base) diff Run configurationrun benchmark tpcdsResults will be posted here when complete File an issue against this benchmark runner |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing rich-T-kid/expose-push-batch-with-filter (d418c19) to 6e4d9ba (merge-base) diff Run configurationrun benchmark clickbench_partitionedResults will be posted here when complete File an issue against this benchmark runner |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing rich-T-kid/expose-push-batch-with-filter (d418c19) to 6e4d9ba (merge-base) diff Run configurationrun benchmark tpchResults will be posted here when complete File an issue against this benchmark runner |
|
🤖 Benchmark completed (GKE) | trigger Instance: Comparing rich-T-kid/expose-push-batch-with-filter (d418c19) to 6e4d9ba (merge-base) diff Run configurationrun benchmark tpchCPU Details (lscpu)Details
Resource Usagetpch — base (merge-base)
tpch — branch
File an issue against this benchmark runner |
|
🤖 Benchmark completed (GKE) | trigger Instance: Comparing rich-T-kid/expose-push-batch-with-filter (d418c19) to 6e4d9ba (merge-base) diff Run configurationrun benchmark tpchCPU Details (lscpu)Details
Resource Usagetpch — base (merge-base)
tpch — branch
File an issue against this benchmark runner |
|
🤖 Benchmark completed (GKE) | trigger Instance: Comparing rich-T-kid/expose-push-batch-with-filter (d418c19) to 6e4d9ba (merge-base) diff Run configurationrun benchmark tpcdsCPU Details (lscpu)Details
Resource Usagetpcds — base (merge-base)
tpcds — branch
File an issue against this benchmark runner |
|
🤖 Benchmark completed (GKE) | trigger Instance: Comparing rich-T-kid/expose-push-batch-with-filter (d418c19) to 6e4d9ba (merge-base) diff Run configurationrun benchmark tpcdsCPU Details (lscpu)Details
Resource Usagetpcds — base (merge-base)
tpcds — branch
File an issue against this benchmark runner |
|
🤖 Benchmark completed (GKE) | trigger Instance: Comparing rich-T-kid/expose-push-batch-with-filter (d418c19) to 6e4d9ba (merge-base) diff Run configurationrun benchmark clickbench_partitionedCPU Details (lscpu)Details
Resource Usageclickbench_partitioned — base (merge-base)
clickbench_partitioned — branch
File an issue against this benchmark runner |
|
🤖 Benchmark completed (GKE) | trigger Instance: Comparing rich-T-kid/expose-push-batch-with-filter (d418c19) to 6e4d9ba (merge-base) diff Run configurationrun benchmark clickbench_partitionedCPU Details (lscpu)Details
Resource Usageclickbench_partitioned — base (merge-base)
clickbench_partitioned — branch
File an issue against this benchmark runner |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #25503 +/- ##
==========================================
+ Coverage 82.36% 82.41% +0.05%
==========================================
Files 1137 1138 +1
Lines 433123 435454 +2331
Branches 433123 435454 +2331
==========================================
+ Hits 356746 358892 +2146
- Misses 54846 54849 +3
- Partials 21531 21713 +182 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
d418c19 to
f471266
Compare
|
Thanks @Rich-T-kid , test coverage issue Fetch logic of #[test]
fn test_push_batch_with_filter_nulls_and_fetch() {
let batch = uint32_batch(0..8);
let mut coalescer = LimitedBatchCoalescer::new(batch.schema(), 100, Some(3));
let filter = BooleanArray::from(vec![
None,
Some(true),
None,
Some(false),
Some(true),
Some(true),
None,
Some(true),
]);
assert_eq!(
coalescer.push_batch_with_filter(batch, &filter).unwrap(),
PushBatchStatus::LimitReached,
);
coalescer.finish().unwrap();
let output = coalescer.next_completed_batch().unwrap();
let expected = UInt32Array::from(vec![1, 4, 5]);
assert_eq!(output.column(0).as_ref(), &expected as &dyn arrow::array::Array);
} |
|
run benchmarks |
|
@jayzhan211 thank you for the review, the latest commit should address your comments 🚀 |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing rich-T-kid/expose-push-batch-with-filter (6b1623c) to 6e4d9ba (merge-base) diff Run configurationrun benchmark tpcdsResults will be posted here when complete File an issue against this benchmark runner |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing rich-T-kid/expose-push-batch-with-filter (6b1623c) to 6e4d9ba (merge-base) diff Run configurationrun benchmark clickbench_partitionedResults will be posted here when complete File an issue against this benchmark runner |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing rich-T-kid/expose-push-batch-with-filter (6b1623c) to 6e4d9ba (merge-base) diff Run configurationrun benchmark tpchResults will be posted here when complete File an issue against this benchmark runner |
|
🤖 Benchmark completed (GKE) | trigger Instance: Comparing rich-T-kid/expose-push-batch-with-filter (6b1623c) to 6e4d9ba (merge-base) diff Run configurationrun benchmark tpchCPU Details (lscpu)Details
Resource Usagetpch — base (merge-base)
tpch — branch
File an issue against this benchmark runner |
|
🤖 Benchmark completed (GKE) | trigger Instance: Comparing rich-T-kid/expose-push-batch-with-filter (6b1623c) to 6e4d9ba (merge-base) diff Run configurationrun benchmark tpcdsCPU Details (lscpu)Details
Resource Usagetpcds — base (merge-base)
tpcds — branch
File an issue against this benchmark runner |
|
🤖 Benchmark completed (GKE) | trigger Instance: Comparing rich-T-kid/expose-push-batch-with-filter (6b1623c) to 6e4d9ba (merge-base) diff Run configurationrun benchmark clickbench_partitionedCPU Details (lscpu)Details
Resource Usageclickbench_partitioned — base (merge-base)
clickbench_partitioned — branch
File an issue against this benchmark runner |
Which issue does this PR close?
Rationale for this change
we should be able to avoid an intermediate record batch allocation by using
push_batch_with_filterWhat changes are included in this PR?
Exposes
push_batch_with_filteron LimitedBatchCoalescer and switches FilterExec to use it instead of callingfilter_record_batchfollowed bypush_batch.What is the testing strategy for this PR?
Are there any user-facing changes?
no