perf(sort): passthrough last stream that left in SortPreservingMergeStream#23328
perf(sort): passthrough last stream that left in SortPreservingMergeStream#23328rluvaton wants to merge 5 commits into
SortPreservingMergeStream#23328Conversation
|
run benchmarks |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing passthrough-last-stream (d7156e6) to 6a0e76e (merge-base) diff using: clickbench_partitioned File an issue against this benchmark runner |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing passthrough-last-stream (d7156e6) to 6a0e76e (merge-base) diff using: tpcds File an issue against this benchmark runner |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing passthrough-last-stream (d7156e6) to 6a0e76e (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 |
|
🤖 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 |
aee6ced to
ed18da7
Compare
|
@alamb the current failing tests are from a fuzz tests you added that verify that we output the same size for all the batches except 1, however this prevent one of the optimization here that just passthrough the stream and avoid the copy or slicing, WDYT? Same idea as |
2a38ad2 to
07411ff
Compare
I think we should look as to why it was added the first time and make sure the intent is still satisfied |
Which issue does this PR close?
N/A
Rationale for this change
No need to go through the entire Loser tree mechanism when only 1 stream left, so in that case, just passthrough last stream
What changes are included in this PR?
added passthrough for last stream in
SortPreservingMergeStreamAre these changes tested?
Yes, basic test + existing tests
Are there any user-facing changes?
No
FYI, this is one of many upcoming optimizations that I will do for Sort