You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@gluten.apache.org by "zhouyuan (via GitHub)" <gi...@apache.org> on 2025/09/05 09:24:07 UTC
[PR] [VL] Adding configuration for hash table build [incubator-gluten]
zhouyuan opened a new pull request, #10634:
URL: https://github.com/apache/incubator-gluten/pull/10634
<!--
Thank you for submitting a pull request! Here are some tips:
1. For first-time contributors, please read our contributing guide:
https://github.com/apache/incubator-gluten/blob/main/CONTRIBUTING.md
2. If necessary, create a GitHub issue for discussion beforehand to avoid duplicate work.
3. If the PR is specific to a single backend, include [VL] or [CH] in the PR title to indicate the
Velox or ClickHouse backend, respectively.
4. If the PR is not ready for review, please mark it as a draft.
-->
## What changes are proposed in this pull request?
Adding configurations for hash map build optimization in https://github.com/facebookincubator/velox/pull/7066
<!--
Provide a clear and concise description of the changes introduced in this PR.
Ensure the PR description aligns with the code changes, especially after updates.
If applicable, include "Fixes #<GitHub_Issue_ID>" to automatically close the corresponding issue
when the PR is merged.
-->
## How was this patch tested?
<!--
Describe how the changes were tested, if applicable.
Include new tests to validate the functionality, if necessary.
For UI-related changes, attach screenshots to demonstrate the updates.
-->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-10660][VL] Adding configuration for hash table build [incubator-gluten]
Posted by "liujiayi771 (via GitHub)" <gi...@apache.org>.
liujiayi771 commented on PR #10634:
URL: https://github.com/apache/incubator-gluten/pull/10634#issuecomment-3323882005
> @liujiayi771 are you suggesting it's better to disable this feature by default?
Yes, for the Q95 build side join keys, the duplication rate is very high, so the optimization is very significant. However, for production jobs without a high duplication rate, there could be a performance regression. It's necessary to adjust the abandon percent based on the specific job conditions.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-10660][VL] Adding configuration for hash table build [incubator-gluten]
Posted by "FelixYBW (via GitHub)" <gi...@apache.org>.
FelixYBW commented on PR #10634:
URL: https://github.com/apache/incubator-gluten/pull/10634#issuecomment-3282257611
At least for TPCDS test, I didn't see any performance regression with ` spark.gluten.velox.abandonbuild.noduphashminpct=100 `. I think we can enable it by default.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-10660][VL] Adding configuration for hash table build [incubator-gluten]
Posted by "zhouyuan (via GitHub)" <gi...@apache.org>.
zhouyuan merged PR #10634:
URL: https://github.com/apache/incubator-gluten/pull/10634
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-10660][VL] Adding configuration for hash table build [incubator-gluten]
Posted by "zhouyuan (via GitHub)" <gi...@apache.org>.
zhouyuan commented on PR #10634:
URL: https://github.com/apache/incubator-gluten/pull/10634#issuecomment-3279214702
@liujiayi771 I vote for enabling this by default
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-10660][VL] Adding configuration for hash table build [incubator-gluten]
Posted by "liujiayi771 (via GitHub)" <gi...@apache.org>.
liujiayi771 commented on PR #10634:
URL: https://github.com/apache/incubator-gluten/pull/10634#issuecomment-3273521453
After the testing, should we enable this feature by default, or should it be disabled by default?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-10660][VL] Adding configuration for hash table build [incubator-gluten]
Posted by "zhouyuan (via GitHub)" <gi...@apache.org>.
zhouyuan commented on PR #10634:
URL: https://github.com/apache/incubator-gluten/pull/10634#issuecomment-3319506621
@liujiayi771 are you suggesting it's better to disable this feature by default?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-10660][VL] Adding configuration for hash table build [incubator-gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #10634:
URL: https://github.com/apache/incubator-gluten/pull/10634#issuecomment-3270774237
https://github.com/apache/incubator-gluten/issues/10660
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-10660][VL] Adding configuration for hash table build [incubator-gluten]
Posted by "liujiayi771 (via GitHub)" <gi...@apache.org>.
liujiayi771 commented on PR #10634:
URL: https://github.com/apache/incubator-gluten/pull/10634#issuecomment-3283673951
TPCDS has very few queries that contain semi/anti joins, and all of these involve duplicate join keys.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [VL] Adding configuration for hash table build [incubator-gluten]
Posted by "liujiayi771 (via GitHub)" <gi...@apache.org>.
liujiayi771 commented on PR #10634:
URL: https://github.com/apache/incubator-gluten/pull/10634#issuecomment-3265699591
We need to set spark.gluten.velox.abandonbuild.noduphashminpct=100 to verify the behavior when deduplication is enabled.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-10660][VL] Adding configuration for hash table build [incubator-gluten]
Posted by "marin-ma (via GitHub)" <gi...@apache.org>.
marin-ma commented on code in PR #10634:
URL: https://github.com/apache/incubator-gluten/pull/10634#discussion_r2368954765
##########
backends-velox/src/main/scala/org/apache/gluten/config/VeloxConfig.scala:
##########
@@ -529,6 +529,20 @@ object VeloxConfig {
.booleanConf
.createWithDefault(false)
+ val VELOX_HASHMAP_ABANDON_BUILD_DUPHASH_MIN_ROWS =
+ buildConf("spark.gluten.velox.abandonbuild.noduphashminrows")
+ .experimental()
+ .doc("Experimental: abandon hashmap build if duplicated rows more than this number.")
+ .intConf
+ .createWithDefault(100000)
+
+ val VELOX_HASHMAP_ABANDON_BUILD_DUPHASH_MIN_PCT =
+ buildConf("spark.gluten.velox.abandonbuild.noduphashminpct")
+ .experimental()
+ .doc("Experimental: abandon hashmap build if duplicated rows are more than this pct.")
Review Comment:
Perhaps use "percentile" to avoid abbreviations in the doc.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-10660][VL] Adding configuration for hash table build [incubator-gluten]
Posted by "zjuwangg (via GitHub)" <gi...@apache.org>.
zjuwangg commented on code in PR #10634:
URL: https://github.com/apache/incubator-gluten/pull/10634#discussion_r2354747315
##########
backends-velox/src/main/scala/org/apache/gluten/config/VeloxConfig.scala:
##########
@@ -545,6 +545,20 @@ object VeloxConfig {
.booleanConf
.createWithDefault(false)
+ val VELOX_HASHMAP_ABANDON_BUILD_DUPHASH_MIN_ROWS =
+ buildConf("spark.gluten.velox.abandonbuild.noduphashminrows")
+ .internal()
+ .doc("Experimental: abandon hashmap build if duplicated rows more than this number.")
Review Comment:
Should add experimental() method and re-generate configuration doc if needed.
##########
backends-velox/src/main/scala/org/apache/gluten/config/VeloxConfig.scala:
##########
@@ -545,6 +545,20 @@ object VeloxConfig {
.booleanConf
.createWithDefault(false)
+ val VELOX_HASHMAP_ABANDON_BUILD_DUPHASH_MIN_ROWS =
+ buildConf("spark.gluten.velox.abandonbuild.noduphashminrows")
+ .internal()
+ .doc("Experimental: abandon hashmap build if duplicated rows more than this number.")
Review Comment:
Should add `experimental()` method and re-generate configuration doc if needed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-10660][VL] Adding configuration for hash table build [incubator-gluten]
Posted by "zjuwangg (via GitHub)" <gi...@apache.org>.
zjuwangg commented on code in PR #10634:
URL: https://github.com/apache/incubator-gluten/pull/10634#discussion_r2354748240
##########
backends-velox/src/main/scala/org/apache/gluten/config/VeloxConfig.scala:
##########
@@ -545,6 +545,20 @@ object VeloxConfig {
.booleanConf
.createWithDefault(false)
+ val VELOX_HASHMAP_ABANDON_BUILD_DUPHASH_MIN_ROWS =
+ buildConf("spark.gluten.velox.abandonbuild.noduphashminrows")
+ .internal()
+ .doc("Experimental: abandon hashmap build if duplicated rows more than this number.")
+ .intConf
+ .createWithDefault(100000)
+
+ val VELOX_HASHMAP_ABANDON_BUILD_DUPHASH_MIN_PCT =
+ buildConf("spark.gluten.velox.abandonbuild.noduphashminpct")
+ .internal()
Review Comment:
ditto.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org