Make push_batch_with_filter up to 3x faster#8951
Make push_batch_with_filter up to 3x faster#8951Dandandan wants to merge 106 commits intoapache:mainfrom
push_batch_with_filter up to 3x faster#8951Conversation
Make filtered coalescing faster for primitive
push_batch_with_filter faster for primitive types
push_batch_with_filter faster for primitive typespush_batch_with_filter faster for primitive types: up to 10x faster
push_batch_with_filter faster for primitive types: up to 10x fasterpush_batch_with_filter up to 10x faster for primitive types
|
@alamb you are probably interested in this |
|
YAAAAASSS -- this is exactly the type of thing I was hoping for with BatchCoalescer. I will check this out shortly |
arrow-select/src/coalesce.rs
Outdated
| let filtered_batch = filter_record_batch(&batch, filter)?; | ||
| self.push_batch(filtered_batch) | ||
| // We only support primitve now, fallback to filter_record_batch for other types | ||
| // Also, skip optimization when filter is not very selective |
There was a problem hiding this comment.
Not sure if always better to take into account biggest_coalesce_batch_size
There was a problem hiding this comment.
Yeah, I do think it would be good to take into account biggest_coalesce_batch_size but maybe we can do so as a follow on PR
|
run benchmark filter_kernels |
|
show benchmark queue |
|
🤖 Hi @alamb, you asked to view the benchmark queue (#8951 (comment)).
|
|
Hm it seems it contains a bug, probably makes the benchmark results off as well (will take a look tomorrow). |
|
🤖 |
|
🤖: Benchmark completed Details
|
|
run benchmark coalesce_kernels |
|
🤖 Hi @Dandandan, thanks for the request (#8951 (comment)).
Please choose one or more of these with |
push_batch_with_filter up to 10x faster for primitive typespush_batch_with_filter up to 2x faster for primitive types
push_batch_with_filter up to 2x faster for primitive typespush_batch_with_filter up to 3x faster for primitive types
|
@alamb I think it's ok now - I called AI (Opus 4.5) for some help on the Mainly needs some polish and seeing if we can improve the |
|
🤖 Hi @Dandandan, thanks for the request (#8951 (comment)).
Please choose one or more of these with |
13 similar comments
|
🤖 Hi @Dandandan, thanks for the request (#8951 (comment)).
Please choose one or more of these with |
|
🤖 Hi @Dandandan, thanks for the request (#8951 (comment)).
Please choose one or more of these with |
|
🤖 Hi @Dandandan, thanks for the request (#8951 (comment)).
Please choose one or more of these with |
|
🤖 Hi @Dandandan, thanks for the request (#8951 (comment)).
Please choose one or more of these with |
|
🤖 Hi @Dandandan, thanks for the request (#8951 (comment)).
Please choose one or more of these with |
|
🤖 Hi @Dandandan, thanks for the request (#8951 (comment)).
Please choose one or more of these with |
|
🤖 Hi @Dandandan, thanks for the request (#8951 (comment)).
Please choose one or more of these with |
|
🤖 Hi @Dandandan, thanks for the request (#8951 (comment)).
Please choose one or more of these with |
|
🤖 Hi @Dandandan, thanks for the request (#8951 (comment)).
Please choose one or more of these with |
|
🤖 Hi @Dandandan, thanks for the request (#8951 (comment)).
Please choose one or more of these with |
|
🤖 Hi @Dandandan, thanks for the request (#8951 (comment)).
Please choose one or more of these with |
|
🤖 Hi @Dandandan, thanks for the request (#8951 (comment)).
Please choose one or more of these with |
|
🤖 Hi @Dandandan, thanks for the request (#8951 (comment)).
Please choose one or more of these with |
|
run benchmark coalesce_kernels |
|
🤖 |
|
🤖: Benchmark completed Details
|
|
Hmm that's weird, my local results are so different. |
|
run benchmark coalesce_kernels |
|
🤖 |
|
🤖: Benchmark completed Details
|
|
run benchmark coalesce_kernels |
|
🤖 |
|
🤖: Benchmark completed Details
|
|
run benchmark coalesce_kernels |
|
🤖 |
|
🤖: Benchmark completed Details
|
|
The plan is now to finalize #9284 to see it's effects on this kernel. |
Which issue does this PR close?
push_batch_with_filterfor primitive types #9136Rationale for this change
What changes are included in this PR?
Are these changes tested?
Are there any user-facing changes?