[GLUTEN-10933][VL] Support resize GPU batch and shuffle reader outputs cudf::table#11090
Conversation
|
Run Gluten ClickHouse CI on ARM |
2 similar comments
|
Run Gluten ClickHouse CI on ARM |
|
Run Gluten ClickHouse CI on ARM |
|
Run Gluten ClickHouse CI on ARM |
|
Run Gluten ClickHouse CI on ARM |
|
Run Gluten ClickHouse CI on ARM |
2 similar comments
|
Run Gluten ClickHouse CI on ARM |
|
Run Gluten ClickHouse CI on ARM |
17f23d0 to
a37f84f
Compare
|
Run Gluten ClickHouse CI on ARM |
2 similar comments
|
Run Gluten ClickHouse CI on ARM |
|
Run Gluten ClickHouse CI on ARM |
|
Could you help review this PR? Thanks! @marin-ma @zhztheplayer |
marin-ma
left a comment
There was a problem hiding this comment.
Just left some comments. Thanks!
| @@ -171,6 +171,17 @@ class ColumnarShuffleWriter[K, V]( | |||
| conf.get(SHUFFLE_SORT_USE_RADIXSORT), | |||
| partitionWriterHandle | |||
| ) | |||
| } else if (dep.shuffleWriterType == GpuHashShuffleWriterType) { | |||
There was a problem hiding this comment.
nit: use if(dep.shuffleWriterType == SortShuffleWriterType) in the condition branch above
There was a problem hiding this comment.
Do you mean change the L162? This looks like not related to this PR?
| @@ -20,6 +20,10 @@ | |||
| #include "shuffle/VeloxRssSortShuffleWriter.h" | |||
| #include "shuffle/VeloxSortShuffleWriter.h" | |||
|
|
|||
| #ifdef GLUTEN_ENABLE_GPU | |||
| #include "GpuShuffleWriter.h" | |||
There was a problem hiding this comment.
nit: #include "shuffle/GpuShuffleWriter.h"
Should we name it as VeloxGpuShuffleWriter to align the naming with the existing shuffle writer types?
| * limitations under the License. | ||
| */ | ||
|
|
||
| #include "GpuShuffleWriter.h" |
|
|
||
| #pragma once | ||
|
|
||
| #include "VeloxHashShuffleWriter.h" |
|
|
||
| namespace gluten { | ||
|
|
||
| class GpuBufferBatchResizer : public ColumnarBatchIterator { |
There was a problem hiding this comment.
It would be nice to have some cpp unit test for the functionality of this class.
There was a problem hiding this comment.
It is in VeloxGpuShuffleWriterTest https://github.com/apache/incubator-gluten/pull/11090/files#diff-54f34ba7d66ecb2863b96eb60ffb3a49823d8880ec280d80499fc9c759478a12R113
2cd677b to
7a304f8
Compare
|
Run Gluten ClickHouse CI on ARM |
1 similar comment
|
Run Gluten ClickHouse CI on ARM |
|
Run Gluten ClickHouse CI on X86 |
|
Run Gluten ClickHouse CI on ARM |
1 similar comment
|
Run Gluten ClickHouse CI on ARM |
c5f37cd to
744d564
Compare
|
Run Gluten ClickHouse CI on ARM |
2 similar comments
|
Run Gluten ClickHouse CI on ARM |
|
Run Gluten ClickHouse CI on ARM |
7633f06 to
89c5c6a
Compare
|
Run Gluten ClickHouse CI on ARM |
|
Run Gluten ClickHouse CI on ARM |
|
Run Gluten ClickHouse CI on ARM |
|
Do you have further comments? @marin-ma |
|
Run Gluten ClickHouse CI on X86 |
| BackendsApiManager.getSparkPlanExecApiInstance.getShuffleWriterType(outputPartitioning, output) | ||
| lazy val shuffleWriterType: ShuffleWriterType = getShuffleWriterType | ||
|
|
||
| // super.stringArgs ++ Iterator(output.map(o => s"${o}#${o.dataType.simpleString}")) |
There was a problem hiding this comment.
Looks like an outdated comment. Could you remove?
…r outputs cudf::table (apache#11090)" This reverts commit 157b96e.
Now in the post rule, we can only get the stage level plan, so we cannot decide if the ColumnarExchange output should be cudf format, currently, we suppose the ColumnarExchange is always CudfColumnarExchange, will support validation in injectQueryStagePrepRule where we can see the total plan, generated the transformed plan and validate it, then discard it.
After CudfColumnarExchange, concat the batches in
GpuResizeBufferColumnarBatchExecto output cudf::table with batch size maximum integer, velox-cudf does not control the batch size now. So the CPU shuffle reader does the decompression and prepare the first batch while GPU executes the CPU tasks one by one.Performance:
After this change, the stage 2 in TPCDS Q95 SF100 time decreases from 27s to 13s.
In the shuffle writer, split the bool to byte, and timestamp to nanoseconds int64_t to match the cudf format.
Now BroadcastExchange outputs RowVector, so the CudfFromVeclox operator should accept RowVector and CudfVector with this commit, oap-project/velox@3d6ab40
Also fix the Gluten build with GPU.
Related issue: #10933