-
Notifications
You must be signed in to change notification settings - Fork 242
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implement optimized AQE support so that exchanges run on GPU where possible #462
Implement optimized AQE support so that exchanges run on GPU where possible #462
Conversation
build |
Signed-off-by: Andy Grove <[email protected]>
9a3b53f
to
85a709b
Compare
Signed-off-by: Andy Grove <[email protected]>
Signed-off-by: Andy Grove <[email protected]>
build |
tests/src/test/scala/com/nvidia/spark/rapids/StringFallbackSuite.scala
Outdated
Show resolved
Hide resolved
Signed-off-by: Andy Grove <[email protected]>
Signed-off-by: Andy Grove <[email protected]>
Signed-off-by: Andy Grove <[email protected]>
Signed-off-by: Andy Grove <[email protected]>
Signed-off-by: Andy Grove <[email protected]>
build |
shims/spark310/src/main/scala/com/nvidia/spark/rapids/shims/spark310/GpuHashJoin.scala
Outdated
Show resolved
Hide resolved
.../spark300/src/main/scala/com/nvidia/spark/rapids/shims/spark300/GpuShuffleExchangeExec.scala
Outdated
Show resolved
Hide resolved
...spark300/src/main/scala/org/apache/spark/sql/rapids/shims/spark300/ShuffleManagerShims.scala
Outdated
Show resolved
Hide resolved
sql-plugin/src/main/scala/org/apache/spark/sql/rapids/ShuffleManagerShims.scala
Outdated
Show resolved
Hide resolved
...park301/src/main/scala/com/nvidia/spark/rapids/shims/spark301/GpuBroadcastExchangeExec.scala
Outdated
Show resolved
Hide resolved
sql-plugin/src/main/scala/com/nvidia/spark/rapids/SparkShims.scala
Outdated
Show resolved
Hide resolved
...plugin/src/main/scala/org/apache/spark/sql/rapids/execution/GpuCustomShuffleReaderExec.scala
Show resolved
Hide resolved
tests/src/test/scala/com/nvidia/spark/rapids/StringFallbackSuite.scala
Outdated
Show resolved
Hide resolved
tests/src/test/scala/com/nvidia/spark/rapids/StringFallbackSuite.scala
Outdated
Show resolved
Hide resolved
Signed-off-by: Andy Grove <[email protected]>
Signed-off-by: Andy Grove <[email protected]>
This PR also closes #492 |
Signed-off-by: Andy Grove <[email protected]>
Signed-off-by: Andy Grove <[email protected]>
Signed-off-by: Andy Grove <[email protected]>
} | ||
|
||
if (!canThisBeReplaced) { | ||
buildSide.willNotWorkOnGpu("the BroadcastHashJoin this feeds is not on the GPU") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also here.
I’m not seeing anything big stand out, just a couple of nits above. Thanks @andygrove |
Signed-off-by: Andy Grove <[email protected]>
build |
Signed-off-by: Andy Grove <[email protected]>
Signed-off-by: Andy Grove <[email protected]>
build |
Signed-off-by: Andy Grove <[email protected]>
build |
@tgravescs @abellina I believe all issues are addressed. I have also enabled integration tests for TPC-H Q2 with AQE now that the GpuFilter fix has been merged. |
Signed-off-by: Andy Grove <[email protected]>
build |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we actually need all the classes with ShuffleManagerShimBase as we could just have a function in Spark300Shims, but I think its fine as they are actively messing with Shuffle, so something else is bound to change there
...spark300/src/main/scala/org/apache/spark/sql/rapids/shims/spark300/ShuffleManagerShims.scala
Outdated
Show resolved
Hide resolved
tests/src/test/scala/com/nvidia/spark/rapids/SparkQueryCompareTestSuite.scala
Show resolved
Hide resolved
sql-plugin/src/main/scala/org/apache/spark/sql/rapids/execution/GpuBroadcastExchangeExec.scala
Outdated
Show resolved
Hide resolved
build |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good pending Jenkins
…ssible (NVIDIA#462) * Implement optimized support for AQE Signed-off-by: Andy Grove <[email protected]> * revert scalastyle config change Signed-off-by: Andy Grove <[email protected]> * prep for review Signed-off-by: Andy Grove <[email protected]> * remove temp debug println Signed-off-by: Andy Grove <[email protected]> * enable AQE tests Signed-off-by: Andy Grove <[email protected]> * fix regression with 3.1.0 Signed-off-by: Andy Grove <[email protected]> * address some formatting issues from PR review Signed-off-by: Andy Grove <[email protected]> * address some formatting issues from PR review Signed-off-by: Andy Grove <[email protected]> * resolve TODO comment and more formatting fixes Signed-off-by: Andy Grove <[email protected]> * remove code duplication Signed-off-by: Andy Grove <[email protected]> * remove blank line Signed-off-by: Andy Grove <[email protected]> * address some style feedback Signed-off-by: Andy Grove <[email protected]> * fix odd indenting in one file Signed-off-by: Andy Grove <[email protected]> * updated configs doc Signed-off-by: Andy Grove <[email protected]> * revert format changes to optimizeGpuPlanTransitions Signed-off-by: Andy Grove <[email protected]> * run python tpch tests with aqe on and off Signed-off-by: Andy Grove <[email protected]> * enable tpch query 2 test in scala and python Signed-off-by: Andy Grove <[email protected]> * Revert "enable tpch query 2 test in scala and python" This reverts commit bcd9783. Signed-off-by: Andy Grove <[email protected]> * fix indent Signed-off-by: Andy Grove <[email protected]> * enable AQE testing for TPC-H query 2 Signed-off-by: Andy Grove <[email protected]> * fix error in python test Signed-off-by: Andy Grove <[email protected]> * rename two source files and remove a blank linke
…ssible (NVIDIA#462) * Implement optimized support for AQE Signed-off-by: Andy Grove <[email protected]> * revert scalastyle config change Signed-off-by: Andy Grove <[email protected]> * prep for review Signed-off-by: Andy Grove <[email protected]> * remove temp debug println Signed-off-by: Andy Grove <[email protected]> * enable AQE tests Signed-off-by: Andy Grove <[email protected]> * fix regression with 3.1.0 Signed-off-by: Andy Grove <[email protected]> * address some formatting issues from PR review Signed-off-by: Andy Grove <[email protected]> * address some formatting issues from PR review Signed-off-by: Andy Grove <[email protected]> * resolve TODO comment and more formatting fixes Signed-off-by: Andy Grove <[email protected]> * remove code duplication Signed-off-by: Andy Grove <[email protected]> * remove blank line Signed-off-by: Andy Grove <[email protected]> * address some style feedback Signed-off-by: Andy Grove <[email protected]> * fix odd indenting in one file Signed-off-by: Andy Grove <[email protected]> * updated configs doc Signed-off-by: Andy Grove <[email protected]> * revert format changes to optimizeGpuPlanTransitions Signed-off-by: Andy Grove <[email protected]> * run python tpch tests with aqe on and off Signed-off-by: Andy Grove <[email protected]> * enable tpch query 2 test in scala and python Signed-off-by: Andy Grove <[email protected]> * Revert "enable tpch query 2 test in scala and python" This reverts commit bcd9783. Signed-off-by: Andy Grove <[email protected]> * fix indent Signed-off-by: Andy Grove <[email protected]> * enable AQE testing for TPC-H query 2 Signed-off-by: Andy Grove <[email protected]> * fix error in python test Signed-off-by: Andy Grove <[email protected]> * rename two source files and remove a blank linke
Signed-off-by: spark-rapids automation <[email protected]>
This PR implements optimized AQE support for Spark 3.0.1 and 3.1.0, where shuffle and broadcast exchanges stay on the GPU where supported.