Re: [PR] [VL] Provide a configuration option to completely turn off off-heap memory tracking with Spark memory manager [incubator-gluten]
zhztheplayer commented on PR #9341: URL: https://github.com/apache/incubator-gluten/pull/9341#issuecomment-281096 > 2) dynamic off-heap sizing. There are still some messy code need to sort out for off-heap sizing. I'll have another PR for that. After this series of work I hope we can either continue on or immediately remove the off-heap sizing feature in future based on our decision. Because the code is made more independent by the effort. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] [VL] Provide a configuration option to completely turn off off-heap memory tracking with Spark memory manager [incubator-gluten]
zhztheplayer merged PR #9341: URL: https://github.com/apache/incubator-gluten/pull/9341 -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] [VL] Provide a configuration option to completely turn off off-heap memory tracking with Spark memory manager [incubator-gluten]
zhztheplayer commented on code in PR #9341:
URL: https://github.com/apache/incubator-gluten/pull/9341#discussion_r2047153954
##
gluten-core/src/main/scala/org/apache/gluten/GlutenPlugin.scala:
##
@@ -135,6 +135,37 @@ private[gluten] class GlutenDriverPlugin extends
DriverPlugin with Logging {
}
}
+ private def checkOffHeapSettings(conf: SparkConf): Unit = {
+if (
+ conf.getBoolean(
+DYNAMIC_OFFHEAP_SIZING_ENABLED.key,
+DYNAMIC_OFFHEAP_SIZING_ENABLED.defaultValue.get)
+) {
+ // When dynamic off-heap sizing is enabled, off-heap mode is not
strictly required to be
+ // enabled. Skip the check.
+ return
+}
+
+if (
+ conf.getBoolean(COLUMNAR_MEMORY_UNTRACKED.key,
COLUMNAR_MEMORY_UNTRACKED.defaultValue.get)
Review Comment:
Yes, as mentioned in the other comment, it's allowed for user to set
```
spark.memory.offHeap.enabled=false
spark.gluten.memory.untracked=true
```
at the same time.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] [VL] Provide a configuration option to completely turn off off-heap memory tracking with Spark memory manager [incubator-gluten]
zhztheplayer commented on code in PR #9341:
URL: https://github.com/apache/incubator-gluten/pull/9341#discussion_r2047152307
##
shims/common/src/main/scala/org/apache/gluten/config/GlutenConfig.scala:
##
@@ -1241,6 +1243,16 @@ object GlutenConfig {
.booleanConf
.createWithDefault(false)
+ val COLUMNAR_MEMORY_UNTRACKED =
+buildConf("spark.gluten.memory.untracked")
+ .internal()
+ .doc(
+"When enabled, turn all native memory allocations in Gluten into
untracked. Spark " +
Review Comment:
It doesn't relate to the off-heap sizing feature, the idea use case is to
allow user set
```
spark.memory.offHeap.enabled=false
spark.gluten.memory.untracked=true
```
to bypass allocation tracking from Spark memory manager.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] [VL] Provide a configuration option to completely turn off off-heap memory tracking with Spark memory manager [incubator-gluten]
zhouyuan commented on code in PR #9341:
URL: https://github.com/apache/incubator-gluten/pull/9341#discussion_r2047094790
##
gluten-core/src/main/scala/org/apache/gluten/GlutenPlugin.scala:
##
@@ -135,6 +135,37 @@ private[gluten] class GlutenDriverPlugin extends
DriverPlugin with Logging {
}
}
+ private def checkOffHeapSettings(conf: SparkConf): Unit = {
+if (
+ conf.getBoolean(
+DYNAMIC_OFFHEAP_SIZING_ENABLED.key,
+DYNAMIC_OFFHEAP_SIZING_ENABLED.defaultValue.get)
+) {
+ // When dynamic off-heap sizing is enabled, off-heap mode is not
strictly required to be
+ // enabled. Skip the check.
+ return
+}
+
+if (
+ conf.getBoolean(COLUMNAR_MEMORY_UNTRACKED.key,
COLUMNAR_MEMORY_UNTRACKED.defaultValue.get)
Review Comment:
looking at the logic, if `DYNAMIC_OFFHEAP_SIZING_ENABLED=false`, and
`COLUMNAR_MEMORY_UNTRACKED=true` then it will also skip the check of off-heap
settings, is this inteded?
##
shims/common/src/main/scala/org/apache/gluten/config/GlutenConfig.scala:
##
@@ -1241,6 +1243,16 @@ object GlutenConfig {
.booleanConf
.createWithDefault(false)
+ val COLUMNAR_MEMORY_UNTRACKED =
+buildConf("spark.gluten.memory.untracked")
+ .internal()
+ .doc(
+"When enabled, turn all native memory allocations in Gluten into
untracked. Spark " +
Review Comment:
based on the description, this feature should be effect only with
`DYNAMIC_OFFHEAP_SIZING_ENABLED` case? or do you intend to introduce this
feature in case with static off-heap also?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
