You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@gluten.apache.org by "zhztheplayer (via GitHub)" <gi...@apache.org> on 2025/04/15 15:42:37 UTC
[PR] [VL] Move pre-configuration code of dynamic off-heap sizing to its own place, and minor refactors against the feature [incubator-gluten]
zhztheplayer opened a new pull request, #9336:
URL: https://github.com/apache/incubator-gluten/pull/9336
1. Move pre-configuration code from `GlutenConfig.scala` to `DynamicOffHeapSizingMemoryTarget`.
2. Avoid chaining `DynamicOffHeapSizingMemoryTarget` with Spark consumer because it basically never triggers spills because of its design. The target will manage OOMs by itself.
3. Other minor refactors.
--
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] Move pre-configuration code of dynamic off-heap sizing to its own place [incubator-gluten]
Posted by "zhztheplayer (via GitHub)" <gi...@apache.org>.
zhztheplayer merged PR #9336:
URL: https://github.com/apache/incubator-gluten/pull/9336
--
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] Move pre-configuration code of dynamic off-heap sizing to its own place, and minor refactors against the feature [incubator-gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9336:
URL: https://github.com/apache/incubator-gluten/pull/9336#issuecomment-2806412410
Run Gluten Clickhouse CI on x86
--
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] Move pre-configuration code of dynamic off-heap sizing to its own place [incubator-gluten]
Posted by "zhztheplayer (via GitHub)" <gi...@apache.org>.
zhztheplayer commented on code in PR #9336:
URL: https://github.com/apache/incubator-gluten/pull/9336#discussion_r2049103253
##########
gluten-core/src/main/scala/org/apache/gluten/GlutenPlugin.scala:
##########
@@ -188,74 +188,25 @@ private[gluten] class GlutenDriverPlugin extends DriverPlugin with Logging {
// check memory off-heap enabled and size.
checkOffHeapSettings(conf)
- // Task slots.
- val taskSlots = SparkResourceUtil.getTaskSlots(conf)
- conf.set(NUM_TASK_SLOTS_PER_EXECUTOR.key, taskSlots.toString)
-
- val onHeapSize: Long = conf.getSizeAsBytes(SPARK_ONHEAP_SIZE_KEY, 1024 * 1024 * 1024)
-
- // If dynamic off-heap sizing is enabled, the off-heap size is calculated based on the on-heap
- // size. Otherwise, the off-heap size is set to the value specified by the user (if any).
- // Note that this means that we will IGNORE the off-heap size specified by the user if the
- // dynamic off-heap feature is enabled.
- val offHeapSize: Long =
- if (
- conf.getBoolean(
- DYNAMIC_OFFHEAP_SIZING_ENABLED.key,
- DYNAMIC_OFFHEAP_SIZING_ENABLED.defaultValue.get)
- ) {
- // Since when dynamic off-heap sizing is enabled, we commingle on-heap
- // and off-heap memory, we set the off-heap size to the usable on-heap size. We will
- // size it with a memory fraction, which can be aggressively set, but the default
- // is using the same way that Spark sizes on-heap memory:
- //
- // spark.gluten.memory.dynamic.offHeap.sizing.memory.fraction *
- // (spark.executor.memory - 300MB).
- //
- // We will be careful to use the same configuration settings as Spark to ensure
- // that we are sizing the off-heap memory in the same way as Spark sizes on-heap memory.
- // The 300MB value, unfortunately, is hard-coded in Spark code.
- ((onHeapSize - (300 * 1024 * 1024)) *
- conf.getDouble(DYNAMIC_OFFHEAP_SIZING_MEMORY_FRACTION.key, 0.6d)).toLong
- } else {
- // Optimistic off-heap sizes, assuming all storage memory can be borrowed into execution
- // memory pool, regardless of Spark option spark.memory.storageFraction.
- conf.getSizeAsBytes(SPARK_OFFHEAP_SIZE_KEY, 0L)
- }
+ // Get the off-heap size set by user.
+ val offHeapSize = conf.getSizeAsBytes(SPARK_OFFHEAP_SIZE_KEY)
+ // Set off-heap size in bytes.
conf.set(COLUMNAR_OFFHEAP_SIZE_IN_BYTES.key, offHeapSize.toString)
- conf.set(SPARK_OFFHEAP_SIZE_KEY, offHeapSize.toString)
+ // Set off-heap size in bytes per task.
+ val taskSlots = SparkResourceUtil.getTaskSlots(conf)
+ conf.set(NUM_TASK_SLOTS_PER_EXECUTOR.key, taskSlots.toString)
val offHeapPerTask = offHeapSize / taskSlots
conf.set(COLUMNAR_TASK_OFFHEAP_SIZE_IN_BYTES.key, offHeapPerTask.toString)
- // If we are using dynamic off-heap sizing, we should also enable off-heap memory
- // officially.
- if (
- conf.getBoolean(
- DYNAMIC_OFFHEAP_SIZING_ENABLED.key,
- DYNAMIC_OFFHEAP_SIZING_ENABLED.defaultValue.get)
- ) {
- conf.set(SPARK_OFFHEAP_ENABLED, "true")
-
- // We already sized the off-heap per task in a conservative manner, so we can just
- // use it.
- conf.set(COLUMNAR_CONSERVATIVE_TASK_OFFHEAP_SIZE_IN_BYTES.key, offHeapPerTask.toString)
- } else {
- // Let's make sure this is set to false explicitly if it is not on as it
- // is looked up when throwing OOF exceptions.
- conf.set(
- DYNAMIC_OFFHEAP_SIZING_ENABLED.key,
- DYNAMIC_OFFHEAP_SIZING_ENABLED.defaultValueString)
-
- // Pessimistic off-heap sizes, with the assumption that all non-borrowable storage memory
- // determined by spark.memory.storageFraction was used.
- val fraction = 1.0d - conf.getDouble("spark.memory.storageFraction", 0.5d)
- val conservativeOffHeapPerTask = (offHeapSize * fraction).toLong / taskSlots
- conf.set(
- COLUMNAR_CONSERVATIVE_TASK_OFFHEAP_SIZE_IN_BYTES.key,
- conservativeOffHeapPerTask.toString)
- }
+ // Pessimistic off-heap sizes, with the assumption that all non-borrowable storage memory
+ // determined by spark.memory.storageFraction was used.
+ val fraction = 1.0d - conf.getDouble("spark.memory.storageFraction", 0.5d)
+ val conservativeOffHeapPerTask = (offHeapSize * fraction).toLong / taskSlots
Review Comment:
We don't have to deal with off-heap size in the future anymore since we no longer rely on off-heap memory manager when the feature is on.
We are just recovering the old code here before off-heap sizing was landed: https://github.com/apache/incubator-gluten/blame/022c208564dbb6dc59b7d64fbc4ad002c166e012/gluten-core/src/main/scala/org/apache/gluten/GlutenPlugin.scala
--
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] Move pre-configuration code of dynamic off-heap sizing to its own place [incubator-gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9336:
URL: https://github.com/apache/incubator-gluten/pull/9336#issuecomment-2812298407
Run Gluten Clickhouse CI on x86
--
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] Move pre-configuration code of dynamic off-heap sizing to its own place [incubator-gluten]
Posted by "zhztheplayer (via GitHub)" <gi...@apache.org>.
zhztheplayer commented on code in PR #9336:
URL: https://github.com/apache/incubator-gluten/pull/9336#discussion_r2049103253
##########
gluten-core/src/main/scala/org/apache/gluten/GlutenPlugin.scala:
##########
@@ -188,74 +188,25 @@ private[gluten] class GlutenDriverPlugin extends DriverPlugin with Logging {
// check memory off-heap enabled and size.
checkOffHeapSettings(conf)
- // Task slots.
- val taskSlots = SparkResourceUtil.getTaskSlots(conf)
- conf.set(NUM_TASK_SLOTS_PER_EXECUTOR.key, taskSlots.toString)
-
- val onHeapSize: Long = conf.getSizeAsBytes(SPARK_ONHEAP_SIZE_KEY, 1024 * 1024 * 1024)
-
- // If dynamic off-heap sizing is enabled, the off-heap size is calculated based on the on-heap
- // size. Otherwise, the off-heap size is set to the value specified by the user (if any).
- // Note that this means that we will IGNORE the off-heap size specified by the user if the
- // dynamic off-heap feature is enabled.
- val offHeapSize: Long =
- if (
- conf.getBoolean(
- DYNAMIC_OFFHEAP_SIZING_ENABLED.key,
- DYNAMIC_OFFHEAP_SIZING_ENABLED.defaultValue.get)
- ) {
- // Since when dynamic off-heap sizing is enabled, we commingle on-heap
- // and off-heap memory, we set the off-heap size to the usable on-heap size. We will
- // size it with a memory fraction, which can be aggressively set, but the default
- // is using the same way that Spark sizes on-heap memory:
- //
- // spark.gluten.memory.dynamic.offHeap.sizing.memory.fraction *
- // (spark.executor.memory - 300MB).
- //
- // We will be careful to use the same configuration settings as Spark to ensure
- // that we are sizing the off-heap memory in the same way as Spark sizes on-heap memory.
- // The 300MB value, unfortunately, is hard-coded in Spark code.
- ((onHeapSize - (300 * 1024 * 1024)) *
- conf.getDouble(DYNAMIC_OFFHEAP_SIZING_MEMORY_FRACTION.key, 0.6d)).toLong
- } else {
- // Optimistic off-heap sizes, assuming all storage memory can be borrowed into execution
- // memory pool, regardless of Spark option spark.memory.storageFraction.
- conf.getSizeAsBytes(SPARK_OFFHEAP_SIZE_KEY, 0L)
- }
+ // Get the off-heap size set by user.
+ val offHeapSize = conf.getSizeAsBytes(SPARK_OFFHEAP_SIZE_KEY)
+ // Set off-heap size in bytes.
conf.set(COLUMNAR_OFFHEAP_SIZE_IN_BYTES.key, offHeapSize.toString)
- conf.set(SPARK_OFFHEAP_SIZE_KEY, offHeapSize.toString)
+ // Set off-heap size in bytes per task.
+ val taskSlots = SparkResourceUtil.getTaskSlots(conf)
+ conf.set(NUM_TASK_SLOTS_PER_EXECUTOR.key, taskSlots.toString)
val offHeapPerTask = offHeapSize / taskSlots
conf.set(COLUMNAR_TASK_OFFHEAP_SIZE_IN_BYTES.key, offHeapPerTask.toString)
- // If we are using dynamic off-heap sizing, we should also enable off-heap memory
- // officially.
- if (
- conf.getBoolean(
- DYNAMIC_OFFHEAP_SIZING_ENABLED.key,
- DYNAMIC_OFFHEAP_SIZING_ENABLED.defaultValue.get)
- ) {
- conf.set(SPARK_OFFHEAP_ENABLED, "true")
-
- // We already sized the off-heap per task in a conservative manner, so we can just
- // use it.
- conf.set(COLUMNAR_CONSERVATIVE_TASK_OFFHEAP_SIZE_IN_BYTES.key, offHeapPerTask.toString)
- } else {
- // Let's make sure this is set to false explicitly if it is not on as it
- // is looked up when throwing OOF exceptions.
- conf.set(
- DYNAMIC_OFFHEAP_SIZING_ENABLED.key,
- DYNAMIC_OFFHEAP_SIZING_ENABLED.defaultValueString)
-
- // Pessimistic off-heap sizes, with the assumption that all non-borrowable storage memory
- // determined by spark.memory.storageFraction was used.
- val fraction = 1.0d - conf.getDouble("spark.memory.storageFraction", 0.5d)
- val conservativeOffHeapPerTask = (offHeapSize * fraction).toLong / taskSlots
- conf.set(
- COLUMNAR_CONSERVATIVE_TASK_OFFHEAP_SIZE_IN_BYTES.key,
- conservativeOffHeapPerTask.toString)
- }
+ // Pessimistic off-heap sizes, with the assumption that all non-borrowable storage memory
+ // determined by spark.memory.storageFraction was used.
+ val fraction = 1.0d - conf.getDouble("spark.memory.storageFraction", 0.5d)
+ val conservativeOffHeapPerTask = (offHeapSize * fraction).toLong / taskSlots
Review Comment:
We don't have to deal with off-heap size in the future anymore since we no longer rely on off-heap memory manager.
We are just recovering the old code here before off-heap sizing was landed: https://github.com/apache/incubator-gluten/blame/022c208564dbb6dc59b7d64fbc4ad002c166e012/gluten-core/src/main/scala/org/apache/gluten/GlutenPlugin.scala
--
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] Move pre-configuration code of dynamic off-heap sizing to its own place [incubator-gluten]
Posted by "zhztheplayer (via GitHub)" <gi...@apache.org>.
zhztheplayer commented on code in PR #9336:
URL: https://github.com/apache/incubator-gluten/pull/9336#discussion_r2049096343
##########
shims/common/src/main/scala/org/apache/gluten/config/GlutenConfig.scala:
##########
@@ -1634,7 +1637,7 @@ object GlutenConfig {
.createWithDefault(true)
val DYNAMIC_OFFHEAP_SIZING_ENABLED =
- buildConf("spark.gluten.memory.dynamic.offHeap.sizing.enabled")
+ buildStaticConf("spark.gluten.memory.dynamic.offHeap.sizing.enabled")
Review Comment:
Not really, it's just an improvement for configuration definitions.
--
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] Move pre-configuration code of dynamic off-heap sizing to its own place [incubator-gluten]
Posted by "zhztheplayer (via GitHub)" <gi...@apache.org>.
zhztheplayer commented on code in PR #9336:
URL: https://github.com/apache/incubator-gluten/pull/9336#discussion_r2049103253
##########
gluten-core/src/main/scala/org/apache/gluten/GlutenPlugin.scala:
##########
@@ -188,74 +188,25 @@ private[gluten] class GlutenDriverPlugin extends DriverPlugin with Logging {
// check memory off-heap enabled and size.
checkOffHeapSettings(conf)
- // Task slots.
- val taskSlots = SparkResourceUtil.getTaskSlots(conf)
- conf.set(NUM_TASK_SLOTS_PER_EXECUTOR.key, taskSlots.toString)
-
- val onHeapSize: Long = conf.getSizeAsBytes(SPARK_ONHEAP_SIZE_KEY, 1024 * 1024 * 1024)
-
- // If dynamic off-heap sizing is enabled, the off-heap size is calculated based on the on-heap
- // size. Otherwise, the off-heap size is set to the value specified by the user (if any).
- // Note that this means that we will IGNORE the off-heap size specified by the user if the
- // dynamic off-heap feature is enabled.
- val offHeapSize: Long =
- if (
- conf.getBoolean(
- DYNAMIC_OFFHEAP_SIZING_ENABLED.key,
- DYNAMIC_OFFHEAP_SIZING_ENABLED.defaultValue.get)
- ) {
- // Since when dynamic off-heap sizing is enabled, we commingle on-heap
- // and off-heap memory, we set the off-heap size to the usable on-heap size. We will
- // size it with a memory fraction, which can be aggressively set, but the default
- // is using the same way that Spark sizes on-heap memory:
- //
- // spark.gluten.memory.dynamic.offHeap.sizing.memory.fraction *
- // (spark.executor.memory - 300MB).
- //
- // We will be careful to use the same configuration settings as Spark to ensure
- // that we are sizing the off-heap memory in the same way as Spark sizes on-heap memory.
- // The 300MB value, unfortunately, is hard-coded in Spark code.
- ((onHeapSize - (300 * 1024 * 1024)) *
- conf.getDouble(DYNAMIC_OFFHEAP_SIZING_MEMORY_FRACTION.key, 0.6d)).toLong
- } else {
- // Optimistic off-heap sizes, assuming all storage memory can be borrowed into execution
- // memory pool, regardless of Spark option spark.memory.storageFraction.
- conf.getSizeAsBytes(SPARK_OFFHEAP_SIZE_KEY, 0L)
- }
+ // Get the off-heap size set by user.
+ val offHeapSize = conf.getSizeAsBytes(SPARK_OFFHEAP_SIZE_KEY)
+ // Set off-heap size in bytes.
conf.set(COLUMNAR_OFFHEAP_SIZE_IN_BYTES.key, offHeapSize.toString)
- conf.set(SPARK_OFFHEAP_SIZE_KEY, offHeapSize.toString)
+ // Set off-heap size in bytes per task.
+ val taskSlots = SparkResourceUtil.getTaskSlots(conf)
+ conf.set(NUM_TASK_SLOTS_PER_EXECUTOR.key, taskSlots.toString)
val offHeapPerTask = offHeapSize / taskSlots
conf.set(COLUMNAR_TASK_OFFHEAP_SIZE_IN_BYTES.key, offHeapPerTask.toString)
- // If we are using dynamic off-heap sizing, we should also enable off-heap memory
- // officially.
- if (
- conf.getBoolean(
- DYNAMIC_OFFHEAP_SIZING_ENABLED.key,
- DYNAMIC_OFFHEAP_SIZING_ENABLED.defaultValue.get)
- ) {
- conf.set(SPARK_OFFHEAP_ENABLED, "true")
-
- // We already sized the off-heap per task in a conservative manner, so we can just
- // use it.
- conf.set(COLUMNAR_CONSERVATIVE_TASK_OFFHEAP_SIZE_IN_BYTES.key, offHeapPerTask.toString)
- } else {
- // Let's make sure this is set to false explicitly if it is not on as it
- // is looked up when throwing OOF exceptions.
- conf.set(
- DYNAMIC_OFFHEAP_SIZING_ENABLED.key,
- DYNAMIC_OFFHEAP_SIZING_ENABLED.defaultValueString)
-
- // Pessimistic off-heap sizes, with the assumption that all non-borrowable storage memory
- // determined by spark.memory.storageFraction was used.
- val fraction = 1.0d - conf.getDouble("spark.memory.storageFraction", 0.5d)
- val conservativeOffHeapPerTask = (offHeapSize * fraction).toLong / taskSlots
- conf.set(
- COLUMNAR_CONSERVATIVE_TASK_OFFHEAP_SIZE_IN_BYTES.key,
- conservativeOffHeapPerTask.toString)
- }
+ // Pessimistic off-heap sizes, with the assumption that all non-borrowable storage memory
+ // determined by spark.memory.storageFraction was used.
+ val fraction = 1.0d - conf.getDouble("spark.memory.storageFraction", 0.5d)
+ val conservativeOffHeapPerTask = (offHeapSize * fraction).toLong / taskSlots
Review Comment:
We don't have to deal with off-heap size in the future anymore since we don't rely on off-heap memory manager.
We are just recovering the old code here before off-heap sizing was landed: https://github.com/apache/incubator-gluten/blame/022c208564dbb6dc59b7d64fbc4ad002c166e012/gluten-core/src/main/scala/org/apache/gluten/GlutenPlugin.scala
--
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] Move pre-configuration code of dynamic off-heap sizing to its own place [incubator-gluten]
Posted by "zhouyuan (via GitHub)" <gi...@apache.org>.
zhouyuan commented on code in PR #9336:
URL: https://github.com/apache/incubator-gluten/pull/9336#discussion_r2049075847
##########
shims/common/src/main/scala/org/apache/gluten/config/GlutenConfig.scala:
##########
@@ -1634,7 +1637,7 @@ object GlutenConfig {
.createWithDefault(true)
val DYNAMIC_OFFHEAP_SIZING_ENABLED =
- buildConf("spark.gluten.memory.dynamic.offHeap.sizing.enabled")
+ buildStaticConf("spark.gluten.memory.dynamic.offHeap.sizing.enabled")
Review Comment:
is this a behavior change by changing this to static config?
##########
gluten-core/src/main/scala/org/apache/gluten/GlutenPlugin.scala:
##########
@@ -188,74 +188,25 @@ private[gluten] class GlutenDriverPlugin extends DriverPlugin with Logging {
// check memory off-heap enabled and size.
checkOffHeapSettings(conf)
- // Task slots.
- val taskSlots = SparkResourceUtil.getTaskSlots(conf)
- conf.set(NUM_TASK_SLOTS_PER_EXECUTOR.key, taskSlots.toString)
-
- val onHeapSize: Long = conf.getSizeAsBytes(SPARK_ONHEAP_SIZE_KEY, 1024 * 1024 * 1024)
-
- // If dynamic off-heap sizing is enabled, the off-heap size is calculated based on the on-heap
- // size. Otherwise, the off-heap size is set to the value specified by the user (if any).
- // Note that this means that we will IGNORE the off-heap size specified by the user if the
- // dynamic off-heap feature is enabled.
- val offHeapSize: Long =
- if (
- conf.getBoolean(
- DYNAMIC_OFFHEAP_SIZING_ENABLED.key,
- DYNAMIC_OFFHEAP_SIZING_ENABLED.defaultValue.get)
- ) {
- // Since when dynamic off-heap sizing is enabled, we commingle on-heap
- // and off-heap memory, we set the off-heap size to the usable on-heap size. We will
- // size it with a memory fraction, which can be aggressively set, but the default
- // is using the same way that Spark sizes on-heap memory:
- //
- // spark.gluten.memory.dynamic.offHeap.sizing.memory.fraction *
- // (spark.executor.memory - 300MB).
- //
- // We will be careful to use the same configuration settings as Spark to ensure
- // that we are sizing the off-heap memory in the same way as Spark sizes on-heap memory.
- // The 300MB value, unfortunately, is hard-coded in Spark code.
- ((onHeapSize - (300 * 1024 * 1024)) *
- conf.getDouble(DYNAMIC_OFFHEAP_SIZING_MEMORY_FRACTION.key, 0.6d)).toLong
- } else {
- // Optimistic off-heap sizes, assuming all storage memory can be borrowed into execution
- // memory pool, regardless of Spark option spark.memory.storageFraction.
- conf.getSizeAsBytes(SPARK_OFFHEAP_SIZE_KEY, 0L)
- }
+ // Get the off-heap size set by user.
+ val offHeapSize = conf.getSizeAsBytes(SPARK_OFFHEAP_SIZE_KEY)
+ // Set off-heap size in bytes.
conf.set(COLUMNAR_OFFHEAP_SIZE_IN_BYTES.key, offHeapSize.toString)
- conf.set(SPARK_OFFHEAP_SIZE_KEY, offHeapSize.toString)
+ // Set off-heap size in bytes per task.
+ val taskSlots = SparkResourceUtil.getTaskSlots(conf)
+ conf.set(NUM_TASK_SLOTS_PER_EXECUTOR.key, taskSlots.toString)
val offHeapPerTask = offHeapSize / taskSlots
conf.set(COLUMNAR_TASK_OFFHEAP_SIZE_IN_BYTES.key, offHeapPerTask.toString)
- // If we are using dynamic off-heap sizing, we should also enable off-heap memory
- // officially.
- if (
- conf.getBoolean(
- DYNAMIC_OFFHEAP_SIZING_ENABLED.key,
- DYNAMIC_OFFHEAP_SIZING_ENABLED.defaultValue.get)
- ) {
- conf.set(SPARK_OFFHEAP_ENABLED, "true")
-
- // We already sized the off-heap per task in a conservative manner, so we can just
- // use it.
- conf.set(COLUMNAR_CONSERVATIVE_TASK_OFFHEAP_SIZE_IN_BYTES.key, offHeapPerTask.toString)
- } else {
- // Let's make sure this is set to false explicitly if it is not on as it
- // is looked up when throwing OOF exceptions.
- conf.set(
- DYNAMIC_OFFHEAP_SIZING_ENABLED.key,
- DYNAMIC_OFFHEAP_SIZING_ENABLED.defaultValueString)
-
- // Pessimistic off-heap sizes, with the assumption that all non-borrowable storage memory
- // determined by spark.memory.storageFraction was used.
- val fraction = 1.0d - conf.getDouble("spark.memory.storageFraction", 0.5d)
- val conservativeOffHeapPerTask = (offHeapSize * fraction).toLong / taskSlots
- conf.set(
- COLUMNAR_CONSERVATIVE_TASK_OFFHEAP_SIZE_IN_BYTES.key,
- conservativeOffHeapPerTask.toString)
- }
+ // Pessimistic off-heap sizes, with the assumption that all non-borrowable storage memory
+ // determined by spark.memory.storageFraction was used.
+ val fraction = 1.0d - conf.getDouble("spark.memory.storageFraction", 0.5d)
+ val conservativeOffHeapPerTask = (offHeapSize * fraction).toLong / taskSlots
Review Comment:
it looks like changed the scope: `conservativeOffHeapPerTask` is effective on `DYNAMIC_OFFHEAP_SIZING_ENABLED=false` on previous code
--
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] Move pre-configuration code of dynamic off-heap sizing to its own place, and minor refactors against the feature [incubator-gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9336:
URL: https://github.com/apache/incubator-gluten/pull/9336#issuecomment-2806757973
Run Gluten Clickhouse CI on x86
--
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] Move pre-configuration code of dynamic off-heap sizing to its own place, and minor refactors against the feature [incubator-gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9336:
URL: https://github.com/apache/incubator-gluten/pull/9336#issuecomment-2812258327
Run Gluten Clickhouse CI on x86
--
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] Move pre-configuration code of dynamic off-heap sizing to its own place [incubator-gluten]
Posted by "zhouyuan (via GitHub)" <gi...@apache.org>.
zhouyuan commented on PR #9336:
URL: https://github.com/apache/incubator-gluten/pull/9336#issuecomment-2813137242
Cc: @zhli1142015 to be aware of this
--
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] Move pre-configuration code of dynamic off-heap sizing to its own place [incubator-gluten]
Posted by "zhztheplayer (via GitHub)" <gi...@apache.org>.
zhztheplayer commented on code in PR #9336:
URL: https://github.com/apache/incubator-gluten/pull/9336#discussion_r2049103253
##########
gluten-core/src/main/scala/org/apache/gluten/GlutenPlugin.scala:
##########
@@ -188,74 +188,25 @@ private[gluten] class GlutenDriverPlugin extends DriverPlugin with Logging {
// check memory off-heap enabled and size.
checkOffHeapSettings(conf)
- // Task slots.
- val taskSlots = SparkResourceUtil.getTaskSlots(conf)
- conf.set(NUM_TASK_SLOTS_PER_EXECUTOR.key, taskSlots.toString)
-
- val onHeapSize: Long = conf.getSizeAsBytes(SPARK_ONHEAP_SIZE_KEY, 1024 * 1024 * 1024)
-
- // If dynamic off-heap sizing is enabled, the off-heap size is calculated based on the on-heap
- // size. Otherwise, the off-heap size is set to the value specified by the user (if any).
- // Note that this means that we will IGNORE the off-heap size specified by the user if the
- // dynamic off-heap feature is enabled.
- val offHeapSize: Long =
- if (
- conf.getBoolean(
- DYNAMIC_OFFHEAP_SIZING_ENABLED.key,
- DYNAMIC_OFFHEAP_SIZING_ENABLED.defaultValue.get)
- ) {
- // Since when dynamic off-heap sizing is enabled, we commingle on-heap
- // and off-heap memory, we set the off-heap size to the usable on-heap size. We will
- // size it with a memory fraction, which can be aggressively set, but the default
- // is using the same way that Spark sizes on-heap memory:
- //
- // spark.gluten.memory.dynamic.offHeap.sizing.memory.fraction *
- // (spark.executor.memory - 300MB).
- //
- // We will be careful to use the same configuration settings as Spark to ensure
- // that we are sizing the off-heap memory in the same way as Spark sizes on-heap memory.
- // The 300MB value, unfortunately, is hard-coded in Spark code.
- ((onHeapSize - (300 * 1024 * 1024)) *
- conf.getDouble(DYNAMIC_OFFHEAP_SIZING_MEMORY_FRACTION.key, 0.6d)).toLong
- } else {
- // Optimistic off-heap sizes, assuming all storage memory can be borrowed into execution
- // memory pool, regardless of Spark option spark.memory.storageFraction.
- conf.getSizeAsBytes(SPARK_OFFHEAP_SIZE_KEY, 0L)
- }
+ // Get the off-heap size set by user.
+ val offHeapSize = conf.getSizeAsBytes(SPARK_OFFHEAP_SIZE_KEY)
+ // Set off-heap size in bytes.
conf.set(COLUMNAR_OFFHEAP_SIZE_IN_BYTES.key, offHeapSize.toString)
- conf.set(SPARK_OFFHEAP_SIZE_KEY, offHeapSize.toString)
+ // Set off-heap size in bytes per task.
+ val taskSlots = SparkResourceUtil.getTaskSlots(conf)
+ conf.set(NUM_TASK_SLOTS_PER_EXECUTOR.key, taskSlots.toString)
val offHeapPerTask = offHeapSize / taskSlots
conf.set(COLUMNAR_TASK_OFFHEAP_SIZE_IN_BYTES.key, offHeapPerTask.toString)
- // If we are using dynamic off-heap sizing, we should also enable off-heap memory
- // officially.
- if (
- conf.getBoolean(
- DYNAMIC_OFFHEAP_SIZING_ENABLED.key,
- DYNAMIC_OFFHEAP_SIZING_ENABLED.defaultValue.get)
- ) {
- conf.set(SPARK_OFFHEAP_ENABLED, "true")
-
- // We already sized the off-heap per task in a conservative manner, so we can just
- // use it.
- conf.set(COLUMNAR_CONSERVATIVE_TASK_OFFHEAP_SIZE_IN_BYTES.key, offHeapPerTask.toString)
- } else {
- // Let's make sure this is set to false explicitly if it is not on as it
- // is looked up when throwing OOF exceptions.
- conf.set(
- DYNAMIC_OFFHEAP_SIZING_ENABLED.key,
- DYNAMIC_OFFHEAP_SIZING_ENABLED.defaultValueString)
-
- // Pessimistic off-heap sizes, with the assumption that all non-borrowable storage memory
- // determined by spark.memory.storageFraction was used.
- val fraction = 1.0d - conf.getDouble("spark.memory.storageFraction", 0.5d)
- val conservativeOffHeapPerTask = (offHeapSize * fraction).toLong / taskSlots
- conf.set(
- COLUMNAR_CONSERVATIVE_TASK_OFFHEAP_SIZE_IN_BYTES.key,
- conservativeOffHeapPerTask.toString)
- }
+ // Pessimistic off-heap sizes, with the assumption that all non-borrowable storage memory
+ // determined by spark.memory.storageFraction was used.
+ val fraction = 1.0d - conf.getDouble("spark.memory.storageFraction", 0.5d)
+ val conservativeOffHeapPerTask = (offHeapSize * fraction).toLong / taskSlots
Review Comment:
We don't have to deal with off-heap size in the future anymore since we no longer rely on off-heap memory manager when the feature is on.
We are just recovering the old code here before off-heap sizing was landed, so it's totally safe: https://github.com/apache/incubator-gluten/blame/022c208564dbb6dc59b7d64fbc4ad002c166e012/gluten-core/src/main/scala/org/apache/gluten/GlutenPlugin.scala
--
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] Move pre-configuration code of dynamic off-heap sizing to its own place, and minor refactors against the feature [incubator-gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9336:
URL: https://github.com/apache/incubator-gluten/pull/9336#issuecomment-2806409658
<!--
Licensed to the Apache Software Foundation (ASF) under one
or more contributor license agreements. See the NOTICE file
distributed with this work for additional information
regarding copyright ownership. The ASF licenses this file
to you under the Apache License, Version 2.0 (the
"License"); you may not use this file except in compliance
with the License. You may obtain a copy of the License at
http://www.apache.org/licenses/LICEGJ-2.0
Unless required by applicable law or agreed to in writing,
software distributed under the License is distributed on an
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express or implied. See the License for the
specific language governing permissions and limitations
under the License.
-->
Thanks for opening a pull request!
Could you open an issue for this pull request on Github Issues?
https://github.com/apache/incubator-gluten/issues
Then could you also rename ***commit message*** and ***pull request title*** in the following format?
[GLUTEN-${ISSUES_ID}][COMPONENT]feat/fix: ${detailed message}
See also:
* [Other pull requests](https://github.com/apache/incubator-gluten/pulls/)
--
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