You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2021/12/09 09:31:15 UTC
[GitHub] [spark] WangGuangxin opened a new pull request #34846: [SPARK-37593] Optimize HeapMemoryAllocator to avoid memory waste in humongous allocation when using G1GC
WangGuangxin opened a new pull request #34846:
URL: https://github.com/apache/spark/pull/34846
<!--
Thanks for sending a pull request! Here are some tips for you:
1. If this is your first time, please read our contributor guidelines: https://spark.apache.org/contributing.html
2. Ensure you have added or run the appropriate tests for your PR: https://spark.apache.org/developer-tools.html
3. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP][SPARK-XXXX] Your PR title ...'.
4. Be sure to keep the PR description updated to reflect all changes.
5. Please write your PR title to summarize what this PR proposes.
6. If possible, provide a concise example to reproduce the issue for a faster review.
7. If you want to add a new configuration, please read the guideline first for naming configurations in
'core/src/main/scala/org/apache/spark/internal/config/ConfigEntry.scala'.
8. If you want to add or modify an error type or message, please read the guideline first in
'core/src/main/resources/error/README.md'.
-->
### What changes were proposed in this pull request?
As we may know, a phenomenon called `humongous allocations` exists in G1GC when allocations that are larger than 50% of the region size.
Spark's tungsten memory model usually tries to allocate memory by one `page` each time and allocated by `long[pageSizeBytes/8]` in `HeapMemoryAllocator.allocate`.
Remember that java long array needs extra object header (usually 16 bytes in 64bit system), so the really bytes allocated is `pageSize+16`.
Assume that the `G1HeapRegionSize` is 4M and `pageSizeBytes` is 4M as well. Since every time we need to allocate 4M+16byte memory, so two regions are used with one region only occupies 16byte. Then there are about **50%** memory waste.
It can happenes under different combinations of G1HeapRegionSize (varies from 1M to 32M) and pageSizeBytes (varies from 1M to 64M).
This PR try to optimize the pageSize to avoid memory waste.
This case exists not only in `MemoryManagement`, but also in other places such as `TorrentBroadcast.blockSize`. I would like to submit a followup PR if this modification is reasonable.
### Why are the changes needed?
To avoid memory waste in G1 GC
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
Existing UT
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34846:
URL: https://github.com/apache/spark/pull/34846#issuecomment-994973824
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50710/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] kiszk commented on a change in pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste in humongous allocation when using G1GC
Posted by GitBox <gi...@apache.org>.
kiszk commented on a change in pull request #34846:
URL: https://github.com/apache/spark/pull/34846#discussion_r767179124
##########
File path: core/src/main/scala/org/apache/spark/util/Utils.scala
##########
@@ -3204,6 +3205,33 @@ private[spark] object Utils extends Logging {
}
files.toSeq
}
+
+ private def isG1GarbageCollector: Boolean = {
+ ManagementFactory.getGarbageCollectorMXBeans
+ .asScala
+ .exists(g =>
+ g.getName.equalsIgnoreCase("G1 Young Generation") ||
+ g.getName.equalsIgnoreCase("G1 Old Generation"))
+ }
+
+ /**
+ * Return true if current GC algorithm is G1 GC and the requested sizeInBytes
+ * are larger than 50% of the region size in G1.
+ */
+ def isHumongousAllocation(sizeInBytes: Long): Boolean = {
+ if (isG1GarbageCollector) {
+ try {
+ val heapRegionSize = ManagementFactory
Review comment:
Can we get this value only once by make it `def ...` as above? I think this value is defined at JVM launch time.
So, we can reduce runtime overhead to access MxBean.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] WangGuangxin commented on a change in pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste in humongous allocation when using G1GC
Posted by GitBox <gi...@apache.org>.
WangGuangxin commented on a change in pull request #34846:
URL: https://github.com/apache/spark/pull/34846#discussion_r769495903
##########
File path: core/src/main/scala/org/apache/spark/util/Utils.scala
##########
@@ -3204,6 +3204,14 @@ private[spark] object Utils extends Logging {
}
files.toSeq
}
+
+ val isG1GarbageCollector: Boolean = {
+ ManagementFactory.getGarbageCollectorMXBeans
Review comment:
emm, seems your solution is more elegant. I'll update it
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34846:
URL: https://github.com/apache/spark/pull/34846#issuecomment-995048554
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/146236/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34846:
URL: https://github.com/apache/spark/pull/34846#issuecomment-995001194
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/50710/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste in humongous allocation when using G1GC
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #34846:
URL: https://github.com/apache/spark/pull/34846#discussion_r765759372
##########
File path: core/src/main/scala/org/apache/spark/memory/MemoryManager.scala
##########
@@ -255,7 +256,15 @@ private[spark] abstract class MemoryManager(
}
val size = ByteArrayMethods.nextPowerOf2(maxTungstenMemory / cores / safetyFactor)
val default = math.min(maxPageSize, math.max(minPageSize, size))
- conf.get(BUFFER_PAGESIZE).getOrElse(default)
+ val sizeAsBytes = conf.get(BUFFER_PAGESIZE).getOrElse(default)
+ if (conf.get(MEMORY_ONHEAP_PAGESIZE_OPTIMIZE_ENABLED) &&
+ tungstenMemoryMode == MemoryMode.ON_HEAP &&
+ sizeAsBytes % (1024 * 1024) == 0 &&
+ Utils.isHumongousAllocation(sizeAsBytes)) {
+ sizeAsBytes - Platform.LONG_ARRAY_OFFSET
Review comment:
what if we always pick this as the page size?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34846:
URL: https://github.com/apache/spark/pull/34846#issuecomment-996500104
**[Test build #146324 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/146324/testReport)** for PR 34846 at commit [`d166097`](https://github.com/apache/spark/commit/d166097a53630d56b5c7e5d7b97840f3fd5eb230).
* This patch **fails to build**.
* This patch merges cleanly.
* This patch adds no public classes.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34846:
URL: https://github.com/apache/spark/pull/34846#issuecomment-995767991
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/50753/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #34846:
URL: https://github.com/apache/spark/pull/34846#issuecomment-1039438672
cc @viirya , @sunchao , @cloud-fan , @kiszk , @attilapiros, @HyukjinKwon again
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] srowen commented on a change in pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
srowen commented on a change in pull request #34846:
URL: https://github.com/apache/spark/pull/34846#discussion_r769744747
##########
File path: core/src/main/scala/org/apache/spark/memory/MemoryManager.scala
##########
@@ -244,17 +245,26 @@ private[spark] abstract class MemoryManager(
* and then divide it by a factor of safety.
*/
val pageSizeBytes: Long = {
- val minPageSize = 1L * 1024 * 1024 // 1MB
- val maxPageSize = 64L * minPageSize // 64MB
- val cores = if (numCores > 0) numCores else Runtime.getRuntime.availableProcessors()
- // Because of rounding to next power of 2, we may have safetyFactor as 8 in worst case
- val safetyFactor = 16
- val maxTungstenMemory: Long = tungstenMemoryMode match {
- case MemoryMode.ON_HEAP => onHeapExecutionMemoryPool.poolSize
- case MemoryMode.OFF_HEAP => offHeapExecutionMemoryPool.poolSize
+ val default = Utils.G1HeapRegionSize match {
Review comment:
Would lazy do anything here? it's a local inside what's evaluated for a val. I could see declaring it lazily outside this block instead. Then even being lazy doesn't matter
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] attilapiros commented on pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
attilapiros commented on pull request #34846:
URL: https://github.com/apache/spark/pull/34846#issuecomment-996613111
Thanks @WangGuangxin!
Could you please check from spark-shell along with different -XX:G1HeapRegionSize settings as extraJavaOptions for the driver?
You can access pageSize as:
```
scala> org.apache.spark.SparkEnv.get.memoryManager.pageSizeBytes
res0: Long = 1048576
```
It would be nice to see some result both on jdk8 and jdk11.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34846:
URL: https://github.com/apache/spark/pull/34846#issuecomment-995704986
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50753/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #34846:
URL: https://github.com/apache/spark/pull/34846#discussion_r770763686
##########
File path: core/src/main/scala/org/apache/spark/util/Utils.scala
##########
@@ -3204,6 +3205,22 @@ private[spark] object Utils extends Logging {
}
files.toSeq
}
+
+ /**
+ * Get the value of -XX:G1HeapRegionSize if we are using G1 GC,
+ * otherwise just return None
+ */
+ val G1HeapRegionSize: Option[Long] = {
Review comment:
`G1HeapRegionSize` -> `maybeG1HeapRegionSize`?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] WangGuangxin commented on a change in pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
WangGuangxin commented on a change in pull request #34846:
URL: https://github.com/apache/spark/pull/34846#discussion_r805219114
##########
File path: core/src/main/scala/org/apache/spark/util/Utils.scala
##########
@@ -3232,6 +3232,24 @@ private[spark] object Utils extends Logging {
case _ => math.max(sortedSize(len / 2), 1)
}
}
+
+ /**
+ * Return whether we are using G1GC or not
+ */
+ val isG1GC: Boolean = {
Review comment:
ok, updated
##########
File path: core/src/main/scala/org/apache/spark/memory/MemoryManager.scala
##########
@@ -254,10 +259,16 @@ private[spark] abstract class MemoryManager(
case MemoryMode.OFF_HEAP => offHeapExecutionMemoryPool.poolSize
}
val size = ByteArrayMethods.nextPowerOf2(maxTungstenMemory / cores / safetyFactor)
- val default = math.min(maxPageSize, math.max(minPageSize, size))
- conf.get(BUFFER_PAGESIZE).getOrElse(default)
+ val choosedPageSize = math.min(maxPageSize, math.max(minPageSize, size))
Review comment:
done
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun closed pull request #34846: [SPARK-37593][CORE] Reduce default page size by `LONG_ARRAY_OFFSET` if `G1GC` and `ON_HEAP` are used
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun closed pull request #34846:
URL: https://github.com/apache/spark/pull/34846
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] kiszk commented on a change in pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste in humongous allocation when using G1GC
Posted by GitBox <gi...@apache.org>.
kiszk commented on a change in pull request #34846:
URL: https://github.com/apache/spark/pull/34846#discussion_r765659849
##########
File path: core/src/main/scala/org/apache/spark/util/Utils.scala
##########
@@ -3204,6 +3205,27 @@ private[spark] object Utils extends Logging {
}
files.toSeq
}
+
+ private def isG1GarbageCollector: Boolean = {
+ val gcs = ManagementFactory.getGarbageCollectorMXBeans.asScala
+ gcs.exists(_.getName.equalsIgnoreCase("G1 Young Generation")) ||
+ gcs.exists(_.getName.equalsIgnoreCase("G1 Old Generation"))
+ }
+
+ /**
+ * Return true if current GC algorithm is G1 GC and the requested sizeInBytes
+ * are larger than 50% of the region size in G1.
+ */
+ def isHumongousAllocation(sizeInBytes: Long): Boolean = {
+ if (isG1GarbageCollector) {
+ val heapRegionSize = ManagementFactory
+ .getPlatformMXBean(classOf[HotSpotDiagnosticMXBean])
+ .getVMOption("G1HeapRegionSize").getValue.toLong
Review comment:
I am afraid that this line may throw an exception in the future due to specification changes.
How about wrapping this by `try ... catch`?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #34846:
URL: https://github.com/apache/spark/pull/34846#issuecomment-1002410066
Thank you.
For the customers who have many jobs, the previous values of `memoryManager.pageSizeBytes` are a kind of magic number per job because it depends on `cores` and `memory`. So, it's hard to recover their jobs one by one. I thought something like the following which disables the new logic completely for them.
```scala
case Some(heapRegionSize) if isEnabled() && tungstenMemoryMode == MemoryMode.ON_HEAP =>
```
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on a change in pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #34846:
URL: https://github.com/apache/spark/pull/34846#discussion_r806420798
##########
File path: core/src/main/scala/org/apache/spark/memory/MemoryManager.scala
##########
@@ -267,4 +281,22 @@ private[spark] abstract class MemoryManager(
case MemoryMode.OFF_HEAP => MemoryAllocator.UNSAFE
}
}
+
+ /**
+ * Return whether we are using G1GC or not
+ */
+ private[memory] final val isG1GC: Boolean = {
Review comment:
```suggestion
private final val isG1GC: Boolean = {
```
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34846:
URL: https://github.com/apache/spark/pull/34846#issuecomment-996498374
**[Test build #146324 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/146324/testReport)** for PR 34846 at commit [`d166097`](https://github.com/apache/spark/commit/d166097a53630d56b5c7e5d7b97840f3fd5eb230).
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste in humongous allocation when using G1GC
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #34846:
URL: https://github.com/apache/spark/pull/34846#discussion_r767223431
##########
File path: core/src/main/scala/org/apache/spark/memory/MemoryManager.scala
##########
@@ -255,7 +256,14 @@ private[spark] abstract class MemoryManager(
}
val size = ByteArrayMethods.nextPowerOf2(maxTungstenMemory / cores / safetyFactor)
val default = math.min(maxPageSize, math.max(minPageSize, size))
- conf.get(BUFFER_PAGESIZE).getOrElse(default)
+ val sizeAsBytes = conf.get(BUFFER_PAGESIZE).getOrElse(default)
+ if (Utils.isG1GarbageCollector &&
+ tungstenMemoryMode == MemoryMode.ON_HEAP &&
+ sizeAsBytes % (1024 * 1024) == 0) {
+ sizeAsBytes - Platform.LONG_ARRAY_OFFSET
Review comment:
Could you update the function description with this new addition?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] attilapiros commented on pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
attilapiros commented on pull request #34846:
URL: https://github.com/apache/spark/pull/34846#issuecomment-994875985
ok to test
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34846:
URL: https://github.com/apache/spark/pull/34846#issuecomment-995042265
**[Test build #146236 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/146236/testReport)** for PR 34846 at commit [`e9eff1a`](https://github.com/apache/spark/commit/e9eff1af75f8c6bb151d4fa155a469ac3a1b6cc2).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #34846:
URL: https://github.com/apache/spark/pull/34846#issuecomment-994899066
**[Test build #146236 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/146236/testReport)** for PR 34846 at commit [`e9eff1a`](https://github.com/apache/spark/commit/e9eff1af75f8c6bb151d4fa155a469ac3a1b6cc2).
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34846:
URL: https://github.com/apache/spark/pull/34846#issuecomment-995609692
**[Test build #146277 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/146277/testReport)** for PR 34846 at commit [`e43878c`](https://github.com/apache/spark/commit/e43878c044e7d2b3876d9a158ece57f0e8d45197).
* This patch **fails to build**.
* This patch merges cleanly.
* This patch adds no public classes.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34846:
URL: https://github.com/apache/spark/pull/34846#issuecomment-995767991
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/50753/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] attilapiros commented on a change in pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
attilapiros commented on a change in pull request #34846:
URL: https://github.com/apache/spark/pull/34846#discussion_r770903547
##########
File path: core/src/main/scala/org/apache/spark/util/Utils.scala
##########
@@ -3204,6 +3205,22 @@ private[spark] object Utils extends Logging {
}
files.toSeq
}
+
+ /**
+ * Get the value of -XX:G1HeapRegionSize if we are using G1 GC,
+ * otherwise just return None
+ */
+ val G1HeapRegionSize: Option[Long] = {
+ Try {
+ val diagnostic = ManagementFactoryHelper.getDiagnosticMXBean()
Review comment:
Great findings! We need to test this on other JVM versions too.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] viirya commented on a change in pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #34846:
URL: https://github.com/apache/spark/pull/34846#discussion_r771904457
##########
File path: core/src/main/scala/org/apache/spark/memory/MemoryManager.scala
##########
@@ -242,22 +243,30 @@ private[spark] abstract class MemoryManager(
* If user didn't explicitly set "spark.buffer.pageSize", we figure out the default value
* by looking at the number of cores available to the process, and the total amount of memory,
* and then divide it by a factor of safety.
+ *
+ * SPARK-37593 If we are using G1GC, it's better to take the LONG_ARRAY_OFFSET
+ * into consideration so that the requested memory size is power of 2
+ * and can be divided by G1 heap region size to reduce memory waste within one G1 region.
*/
- val pageSizeBytes: Long = {
- val minPageSize = 1L * 1024 * 1024 // 1MB
- val maxPageSize = 64L * minPageSize // 64MB
- val cores = if (numCores > 0) numCores else Runtime.getRuntime.availableProcessors()
- // Because of rounding to next power of 2, we may have safetyFactor as 8 in worst case
- val safetyFactor = 16
- val maxTungstenMemory: Long = tungstenMemoryMode match {
- case MemoryMode.ON_HEAP => onHeapExecutionMemoryPool.poolSize
- case MemoryMode.OFF_HEAP => offHeapExecutionMemoryPool.poolSize
- }
- val size = ByteArrayMethods.nextPowerOf2(maxTungstenMemory / cores / safetyFactor)
- val default = math.min(maxPageSize, math.max(minPageSize, size))
- conf.get(BUFFER_PAGESIZE).getOrElse(default)
+ private lazy val defaultPageSizeBytes = Utils.maybeG1HeapRegionSize match {
+ case Some(heapRegionSize) if tungstenMemoryMode == MemoryMode.ON_HEAP =>
+ heapRegionSize - Platform.LONG_ARRAY_OFFSET
+ case _ =>
Review comment:
Hmm, so as this is now limited by `G1HeapRegionSize`, so the maximum `pageSizeBytes` will be reduced to `32MB - Platform.LONG_ARRAY_OFFSET` after this change? What does it possibly affect users?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] srowen commented on pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
srowen commented on pull request #34846:
URL: https://github.com/apache/spark/pull/34846#issuecomment-1004859769
OK, I think my question at the moment is, can users not recover the original behavior by setting spark.buffer.pageSize? would we need a second config?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #34846:
URL: https://github.com/apache/spark/pull/34846#issuecomment-1039438672
cc @viirya , @sunchao , @cloud-fan , @kiszk , @attilapiros, @HyukjinKwon again
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste in humongous allocation when using G1GC
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #34846:
URL: https://github.com/apache/spark/pull/34846#discussion_r767223314
##########
File path: core/src/main/scala/org/apache/spark/memory/MemoryManager.scala
##########
@@ -255,7 +256,14 @@ private[spark] abstract class MemoryManager(
}
val size = ByteArrayMethods.nextPowerOf2(maxTungstenMemory / cores / safetyFactor)
val default = math.min(maxPageSize, math.max(minPageSize, size))
- conf.get(BUFFER_PAGESIZE).getOrElse(default)
+ val sizeAsBytes = conf.get(BUFFER_PAGESIZE).getOrElse(default)
+ if (Utils.isG1GarbageCollector &&
+ tungstenMemoryMode == MemoryMode.ON_HEAP &&
Review comment:
nit. Indentation. We need two more spaces.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste in humongous allocation when using G1GC
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #34846:
URL: https://github.com/apache/spark/pull/34846#discussion_r767956731
##########
File path: core/src/main/scala/org/apache/spark/memory/MemoryManager.scala
##########
@@ -255,7 +256,14 @@ private[spark] abstract class MemoryManager(
}
val size = ByteArrayMethods.nextPowerOf2(maxTungstenMemory / cores / safetyFactor)
val default = math.min(maxPageSize, math.max(minPageSize, size))
- conf.get(BUFFER_PAGESIZE).getOrElse(default)
+ val sizeAsBytes = conf.get(BUFFER_PAGESIZE).getOrElse(default)
+ if (Utils.isG1GarbageCollector &&
+ tungstenMemoryMode == MemoryMode.ON_HEAP &&
Review comment:
Yes.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste in humongous allocation when using G1GC
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #34846:
URL: https://github.com/apache/spark/pull/34846#discussion_r767223314
##########
File path: core/src/main/scala/org/apache/spark/memory/MemoryManager.scala
##########
@@ -255,7 +256,14 @@ private[spark] abstract class MemoryManager(
}
val size = ByteArrayMethods.nextPowerOf2(maxTungstenMemory / cores / safetyFactor)
val default = math.min(maxPageSize, math.max(minPageSize, size))
- conf.get(BUFFER_PAGESIZE).getOrElse(default)
+ val sizeAsBytes = conf.get(BUFFER_PAGESIZE).getOrElse(default)
+ if (Utils.isG1GarbageCollector &&
+ tungstenMemoryMode == MemoryMode.ON_HEAP &&
Review comment:
nit. Indentation. We need two more spaces.
##########
File path: core/src/main/scala/org/apache/spark/memory/MemoryManager.scala
##########
@@ -255,7 +256,14 @@ private[spark] abstract class MemoryManager(
}
val size = ByteArrayMethods.nextPowerOf2(maxTungstenMemory / cores / safetyFactor)
val default = math.min(maxPageSize, math.max(minPageSize, size))
- conf.get(BUFFER_PAGESIZE).getOrElse(default)
+ val sizeAsBytes = conf.get(BUFFER_PAGESIZE).getOrElse(default)
+ if (Utils.isG1GarbageCollector &&
+ tungstenMemoryMode == MemoryMode.ON_HEAP &&
+ sizeAsBytes % (1024 * 1024) == 0) {
+ sizeAsBytes - Platform.LONG_ARRAY_OFFSET
Review comment:
Could you update the function description with this new addition?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] WangGuangxin commented on pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste in humongous allocation when using G1GC
Posted by GitBox <gi...@apache.org>.
WangGuangxin commented on pull request #34846:
URL: https://github.com/apache/spark/pull/34846#issuecomment-992585451
> @WangGuangxin . Do you think you can add one specific example about the case to the PR description?
We can demo it using following piece of code.
```
public static void bufferSizeTest(boolean optimize) {
long totalAllocatedSize = 0L;
int blockSize = 1024 * 1024 * 4; // 4m
if (optimize) {
blockSize -= 16;
}
List<long[]> buffers = new ArrayList<>();
while (true) {
long[] arr = new long[blockSize/8];
buffers.add(arr);
totalAllocatedSize += blockSize;
System.out.println("Total allocated size: " + totalAllocatedSize);
}
}
```
Run it using following jvm params
```
java -Xmx100m -XX:+UseG1GC -XX:G1HeapRegionSize=4m -XX:-UseGCOverheadLimit -verbose:gc -XX:+UnlockDiagnosticVMOptions -XX:+G1SummarizeConcMark -Xss4m -XX:+ExitOnOutOfMemoryError -XX:ParallelGCThreads=4 -XX:ConcGCThreads=4
```
with optimized = false
```
Total allocated size: 46137344
[GC pause (G1 Humongous Allocation) (young) 44M->44M(100M), 0.0007091 secs]
[GC pause (G1 Evacuation Pause) (young) (initial-mark)-- 48M->48M(100M), 0.0021528 secs]
[GC concurrent-root-region-scan-start]
[GC concurrent-root-region-scan-end, 0.0000021 secs]
[GC concurrent-mark-start]
[GC pause (G1 Evacuation Pause) (young) 48M->48M(100M), 0.0011289 secs]
[Full GC (Allocation Failure) 48M->48M(100M), 0.0017284 secs]
[Full GC (Allocation Failure) 48M->48M(100M), 0.0013437 secs]
Terminating due to java.lang.OutOfMemoryError: Java heap space
```
with optimzied = true
```
Total allocated size: 96468624
[GC pause (G1 Humongous Allocation) (young)-- 92M->92M(100M), 0.0024416 secs]
[Full GC (Allocation Failure) 92M->92M(100M), 0.0019883 secs]
[GC pause (G1 Evacuation Pause) (young) (initial-mark) 96M->96M(100M), 0.0004282 secs]
[GC concurrent-root-region-scan-start]
[GC concurrent-root-region-scan-end, 0.0000040 secs]
[GC concurrent-mark-start]
[GC pause (G1 Evacuation Pause) (young) 96M->96M(100M), 0.0003269 secs]
[Full GC (Allocation Failure) 96M->96M(100M), 0.0012409 secs]
[Full GC (Allocation Failure) 96M->96M(100M), 0.0012607 secs]
Terminating due to java.lang.OutOfMemoryError: Java heap space
```
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] viirya commented on a change in pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #34846:
URL: https://github.com/apache/spark/pull/34846#discussion_r771904842
##########
File path: core/src/main/scala/org/apache/spark/memory/MemoryManager.scala
##########
@@ -242,22 +243,30 @@ private[spark] abstract class MemoryManager(
* If user didn't explicitly set "spark.buffer.pageSize", we figure out the default value
* by looking at the number of cores available to the process, and the total amount of memory,
* and then divide it by a factor of safety.
+ *
+ * SPARK-37593 If we are using G1GC, it's better to take the LONG_ARRAY_OFFSET
+ * into consideration so that the requested memory size is power of 2
+ * and can be divided by G1 heap region size to reduce memory waste within one G1 region.
*/
- val pageSizeBytes: Long = {
- val minPageSize = 1L * 1024 * 1024 // 1MB
- val maxPageSize = 64L * minPageSize // 64MB
- val cores = if (numCores > 0) numCores else Runtime.getRuntime.availableProcessors()
- // Because of rounding to next power of 2, we may have safetyFactor as 8 in worst case
- val safetyFactor = 16
- val maxTungstenMemory: Long = tungstenMemoryMode match {
- case MemoryMode.ON_HEAP => onHeapExecutionMemoryPool.poolSize
- case MemoryMode.OFF_HEAP => offHeapExecutionMemoryPool.poolSize
- }
- val size = ByteArrayMethods.nextPowerOf2(maxTungstenMemory / cores / safetyFactor)
- val default = math.min(maxPageSize, math.max(minPageSize, size))
- conf.get(BUFFER_PAGESIZE).getOrElse(default)
+ private lazy val defaultPageSizeBytes = Utils.maybeG1HeapRegionSize match {
+ case Some(heapRegionSize) if tungstenMemoryMode == MemoryMode.ON_HEAP =>
+ heapRegionSize - Platform.LONG_ARRAY_OFFSET
+ case _ =>
Review comment:
If the goal is to reduce memory waste, cannot we also keep as close as possible to original maximum page size (64MB)? For example, `64MB - Platform.LONG_ARRAY_OFFSET`?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] WangGuangxin commented on a change in pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste in humongous allocation when using G1GC
Posted by GitBox <gi...@apache.org>.
WangGuangxin commented on a change in pull request #34846:
URL: https://github.com/apache/spark/pull/34846#discussion_r767166793
##########
File path: core/src/main/scala/org/apache/spark/memory/MemoryManager.scala
##########
@@ -255,7 +256,15 @@ private[spark] abstract class MemoryManager(
}
val size = ByteArrayMethods.nextPowerOf2(maxTungstenMemory / cores / safetyFactor)
val default = math.min(maxPageSize, math.max(minPageSize, size))
- conf.get(BUFFER_PAGESIZE).getOrElse(default)
+ val sizeAsBytes = conf.get(BUFFER_PAGESIZE).getOrElse(default)
+ if (conf.get(MEMORY_ONHEAP_PAGESIZE_OPTIMIZE_ENABLED) &&
+ tungstenMemoryMode == MemoryMode.ON_HEAP &&
+ sizeAsBytes % (1024 * 1024) == 0 &&
+ Utils.isHumongousAllocation(sizeAsBytes)) {
+ sizeAsBytes - Platform.LONG_ARRAY_OFFSET
Review comment:
you mean even if it is not homongous allocation?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34846:
URL: https://github.com/apache/spark/pull/34846#issuecomment-994899066
**[Test build #146236 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/146236/testReport)** for PR 34846 at commit [`e9eff1a`](https://github.com/apache/spark/commit/e9eff1af75f8c6bb151d4fa155a469ac3a1b6cc2).
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] WangGuangxin commented on a change in pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
WangGuangxin commented on a change in pull request #34846:
URL: https://github.com/apache/spark/pull/34846#discussion_r771237521
##########
File path: core/src/main/scala/org/apache/spark/util/Utils.scala
##########
@@ -3204,6 +3205,22 @@ private[spark] object Utils extends Logging {
}
files.toSeq
}
+
+ /**
+ * Get the value of -XX:G1HeapRegionSize if we are using G1 GC,
+ * otherwise just return None
+ */
+ val G1HeapRegionSize: Option[Long] = {
+ Try {
+ val diagnostic = ManagementFactoryHelper.getDiagnosticMXBean()
Review comment:
@dongjoon-hyun Thanks for your findings. I've updated and checked against JDK8, 11, and 17. also cc @attilapiros
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34846:
URL: https://github.com/apache/spark/pull/34846#issuecomment-995636946
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/146277/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #34846:
URL: https://github.com/apache/spark/pull/34846#discussion_r770767634
##########
File path: core/src/main/scala/org/apache/spark/util/Utils.scala
##########
@@ -3204,6 +3205,22 @@ private[spark] object Utils extends Logging {
}
files.toSeq
}
+
+ /**
+ * Get the value of -XX:G1HeapRegionSize if we are using G1 GC,
+ * otherwise just return None
+ */
+ val G1HeapRegionSize: Option[Long] = {
+ Try {
+ val diagnostic = ManagementFactoryHelper.getDiagnosticMXBean()
Review comment:
@WangGuangxin This is not Java11 and Java17 API.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] attilapiros commented on a change in pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
attilapiros commented on a change in pull request #34846:
URL: https://github.com/apache/spark/pull/34846#discussion_r769585232
##########
File path: core/src/main/scala/org/apache/spark/util/Utils.scala
##########
@@ -3204,6 +3204,14 @@ private[spark] object Utils extends Logging {
}
files.toSeq
}
+
+ val isG1GarbageCollector: Boolean = {
+ ManagementFactory.getGarbageCollectorMXBeans
Review comment:
Thanks!
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] attilapiros commented on a change in pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
attilapiros commented on a change in pull request #34846:
URL: https://github.com/apache/spark/pull/34846#discussion_r769751622
##########
File path: core/src/main/scala/org/apache/spark/memory/MemoryManager.scala
##########
@@ -244,17 +245,26 @@ private[spark] abstract class MemoryManager(
* and then divide it by a factor of safety.
*/
val pageSizeBytes: Long = {
- val minPageSize = 1L * 1024 * 1024 // 1MB
- val maxPageSize = 64L * minPageSize // 64MB
- val cores = if (numCores > 0) numCores else Runtime.getRuntime.availableProcessors()
- // Because of rounding to next power of 2, we may have safetyFactor as 8 in worst case
- val safetyFactor = 16
- val maxTungstenMemory: Long = tungstenMemoryMode match {
- case MemoryMode.ON_HEAP => onHeapExecutionMemoryPool.poolSize
- case MemoryMode.OFF_HEAP => offHeapExecutionMemoryPool.poolSize
+ val default = Utils.G1HeapRegionSize match {
Review comment:
It would do:
```
scala> lazy val foo = {
| println("Initialized")
| 1
| }
foo: Int = <lazy>
scala> "test"
res0: String = test
scala> None.getOrElse(foo)
Initialized
res1: Int = 1
```
But have it outside even better!
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34846:
URL: https://github.com/apache/spark/pull/34846#issuecomment-995779682
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/146279/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #34846:
URL: https://github.com/apache/spark/pull/34846#issuecomment-1002362824
For `I think that's what this PR does?`, YES! I pointed out that too like the following,
> Given that we can set spark.buffer.pageSize manually to mitigate the mentioned issue, this PR's contribution is doing that automatically when the users changes the -Xmx option.
I didn't argue that this causes a performance regression always. What I asked was only the following thing to rescue specific production jobs from this change, @srowen . The following was my literal comment.
> I'd like to suggest to make a configuration to recover to the previous behavior,
Without this configuration, it's hard to restore the production jobs' behavior back to the previous one.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun edited a comment on pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun edited a comment on pull request #34846:
URL: https://github.com/apache/spark/pull/34846#issuecomment-1005149199
> OK, I think my question at the moment is, can users not recover the original behavior by setting spark.buffer.pageSize? would we need a second config?
Technically, yes in my perspective! As I wrote in [the very previous comment](https://github.com/apache/spark/pull/34846#issuecomment-1002410066), how does user know the previous value per job?
When you have 10k jobs in the production, can we persuade the customers to find out those values one by one and set `spark.buffer.pageSize` explicitly for 10k times?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #34846:
URL: https://github.com/apache/spark/pull/34846#issuecomment-1005149199
> OK, I think my question at the moment is, can users not recover the original behavior by setting spark.buffer.pageSize? would we need a second config?
Technically, yes in my perspective! As I wrote in [the very previous comment](https://github.com/apache/spark/pull/34846#issuecomment-1002410066), how does user know the previous value per job?
When you have 10k jobs in the production, can we persuade the customers to do that one by one for 10k times?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] rednaxelafx commented on pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
rednaxelafx commented on pull request #34846:
URL: https://github.com/apache/spark/pull/34846#issuecomment-1041121355
I have a question to those who are more familiar with Spark internals though: are there any places inside of Spark that implicitly depends on the page size being a power-of-two? Do we run any risk that such assumption is not checked at runtime, which can lead to out-of-bounds access in the boundary cases?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] attilapiros commented on a change in pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste in humongous allocation when using G1GC
Posted by GitBox <gi...@apache.org>.
attilapiros commented on a change in pull request #34846:
URL: https://github.com/apache/spark/pull/34846#discussion_r769417875
##########
File path: core/src/main/scala/org/apache/spark/util/Utils.scala
##########
@@ -3204,6 +3204,14 @@ private[spark] object Utils extends Logging {
}
files.toSeq
}
+
+ val isG1GarbageCollector: Boolean = {
+ ManagementFactory.getGarbageCollectorMXBeans
Review comment:
When I looked for how to find out what the garbage collector type is I bumped into this several times:
```java
HotSpotDiagnosticMXBean diagnostic = ManagementFactoryHelper.getDiagnosticMXBean();
VMOption option = diagnostic.getVMOption("UseG1GC");
if (option.getValue().equals("false")) {
...
}
```
For example at the [OpenJDK tests](https://www.programcreek.com/java-api-examples/?code=AdoptOpenJDK%2Fopenjdk-jdk8u%2Fopenjdk-jdk8u-master%2Fhotspot%2Ftest%2Fgc%2Farguments%2FTestG1HeapRegionSize.java
).
Is there any reason why a different solution have been chosen here?
##########
File path: core/src/main/scala/org/apache/spark/memory/MemoryManager.scala
##########
@@ -255,7 +256,17 @@ private[spark] abstract class MemoryManager(
}
val size = ByteArrayMethods.nextPowerOf2(maxTungstenMemory / cores / safetyFactor)
val default = math.min(maxPageSize, math.max(minPageSize, size))
- conf.get(BUFFER_PAGESIZE).getOrElse(default)
+ val sizeAsBytes = conf.get(BUFFER_PAGESIZE).getOrElse(default)
+ // If we are using G1 GC, it's better to take the LONG_ARRAY_OFFSET into consideration
+ // so that the requested memory size is power of 2 and can be divided by G1 region size
+ // to reduce memory waste within one G1 region
+ if (Utils.isG1GarbageCollector &&
+ tungstenMemoryMode == MemoryMode.ON_HEAP &&
+ sizeAsBytes % (1024 * 1024) == 0) {
+ sizeAsBytes - Platform.LONG_ARRAY_OFFSET
Review comment:
If I get this right in case of G1GC the best would be if we choose a pageSize where the following holds:
```
G1HeapRegionSize % (pageSize + Platform.LONG_ARRAY_OFFSET) == 0
```
And when When BUFFER_PAGESIZE is not set we are free to choose it as:
```
pageSize = G1HeapRegionSize - Platform.LONG_ARRAY_OFFSET;
```
And with the above code we just try to calculate G1HeapRegionSize with our own way. But what about accessing this value?
Like the same way used in the [OpenJDK tests](https://www.programcreek.com/java-api-examples/?code=AdoptOpenJDK%2Fopenjdk-jdk8u%2Fopenjdk-jdk8u-master%2Fhotspot%2Ftest%2Fgc%2Farguments%2FTestG1HeapRegionSize.java):
```java
HotSpotDiagnosticMXBean diagnostic = ManagementFactoryHelper.getDiagnosticMXBean();
option = diagnostic.getVMOption("G1HeapRegionSize");
```
As I see the OpenJDK test was executed like:
```
run main/othervm -Xmx64m TestG1HeapRegionSize 1048576
```
So `diagnostic.getVMOption("G1HeapRegionSize")` gives back the calculated region size.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34846:
URL: https://github.com/apache/spark/pull/34846#issuecomment-994938170
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50710/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste in humongous allocation when using G1GC
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #34846:
URL: https://github.com/apache/spark/pull/34846#issuecomment-992693894
Also, cc @sunchao , @viirya , @huaxingao , too
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] WangGuangxin commented on a change in pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste in humongous allocation when using G1GC
Posted by GitBox <gi...@apache.org>.
WangGuangxin commented on a change in pull request #34846:
URL: https://github.com/apache/spark/pull/34846#discussion_r769493813
##########
File path: core/src/main/scala/org/apache/spark/memory/MemoryManager.scala
##########
@@ -255,7 +256,17 @@ private[spark] abstract class MemoryManager(
}
val size = ByteArrayMethods.nextPowerOf2(maxTungstenMemory / cores / safetyFactor)
val default = math.min(maxPageSize, math.max(minPageSize, size))
- conf.get(BUFFER_PAGESIZE).getOrElse(default)
+ val sizeAsBytes = conf.get(BUFFER_PAGESIZE).getOrElse(default)
+ // If we are using G1 GC, it's better to take the LONG_ARRAY_OFFSET into consideration
+ // so that the requested memory size is power of 2 and can be divided by G1 region size
+ // to reduce memory waste within one G1 region
+ if (Utils.isG1GarbageCollector &&
+ tungstenMemoryMode == MemoryMode.ON_HEAP &&
+ sizeAsBytes % (1024 * 1024) == 0) {
+ sizeAsBytes - Platform.LONG_ARRAY_OFFSET
Review comment:
Yes, it's better to make sure `G1HeapRegionSize % (pageSize + Platform.LONG_ARRAY_OFFSET) == 0`.
But when BUFFER_PAGESIZE is not set, I'm not quite sure if it's reasonable to choose it as `pageSize = G1HeapRegionSize - Platform.LONG_ARRAY_OFFSET`, which seems a bit different with current logic to get default page size.
By following the current logic to calculte default page size and then minus `Platform.LONG_ARRAY_OFFSET` can also make sure `G1HeapRegionSize % (pageSize + Platform.LONG_ARRAY_OFFSET) == 0`
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34846:
URL: https://github.com/apache/spark/pull/34846#issuecomment-996499527
Kubernetes integration test unable to build dist.
exiting with code: 1
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50798/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34846:
URL: https://github.com/apache/spark/pull/34846#issuecomment-996608856
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/50800/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34846:
URL: https://github.com/apache/spark/pull/34846#issuecomment-996650108
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/146326/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun edited a comment on pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun edited a comment on pull request #34846:
URL: https://github.com/apache/spark/pull/34846#issuecomment-1002336872
The new proposed code focused only on fitting to JVM G1HeapRegionSize and it claims that it will reduce the Spark's pageSize (humongous) to smaller ones. In this case, there is no guarantee that new defaultPageSizeBytes is better than the previous calculated ones (whatever they were in the production) in terms of the executor time (if no GC occurs because there is enough memory in the extreme cases).
```
private lazy val defaultPageSizeBytes = Utils.maybeG1HeapRegionSize match {
case Some(heapRegionSize) if tungstenMemoryMode == MemoryMode.ON_HEAP =>
heapRegionSize - Platform.LONG_ARRAY_OFFSET
```
If the smaller page size always wins, we may want to have a default value, `1MB` (the minimum value of G1HeapRegionSize) - Platform.LONG_ARRAY_OFFSET, for `spark.buffer.pageSize` configuration, instead of this PR.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun edited a comment on pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun edited a comment on pull request #34846:
URL: https://github.com/apache/spark/pull/34846#issuecomment-1002354003
I agreed that this PR makes Spark utilize more memory without waste.
> that you don't think this causes more mem allocation?
Yes, indeed. I also agree with the following, too. My point is `G1HeapRegionSize - Platform.LONG_ARRAY_OFFSET` is not a silver bullet to guarantee **always-win**.
> It seems like it should be a better idea in a lot of cases.
Let me ask you in this way. Do you think `spark.buffer.pageSize = 1MB(G1HeapRegionSize) - Platform.LONG_ARRAY_OFFSET` is always better than the other values in the production?
> What's the perf regression you have in mind?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste in humongous allocation when using G1GC
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #34846:
URL: https://github.com/apache/spark/pull/34846#issuecomment-989823911
cc @rednaxelafx @zsxwing
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #34846:
URL: https://github.com/apache/spark/pull/34846#issuecomment-1041022349
Looks making sense to me.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun edited a comment on pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun edited a comment on pull request #34846:
URL: https://github.com/apache/spark/pull/34846#issuecomment-1002354003
I agreed that this PR makes Spark utilize more memory without waste.
> that you don't think this causes more mem allocation?
Yes, indeed. I also agree with that. My point is `G1HeapRegionSize - Platform.LONG_ARRAY_OFFSET` is not a silver bullet to guarantee **always-win**.
> It seems like it should be a better idea in a lot of cases.
Let me ask you in this way. Do you think `spark.buffer.pageSize = 1MB(G1HeapRegionSize) - Platform.LONG_ARRAY_OFFSET` is always better than the other values in the production?
> What's the perf regression you have in mind?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] srowen commented on pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
srowen commented on pull request #34846:
URL: https://github.com/apache/spark/pull/34846#issuecomment-1002137936
What's the perf regression? Are we likely to have better or worse perf across normal usage with this as the 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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #34846:
URL: https://github.com/apache/spark/pull/34846#issuecomment-1039824452
cc @rednaxelafx too FYI
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on pull request #34846: [SPARK-37593][CORE] Reduce default page size by `LONG_ARRAY_OFFSET` if `G1GC` and `ON_HEAP` are used
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #34846:
URL: https://github.com/apache/spark/pull/34846#issuecomment-1055815965
As the above comments, I agree that there are more combinations like new GC and different alignments. However, this PR is very narrow-downed to G1GC and OnHeap and the adjusted amount is a little. I believe we can merge this and move forward on top of this.
I revise the PR title according to the PR content because we have more chances of optimization.
Merged to master for Apache Spark 3.3.0.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] srowen commented on a change in pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste in humongous allocation when using G1GC
Posted by GitBox <gi...@apache.org>.
srowen commented on a change in pull request #34846:
URL: https://github.com/apache/spark/pull/34846#discussion_r766851255
##########
File path: core/src/main/scala/org/apache/spark/internal/config/package.scala
##########
@@ -2267,4 +2267,12 @@ package object config {
.version("3.3.0")
.intConf
.createWithDefault(5)
+
+ private[spark] val MEMORY_ONHEAP_PAGESIZE_OPTIMIZE_ENABLED =
Review comment:
Do we need a flag for this? when would I not want it?
##########
File path: core/src/main/scala/org/apache/spark/util/Utils.scala
##########
@@ -3204,6 +3205,33 @@ private[spark] object Utils extends Logging {
}
files.toSeq
}
+
+ private def isG1GarbageCollector: Boolean = {
+ val gcs = ManagementFactory.getGarbageCollectorMXBeans.asScala
+ gcs.exists(_.getName.equalsIgnoreCase("G1 Young Generation")) ||
+ gcs.exists(_.getName.equalsIgnoreCase("G1 Old Generation"))
+ }
+
+ /**
+ * Return true if current GC algorithm is G1 GC and the requested sizeInBytes
+ * are larger than 50% of the region size in G1.
+ */
+ def isHumongousAllocation(sizeInBytes: Long): Boolean = {
+ if (isG1GarbageCollector) {
+ try {
+ val heapRegionSize = ManagementFactory
+ .getPlatformMXBean(classOf[HotSpotDiagnosticMXBean])
+ .getVMOption("G1HeapRegionSize").getValue.toLong
+ sizeInBytes > (heapRegionSize / 2)
+ } catch {
+ case t: Throwable =>
+ logWarning("Try to get G1HeapRegionSize failed. ", t)
+ false
Review comment:
Just merge both false lines into one at the end
##########
File path: core/src/main/scala/org/apache/spark/util/Utils.scala
##########
@@ -3204,6 +3205,33 @@ private[spark] object Utils extends Logging {
}
files.toSeq
}
+
+ private def isG1GarbageCollector: Boolean = {
+ val gcs = ManagementFactory.getGarbageCollectorMXBeans.asScala
+ gcs.exists(_.getName.equalsIgnoreCase("G1 Young Generation")) ||
Review comment:
Nit: you could make this one exists() statement
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] WangGuangxin commented on a change in pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
WangGuangxin commented on a change in pull request #34846:
URL: https://github.com/apache/spark/pull/34846#discussion_r769700852
##########
File path: core/src/main/scala/org/apache/spark/memory/MemoryManager.scala
##########
@@ -255,7 +256,17 @@ private[spark] abstract class MemoryManager(
}
val size = ByteArrayMethods.nextPowerOf2(maxTungstenMemory / cores / safetyFactor)
val default = math.min(maxPageSize, math.max(minPageSize, size))
- conf.get(BUFFER_PAGESIZE).getOrElse(default)
+ val sizeAsBytes = conf.get(BUFFER_PAGESIZE).getOrElse(default)
+ // If we are using G1 GC, it's better to take the LONG_ARRAY_OFFSET into consideration
+ // so that the requested memory size is power of 2 and can be divided by G1 region size
+ // to reduce memory waste within one G1 region
+ if (Utils.isG1GarbageCollector &&
+ tungstenMemoryMode == MemoryMode.ON_HEAP &&
+ sizeAsBytes % (1024 * 1024) == 0) {
+ sizeAsBytes - Platform.LONG_ARRAY_OFFSET
Review comment:
I see what you mean, you are right
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34846:
URL: https://github.com/apache/spark/pull/34846#issuecomment-995001194
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/50710/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] WangGuangxin commented on a change in pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste in humongous allocation when using G1GC
Posted by GitBox <gi...@apache.org>.
WangGuangxin commented on a change in pull request #34846:
URL: https://github.com/apache/spark/pull/34846#discussion_r769443684
##########
File path: core/src/main/scala/org/apache/spark/memory/MemoryManager.scala
##########
@@ -255,7 +256,17 @@ private[spark] abstract class MemoryManager(
}
val size = ByteArrayMethods.nextPowerOf2(maxTungstenMemory / cores / safetyFactor)
val default = math.min(maxPageSize, math.max(minPageSize, size))
- conf.get(BUFFER_PAGESIZE).getOrElse(default)
+ val sizeAsBytes = conf.get(BUFFER_PAGESIZE).getOrElse(default)
+ // If we are using G1 GC, it's better to take the LONG_ARRAY_OFFSET into consideration
+ // so that the requested memory size is power of 2 and can be divided by G1 region size
+ // to reduce memory waste within one G1 region
+ if (Utils.isG1GarbageCollector &&
Review comment:
I prefer not to put it in `HeapMemoryAllocator.allocate` because it may break the semantics. When we do `HeapMemoryAllocator.allocate(size)` we expected to get memory with specified size or throw oom, but we internally change it to another value `(size-Platform.LONG_ARRAY_OFFSET)`, which may crash the caller's code or brings confusion
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34846:
URL: https://github.com/apache/spark/pull/34846#issuecomment-995048554
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/146236/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] WangGuangxin commented on a change in pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste in humongous allocation when using G1GC
Posted by GitBox <gi...@apache.org>.
WangGuangxin commented on a change in pull request #34846:
URL: https://github.com/apache/spark/pull/34846#discussion_r767167632
##########
File path: core/src/main/scala/org/apache/spark/util/Utils.scala
##########
@@ -3204,6 +3205,33 @@ private[spark] object Utils extends Logging {
}
files.toSeq
}
+
+ private def isG1GarbageCollector: Boolean = {
Review comment:
ok
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste in humongous allocation when using G1GC
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #34846:
URL: https://github.com/apache/spark/pull/34846#issuecomment-992690791
@WangGuangxin . Thank you. Please include https://github.com/apache/spark/pull/34846#issuecomment-992585451 into the PR description. It will be a permanent commit log.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #34846:
URL: https://github.com/apache/spark/pull/34846#issuecomment-995605235
**[Test build #146277 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/146277/testReport)** for PR 34846 at commit [`e43878c`](https://github.com/apache/spark/commit/e43878c044e7d2b3876d9a158ece57f0e8d45197).
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34846:
URL: https://github.com/apache/spark/pull/34846#issuecomment-995639881
**[Test build #146279 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/146279/testReport)** for PR 34846 at commit [`c2b0cb9`](https://github.com/apache/spark/commit/c2b0cb9b042050a4e284ca2745b59fad8cec30a7).
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34846:
URL: https://github.com/apache/spark/pull/34846#issuecomment-996499546
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/50798/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34846:
URL: https://github.com/apache/spark/pull/34846#issuecomment-996500126
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/146324/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34846:
URL: https://github.com/apache/spark/pull/34846#issuecomment-996530153
**[Test build #146326 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/146326/testReport)** for PR 34846 at commit [`0ff2bd9`](https://github.com/apache/spark/commit/0ff2bd9729df3d8f50e3639a70d4995b5b93cdd8).
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34846:
URL: https://github.com/apache/spark/pull/34846#issuecomment-996650108
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/146326/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #34846:
URL: https://github.com/apache/spark/pull/34846#issuecomment-995639881
**[Test build #146279 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/146279/testReport)** for PR 34846 at commit [`c2b0cb9`](https://github.com/apache/spark/commit/c2b0cb9b042050a4e284ca2745b59fad8cec30a7).
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34846:
URL: https://github.com/apache/spark/pull/34846#issuecomment-995779682
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/146279/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34846:
URL: https://github.com/apache/spark/pull/34846#issuecomment-995636946
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/146277/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #34846:
URL: https://github.com/apache/spark/pull/34846#issuecomment-1035913055
Thank you for updates, @WangGuangxin .
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] srowen commented on a change in pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
srowen commented on a change in pull request #34846:
URL: https://github.com/apache/spark/pull/34846#discussion_r804655594
##########
File path: core/src/main/scala/org/apache/spark/memory/MemoryManager.scala
##########
@@ -254,10 +259,16 @@ private[spark] abstract class MemoryManager(
case MemoryMode.OFF_HEAP => offHeapExecutionMemoryPool.poolSize
}
val size = ByteArrayMethods.nextPowerOf2(maxTungstenMemory / cores / safetyFactor)
- val default = math.min(maxPageSize, math.max(minPageSize, size))
- conf.get(BUFFER_PAGESIZE).getOrElse(default)
+ val choosedPageSize = math.min(maxPageSize, math.max(minPageSize, size))
Review comment:
Total nit, but choosed -> chosen
##########
File path: core/src/main/scala/org/apache/spark/util/Utils.scala
##########
@@ -3232,6 +3232,24 @@ private[spark] object Utils extends Logging {
case _ => math.max(sortedSize(len / 2), 1)
}
}
+
+ /**
+ * Return whether we are using G1GC or not
+ */
+ val isG1GC: Boolean = {
Review comment:
This is OK here, though if it's only used here, maybe leave it in MemoryManager until it has reason to be shared? Utils is quite big already
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] srowen commented on a change in pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste in humongous allocation when using G1GC
Posted by GitBox <gi...@apache.org>.
srowen commented on a change in pull request #34846:
URL: https://github.com/apache/spark/pull/34846#discussion_r767166638
##########
File path: core/src/main/scala/org/apache/spark/memory/MemoryManager.scala
##########
@@ -255,7 +256,15 @@ private[spark] abstract class MemoryManager(
}
val size = ByteArrayMethods.nextPowerOf2(maxTungstenMemory / cores / safetyFactor)
val default = math.min(maxPageSize, math.max(minPageSize, size))
- conf.get(BUFFER_PAGESIZE).getOrElse(default)
+ val sizeAsBytes = conf.get(BUFFER_PAGESIZE).getOrElse(default)
+ if (conf.get(MEMORY_ONHEAP_PAGESIZE_OPTIMIZE_ENABLED) &&
+ tungstenMemoryMode == MemoryMode.ON_HEAP &&
+ sizeAsBytes % (1024 * 1024) == 0 &&
+ Utils.isHumongousAllocation(sizeAsBytes)) {
+ sizeAsBytes - Platform.LONG_ARRAY_OFFSET
Review comment:
I suppose we lose a few bytes in the allocation, and maybe that makes some nice power-of-two data structure not fit, but, I wonder if that's pretty rare and if we can just go with this always indeed
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] WangGuangxin commented on a change in pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste in humongous allocation when using G1GC
Posted by GitBox <gi...@apache.org>.
WangGuangxin commented on a change in pull request #34846:
URL: https://github.com/apache/spark/pull/34846#discussion_r767166389
##########
File path: core/src/main/scala/org/apache/spark/internal/config/package.scala
##########
@@ -2267,4 +2267,12 @@ package object config {
.version("3.3.0")
.intConf
.createWithDefault(5)
+
+ private[spark] val MEMORY_ONHEAP_PAGESIZE_OPTIMIZE_ENABLED =
Review comment:
yeah, it's indeed no need. removed it
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] kiszk commented on a change in pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste in humongous allocation when using G1GC
Posted by GitBox <gi...@apache.org>.
kiszk commented on a change in pull request #34846:
URL: https://github.com/apache/spark/pull/34846#discussion_r767179124
##########
File path: core/src/main/scala/org/apache/spark/util/Utils.scala
##########
@@ -3204,6 +3205,33 @@ private[spark] object Utils extends Logging {
}
files.toSeq
}
+
+ private def isG1GarbageCollector: Boolean = {
+ ManagementFactory.getGarbageCollectorMXBeans
+ .asScala
+ .exists(g =>
+ g.getName.equalsIgnoreCase("G1 Young Generation") ||
+ g.getName.equalsIgnoreCase("G1 Old Generation"))
+ }
+
+ /**
+ * Return true if current GC algorithm is G1 GC and the requested sizeInBytes
+ * are larger than 50% of the region size in G1.
+ */
+ def isHumongousAllocation(sizeInBytes: Long): Boolean = {
+ if (isG1GarbageCollector) {
+ try {
+ val heapRegionSize = ManagementFactory
Review comment:
Can we get this value only once by make it `def ...` as above? I think this value is defined at JVM launch time.
So, we can reduce runtime overhead to access MxBean.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun edited a comment on pull request #34846: [SPARK-37593][CORE] Reduce default page size by `LONG_ARRAY_OFFSET` if `G1GC` and `ON_HEAP` are used
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun edited a comment on pull request #34846:
URL: https://github.com/apache/spark/pull/34846#issuecomment-1055815965
Thank you, @WangGuangxin and all!
As the above comments, I agree that there are more combinations like new GC and different alignments. However, this PR is very narrow-downed to G1GC and OnHeap and the adjusted amount is a little. I believe we can merge this and move forward on top of this.
I revise the PR title according to the PR content because we have more chances of optimization.
Merged to master for Apache Spark 3.3.0.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] srowen commented on a change in pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste in humongous allocation when using G1GC
Posted by GitBox <gi...@apache.org>.
srowen commented on a change in pull request #34846:
URL: https://github.com/apache/spark/pull/34846#discussion_r767166638
##########
File path: core/src/main/scala/org/apache/spark/memory/MemoryManager.scala
##########
@@ -255,7 +256,15 @@ private[spark] abstract class MemoryManager(
}
val size = ByteArrayMethods.nextPowerOf2(maxTungstenMemory / cores / safetyFactor)
val default = math.min(maxPageSize, math.max(minPageSize, size))
- conf.get(BUFFER_PAGESIZE).getOrElse(default)
+ val sizeAsBytes = conf.get(BUFFER_PAGESIZE).getOrElse(default)
+ if (conf.get(MEMORY_ONHEAP_PAGESIZE_OPTIMIZE_ENABLED) &&
+ tungstenMemoryMode == MemoryMode.ON_HEAP &&
+ sizeAsBytes % (1024 * 1024) == 0 &&
+ Utils.isHumongousAllocation(sizeAsBytes)) {
+ sizeAsBytes - Platform.LONG_ARRAY_OFFSET
Review comment:
I suppose we lose a few bytes in the allocation, and maybe that makes some nice power-of-two data structure not fit, but, I wonder if that's pretty rare and if we can just go with this always indeed
##########
File path: core/src/main/scala/org/apache/spark/util/Utils.scala
##########
@@ -3204,6 +3205,33 @@ private[spark] object Utils extends Logging {
}
files.toSeq
}
+
+ private def isG1GarbageCollector: Boolean = {
Review comment:
Can this be a `val`? I don't think it will change
Likewise could you just try to get G1HeapRegionSize once here? if it's not G1GC, just store "None". Then you don't check it each time. Because I think this can't change during the JVM's lifetime.
Then you don't need a utility method below, even
##########
File path: core/src/main/scala/org/apache/spark/memory/MemoryManager.scala
##########
@@ -255,7 +256,15 @@ private[spark] abstract class MemoryManager(
}
val size = ByteArrayMethods.nextPowerOf2(maxTungstenMemory / cores / safetyFactor)
val default = math.min(maxPageSize, math.max(minPageSize, size))
- conf.get(BUFFER_PAGESIZE).getOrElse(default)
+ val sizeAsBytes = conf.get(BUFFER_PAGESIZE).getOrElse(default)
+ if (conf.get(MEMORY_ONHEAP_PAGESIZE_OPTIMIZE_ENABLED) &&
+ tungstenMemoryMode == MemoryMode.ON_HEAP &&
+ sizeAsBytes % (1024 * 1024) == 0 &&
+ Utils.isHumongousAllocation(sizeAsBytes)) {
+ sizeAsBytes - Platform.LONG_ARRAY_OFFSET
Review comment:
Yes. I think the current logic is good, just wondering if it matter much if we do it in all cases. Maybe it's bad for small allocations, but is the offset ever significant relative to the allocation size I wonder? probably not. I wonder if there are future cases, different GCs, that we're not checking here that also need this treatment
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] srowen commented on a change in pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste in humongous allocation when using G1GC
Posted by GitBox <gi...@apache.org>.
srowen commented on a change in pull request #34846:
URL: https://github.com/apache/spark/pull/34846#discussion_r767166790
##########
File path: core/src/main/scala/org/apache/spark/util/Utils.scala
##########
@@ -3204,6 +3205,33 @@ private[spark] object Utils extends Logging {
}
files.toSeq
}
+
+ private def isG1GarbageCollector: Boolean = {
Review comment:
Can this be a `val`? I don't think it will change
Likewise could you just try to get G1HeapRegionSize once here? if it's not G1GC, just store "None". Then you don't check it each time. Because I think this can't change during the JVM's lifetime.
Then you don't need a utility method below, even
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] attilapiros commented on a change in pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
attilapiros commented on a change in pull request #34846:
URL: https://github.com/apache/spark/pull/34846#discussion_r769731048
##########
File path: core/src/main/scala/org/apache/spark/memory/MemoryManager.scala
##########
@@ -244,17 +245,26 @@ private[spark] abstract class MemoryManager(
* and then divide it by a factor of safety.
*/
val pageSizeBytes: Long = {
- val minPageSize = 1L * 1024 * 1024 // 1MB
- val maxPageSize = 64L * minPageSize // 64MB
- val cores = if (numCores > 0) numCores else Runtime.getRuntime.availableProcessors()
- // Because of rounding to next power of 2, we may have safetyFactor as 8 in worst case
- val safetyFactor = 16
- val maxTungstenMemory: Long = tungstenMemoryMode match {
- case MemoryMode.ON_HEAP => onHeapExecutionMemoryPool.poolSize
- case MemoryMode.OFF_HEAP => offHeapExecutionMemoryPool.poolSize
+ val default = Utils.G1HeapRegionSize match {
Review comment:
lazy val
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34846:
URL: https://github.com/apache/spark/pull/34846#issuecomment-995758432
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50753/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] WangGuangxin edited a comment on pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste in humongous allocation when using G1GC
Posted by GitBox <gi...@apache.org>.
WangGuangxin edited a comment on pull request #34846:
URL: https://github.com/apache/spark/pull/34846#issuecomment-992585451
> @WangGuangxin . Do you think you can add one specific example about the case to the PR description?
Hi @dongjoon-hyun , We can demo it using following piece of code.
```
public static void bufferSizeTest(boolean optimize) {
long totalAllocatedSize = 0L;
int blockSize = 1024 * 1024 * 4; // 4m
if (optimize) {
blockSize -= 16;
}
List<long[]> buffers = new ArrayList<>();
while (true) {
long[] arr = new long[blockSize/8];
buffers.add(arr);
totalAllocatedSize += blockSize;
System.out.println("Total allocated size: " + totalAllocatedSize);
}
}
```
Run it using following jvm params
```
java -Xmx100m -XX:+UseG1GC -XX:G1HeapRegionSize=4m -XX:-UseGCOverheadLimit -verbose:gc -XX:+UnlockDiagnosticVMOptions -XX:+G1SummarizeConcMark -Xss4m -XX:+ExitOnOutOfMemoryError -XX:ParallelGCThreads=4 -XX:ConcGCThreads=4
```
with optimized = false
```
Total allocated size: 46137344
[GC pause (G1 Humongous Allocation) (young) 44M->44M(100M), 0.0007091 secs]
[GC pause (G1 Evacuation Pause) (young) (initial-mark)-- 48M->48M(100M), 0.0021528 secs]
[GC concurrent-root-region-scan-start]
[GC concurrent-root-region-scan-end, 0.0000021 secs]
[GC concurrent-mark-start]
[GC pause (G1 Evacuation Pause) (young) 48M->48M(100M), 0.0011289 secs]
[Full GC (Allocation Failure) 48M->48M(100M), 0.0017284 secs]
[Full GC (Allocation Failure) 48M->48M(100M), 0.0013437 secs]
Terminating due to java.lang.OutOfMemoryError: Java heap space
```
with optimzied = true
```
Total allocated size: 96468624
[GC pause (G1 Humongous Allocation) (young)-- 92M->92M(100M), 0.0024416 secs]
[Full GC (Allocation Failure) 92M->92M(100M), 0.0019883 secs]
[GC pause (G1 Evacuation Pause) (young) (initial-mark) 96M->96M(100M), 0.0004282 secs]
[GC concurrent-root-region-scan-start]
[GC concurrent-root-region-scan-end, 0.0000040 secs]
[GC concurrent-mark-start]
[GC pause (G1 Evacuation Pause) (young) 96M->96M(100M), 0.0003269 secs]
[Full GC (Allocation Failure) 96M->96M(100M), 0.0012409 secs]
[Full GC (Allocation Failure) 96M->96M(100M), 0.0012607 secs]
Terminating due to java.lang.OutOfMemoryError: Java heap space
```
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] sunchao commented on a change in pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste in humongous allocation when using G1GC
Posted by GitBox <gi...@apache.org>.
sunchao commented on a change in pull request #34846:
URL: https://github.com/apache/spark/pull/34846#discussion_r768078534
##########
File path: core/src/main/scala/org/apache/spark/memory/MemoryManager.scala
##########
@@ -255,7 +256,17 @@ private[spark] abstract class MemoryManager(
}
val size = ByteArrayMethods.nextPowerOf2(maxTungstenMemory / cores / safetyFactor)
val default = math.min(maxPageSize, math.max(minPageSize, size))
- conf.get(BUFFER_PAGESIZE).getOrElse(default)
+ val sizeAsBytes = conf.get(BUFFER_PAGESIZE).getOrElse(default)
+ // If we are using G1 GC, it's better to take the LONG_ARRAY_OFFSET into consideration
+ // so that the requested memory size is power of 2 and can be divided by G1 region size
+ // to reduce memory waste within one G1 region
+ if (Utils.isG1GarbageCollector &&
Review comment:
I'm just curious whether this adjustment can be done in `HeapMemoryAllocator.allocate` instead? since it's closer to the actual logic.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34846:
URL: https://github.com/apache/spark/pull/34846#issuecomment-996558483
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50800/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34846:
URL: https://github.com/apache/spark/pull/34846#issuecomment-996608856
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/50800/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34846:
URL: https://github.com/apache/spark/pull/34846#issuecomment-996500126
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/146324/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34846:
URL: https://github.com/apache/spark/pull/34846#issuecomment-996603112
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50800/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] srowen commented on pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
srowen commented on pull request #34846:
URL: https://github.com/apache/spark/pull/34846#issuecomment-1001660662
What's the latest on this - looks good but a few outstanding comments
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] srowen commented on pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
srowen commented on pull request #34846:
URL: https://github.com/apache/spark/pull/34846#issuecomment-1002358271
I don't think it's always better; it sounds like a better default. I think that's what this PR does? I am reading your argument as one that it causes a performance regression as a default and I am not following that.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #34846:
URL: https://github.com/apache/spark/pull/34846#issuecomment-1002336872
The new proposed code focused only on fitting to JVM G1HeapRegionSize and it claims that it will reduce the Spark's pageSize (humongous) to smaller ones. In this case, there is no guarantee that new defaultPageSizeBytes is better than the previous calculated ones (whatever they were in the production) in terms of the executor time (if no GC occurs because there is enough memory in the extreme cases), isn't it, @srowen ?
```
private lazy val defaultPageSizeBytes = Utils.maybeG1HeapRegionSize match {
case Some(heapRegionSize) if tungstenMemoryMode == MemoryMode.ON_HEAP =>
heapRegionSize - Platform.LONG_ARRAY_OFFSET
```
If the smaller page size always wins, we may want to have a default value, `1MB` (the minimum value of G1HeapRegionSize) - Platform.LONG_ARRAY_OFFSET, for `spark.buffer.pageSize` configuration, instead of this PR.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #34846:
URL: https://github.com/apache/spark/pull/34846#issuecomment-1039824452
cc @rednaxelafx too FYI
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste in humongous allocation when using G1GC
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34846:
URL: https://github.com/apache/spark/pull/34846#issuecomment-989690943
Can one of the admins verify this patch?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] WangGuangxin commented on a change in pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste in humongous allocation when using G1GC
Posted by GitBox <gi...@apache.org>.
WangGuangxin commented on a change in pull request #34846:
URL: https://github.com/apache/spark/pull/34846#discussion_r767166389
##########
File path: core/src/main/scala/org/apache/spark/internal/config/package.scala
##########
@@ -2267,4 +2267,12 @@ package object config {
.version("3.3.0")
.intConf
.createWithDefault(5)
+
+ private[spark] val MEMORY_ONHEAP_PAGESIZE_OPTIMIZE_ENABLED =
Review comment:
yeah, it's indeed no need. removed it
##########
File path: core/src/main/scala/org/apache/spark/memory/MemoryManager.scala
##########
@@ -255,7 +256,15 @@ private[spark] abstract class MemoryManager(
}
val size = ByteArrayMethods.nextPowerOf2(maxTungstenMemory / cores / safetyFactor)
val default = math.min(maxPageSize, math.max(minPageSize, size))
- conf.get(BUFFER_PAGESIZE).getOrElse(default)
+ val sizeAsBytes = conf.get(BUFFER_PAGESIZE).getOrElse(default)
+ if (conf.get(MEMORY_ONHEAP_PAGESIZE_OPTIMIZE_ENABLED) &&
+ tungstenMemoryMode == MemoryMode.ON_HEAP &&
+ sizeAsBytes % (1024 * 1024) == 0 &&
+ Utils.isHumongousAllocation(sizeAsBytes)) {
+ sizeAsBytes - Platform.LONG_ARRAY_OFFSET
Review comment:
you mean even if it is not homongous allocation?
##########
File path: core/src/main/scala/org/apache/spark/util/Utils.scala
##########
@@ -3204,6 +3205,33 @@ private[spark] object Utils extends Logging {
}
files.toSeq
}
+
+ private def isG1GarbageCollector: Boolean = {
Review comment:
ok
##########
File path: core/src/main/scala/org/apache/spark/memory/MemoryManager.scala
##########
@@ -255,7 +256,15 @@ private[spark] abstract class MemoryManager(
}
val size = ByteArrayMethods.nextPowerOf2(maxTungstenMemory / cores / safetyFactor)
val default = math.min(maxPageSize, math.max(minPageSize, size))
- conf.get(BUFFER_PAGESIZE).getOrElse(default)
+ val sizeAsBytes = conf.get(BUFFER_PAGESIZE).getOrElse(default)
+ if (conf.get(MEMORY_ONHEAP_PAGESIZE_OPTIMIZE_ENABLED) &&
+ tungstenMemoryMode == MemoryMode.ON_HEAP &&
+ sizeAsBytes % (1024 * 1024) == 0 &&
+ Utils.isHumongousAllocation(sizeAsBytes)) {
+ sizeAsBytes - Platform.LONG_ARRAY_OFFSET
Review comment:
I revisit here, it's really no need to restrict to only homongous allocation
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] srowen commented on a change in pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste in humongous allocation when using G1GC
Posted by GitBox <gi...@apache.org>.
srowen commented on a change in pull request #34846:
URL: https://github.com/apache/spark/pull/34846#discussion_r767166912
##########
File path: core/src/main/scala/org/apache/spark/memory/MemoryManager.scala
##########
@@ -255,7 +256,15 @@ private[spark] abstract class MemoryManager(
}
val size = ByteArrayMethods.nextPowerOf2(maxTungstenMemory / cores / safetyFactor)
val default = math.min(maxPageSize, math.max(minPageSize, size))
- conf.get(BUFFER_PAGESIZE).getOrElse(default)
+ val sizeAsBytes = conf.get(BUFFER_PAGESIZE).getOrElse(default)
+ if (conf.get(MEMORY_ONHEAP_PAGESIZE_OPTIMIZE_ENABLED) &&
+ tungstenMemoryMode == MemoryMode.ON_HEAP &&
+ sizeAsBytes % (1024 * 1024) == 0 &&
+ Utils.isHumongousAllocation(sizeAsBytes)) {
+ sizeAsBytes - Platform.LONG_ARRAY_OFFSET
Review comment:
Yes. I think the current logic is good, just wondering if it matter much if we do it in all cases. Maybe it's bad for small allocations, but is the offset ever significant relative to the allocation size I wonder? probably not. I wonder if there are future cases, different GCs, that we're not checking here that also need this treatment
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] WangGuangxin commented on a change in pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste in humongous allocation when using G1GC
Posted by GitBox <gi...@apache.org>.
WangGuangxin commented on a change in pull request #34846:
URL: https://github.com/apache/spark/pull/34846#discussion_r767863314
##########
File path: core/src/main/scala/org/apache/spark/memory/MemoryManager.scala
##########
@@ -255,7 +256,14 @@ private[spark] abstract class MemoryManager(
}
val size = ByteArrayMethods.nextPowerOf2(maxTungstenMemory / cores / safetyFactor)
val default = math.min(maxPageSize, math.max(minPageSize, size))
- conf.get(BUFFER_PAGESIZE).getOrElse(default)
+ val sizeAsBytes = conf.get(BUFFER_PAGESIZE).getOrElse(default)
+ if (Utils.isG1GarbageCollector &&
+ tungstenMemoryMode == MemoryMode.ON_HEAP &&
Review comment:
I did't get this, do you mean Line 261?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] WangGuangxin commented on a change in pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste in humongous allocation when using G1GC
Posted by GitBox <gi...@apache.org>.
WangGuangxin commented on a change in pull request #34846:
URL: https://github.com/apache/spark/pull/34846#discussion_r767862725
##########
File path: core/src/main/scala/org/apache/spark/memory/MemoryManager.scala
##########
@@ -255,7 +256,14 @@ private[spark] abstract class MemoryManager(
}
val size = ByteArrayMethods.nextPowerOf2(maxTungstenMemory / cores / safetyFactor)
val default = math.min(maxPageSize, math.max(minPageSize, size))
- conf.get(BUFFER_PAGESIZE).getOrElse(default)
+ val sizeAsBytes = conf.get(BUFFER_PAGESIZE).getOrElse(default)
+ if (Utils.isG1GarbageCollector &&
+ tungstenMemoryMode == MemoryMode.ON_HEAP &&
+ sizeAsBytes % (1024 * 1024) == 0) {
+ sizeAsBytes - Platform.LONG_ARRAY_OFFSET
Review comment:
updated
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] attilapiros commented on a change in pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
attilapiros commented on a change in pull request #34846:
URL: https://github.com/apache/spark/pull/34846#discussion_r769584860
##########
File path: core/src/main/scala/org/apache/spark/memory/MemoryManager.scala
##########
@@ -255,7 +256,17 @@ private[spark] abstract class MemoryManager(
}
val size = ByteArrayMethods.nextPowerOf2(maxTungstenMemory / cores / safetyFactor)
val default = math.min(maxPageSize, math.max(minPageSize, size))
- conf.get(BUFFER_PAGESIZE).getOrElse(default)
+ val sizeAsBytes = conf.get(BUFFER_PAGESIZE).getOrElse(default)
+ // If we are using G1 GC, it's better to take the LONG_ARRAY_OFFSET into consideration
+ // so that the requested memory size is power of 2 and can be divided by G1 region size
+ // to reduce memory waste within one G1 region
+ if (Utils.isG1GarbageCollector &&
+ tungstenMemoryMode == MemoryMode.ON_HEAP &&
+ sizeAsBytes % (1024 * 1024) == 0) {
+ sizeAsBytes - Platform.LONG_ARRAY_OFFSET
Review comment:
>But when BUFFER_PAGESIZE is not set, I'm not quite sure if it's reasonable to choose it as pageSize = >G1HeapRegionSize - Platform.LONG_ARRAY_OFFSET, which seems a bit different with current logic to get default >page size.
How the current logic for the default handles a case where a custom -XX:G1HeapRegionSize is given as extra java options?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34846:
URL: https://github.com/apache/spark/pull/34846#issuecomment-989690943
Can one of the admins verify this patch?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34846:
URL: https://github.com/apache/spark/pull/34846#issuecomment-996499546
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/50798/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] viirya commented on a change in pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #34846:
URL: https://github.com/apache/spark/pull/34846#discussion_r771904579
##########
File path: core/src/main/scala/org/apache/spark/memory/MemoryManager.scala
##########
@@ -242,22 +243,30 @@ private[spark] abstract class MemoryManager(
* If user didn't explicitly set "spark.buffer.pageSize", we figure out the default value
* by looking at the number of cores available to the process, and the total amount of memory,
* and then divide it by a factor of safety.
+ *
+ * SPARK-37593 If we are using G1GC, it's better to take the LONG_ARRAY_OFFSET
+ * into consideration so that the requested memory size is power of 2
+ * and can be divided by G1 heap region size to reduce memory waste within one G1 region.
*/
- val pageSizeBytes: Long = {
- val minPageSize = 1L * 1024 * 1024 // 1MB
- val maxPageSize = 64L * minPageSize // 64MB
- val cores = if (numCores > 0) numCores else Runtime.getRuntime.availableProcessors()
- // Because of rounding to next power of 2, we may have safetyFactor as 8 in worst case
- val safetyFactor = 16
- val maxTungstenMemory: Long = tungstenMemoryMode match {
- case MemoryMode.ON_HEAP => onHeapExecutionMemoryPool.poolSize
- case MemoryMode.OFF_HEAP => offHeapExecutionMemoryPool.poolSize
- }
- val size = ByteArrayMethods.nextPowerOf2(maxTungstenMemory / cores / safetyFactor)
- val default = math.min(maxPageSize, math.max(minPageSize, size))
- conf.get(BUFFER_PAGESIZE).getOrElse(default)
+ private lazy val defaultPageSizeBytes = Utils.maybeG1HeapRegionSize match {
+ case Some(heapRegionSize) if tungstenMemoryMode == MemoryMode.ON_HEAP =>
+ heapRegionSize - Platform.LONG_ARRAY_OFFSET
+ case _ =>
+ val minPageSize = 1L * 1024 * 1024 // 1MB
+ val maxPageSize = 64L * minPageSize // 64MB
+ val cores = if (numCores > 0) numCores else Runtime.getRuntime.availableProcessors()
+ // Because of rounding to next power of 2, we may have safetyFactor as 8 in worst case
+ val safetyFactor = 16
+ val maxTungstenMemory: Long = tungstenMemoryMode match {
+ case MemoryMode.ON_HEAP => onHeapExecutionMemoryPool.poolSize
+ case MemoryMode.OFF_HEAP => offHeapExecutionMemoryPool.poolSize
+ }
+ val size = ByteArrayMethods.nextPowerOf2(maxTungstenMemory / cores / safetyFactor)
+ math.min(maxPageSize, math.max(minPageSize, size))
}
+ val pageSizeBytes: Long = conf.get(BUFFER_PAGESIZE).getOrElse(defaultPageSizeBytes)
+
Review comment:
So once users set the buffer size config, this optimization won't apply, right?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34846:
URL: https://github.com/apache/spark/pull/34846#issuecomment-995778665
**[Test build #146279 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/146279/testReport)** for PR 34846 at commit [`c2b0cb9`](https://github.com/apache/spark/commit/c2b0cb9b042050a4e284ca2745b59fad8cec30a7).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #34846:
URL: https://github.com/apache/spark/pull/34846#issuecomment-996530153
**[Test build #146326 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/146326/testReport)** for PR 34846 at commit [`0ff2bd9`](https://github.com/apache/spark/commit/0ff2bd9729df3d8f50e3639a70d4995b5b93cdd8).
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34846:
URL: https://github.com/apache/spark/pull/34846#issuecomment-995605235
**[Test build #146277 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/146277/testReport)** for PR 34846 at commit [`e43878c`](https://github.com/apache/spark/commit/e43878c044e7d2b3876d9a158ece57f0e8d45197).
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #34846:
URL: https://github.com/apache/spark/pull/34846#issuecomment-1002354003
What do you mean? I agreed that this makes Spark utilize more memory without waste.
> that you don't think this causes more mem allocation?
Yes, indeed. I also agree with that. My point is `G1HeapRegionSize - Platform.LONG_ARRAY_OFFSET` is not a silver bullet to guarantee **always-win**.
> It seems like it should be a better idea in a lot of cases.
Let me ask you in this way. Do you think `spark.buffer.pageSize = 1MB(G1HeapRegionSize) - Platform.LONG_ARRAY_OFFSET` is always better than the other values in the production?
> What's the perf regression you have in mind?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] WangGuangxin commented on a change in pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
WangGuangxin commented on a change in pull request #34846:
URL: https://github.com/apache/spark/pull/34846#discussion_r807475538
##########
File path: core/src/main/scala/org/apache/spark/memory/MemoryManager.scala
##########
@@ -267,4 +281,22 @@ private[spark] abstract class MemoryManager(
case MemoryMode.OFF_HEAP => MemoryAllocator.UNSAFE
}
}
+
+ /**
+ * Return whether we are using G1GC or not
+ */
+ private[memory] final val isG1GC: Boolean = {
Review comment:
fine, updated
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on a change in pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #34846:
URL: https://github.com/apache/spark/pull/34846#discussion_r806420798
##########
File path: core/src/main/scala/org/apache/spark/memory/MemoryManager.scala
##########
@@ -267,4 +281,22 @@ private[spark] abstract class MemoryManager(
case MemoryMode.OFF_HEAP => MemoryAllocator.UNSAFE
}
}
+
+ /**
+ * Return whether we are using G1GC or not
+ */
+ private[memory] final val isG1GC: Boolean = {
Review comment:
```suggestion
private final val isG1GC: Boolean = {
```
##########
File path: core/src/main/scala/org/apache/spark/memory/MemoryManager.scala
##########
@@ -267,4 +281,22 @@ private[spark] abstract class MemoryManager(
case MemoryMode.OFF_HEAP => MemoryAllocator.UNSAFE
}
}
+
+ /**
+ * Return whether we are using G1GC or not
+ */
+ private[memory] final val isG1GC: Boolean = {
Review comment:
Should we make it `lazy` too to fully leverage `private lazy val defaultPageSizeBytes` you wrote?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] WangGuangxin commented on pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
WangGuangxin commented on pull request #34846:
URL: https://github.com/apache/spark/pull/34846#issuecomment-1041227864
> I have a question to those who are more familiar with Spark internals though: are there any places inside of Spark that implicitly depends on the page size being a power-of-two? Do we run any risk that such assumption is not checked at runtime, which can lead to out-of-bounds access in the boundary cases?
As far as I know there is no such constraint. And Spark allows users set custom page size by conf `spark.buffer.pageSize`, whichi is not restricted to the power-of-two.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] WangGuangxin commented on pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste in humongous allocation when using G1GC
Posted by GitBox <gi...@apache.org>.
WangGuangxin commented on pull request #34846:
URL: https://github.com/apache/spark/pull/34846#issuecomment-989680510
@kiszk @cloud-fan @maropu @JoshRosen Can you please help review 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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] WangGuangxin commented on a change in pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste in humongous allocation when using G1GC
Posted by GitBox <gi...@apache.org>.
WangGuangxin commented on a change in pull request #34846:
URL: https://github.com/apache/spark/pull/34846#discussion_r767219839
##########
File path: core/src/main/scala/org/apache/spark/memory/MemoryManager.scala
##########
@@ -255,7 +256,15 @@ private[spark] abstract class MemoryManager(
}
val size = ByteArrayMethods.nextPowerOf2(maxTungstenMemory / cores / safetyFactor)
val default = math.min(maxPageSize, math.max(minPageSize, size))
- conf.get(BUFFER_PAGESIZE).getOrElse(default)
+ val sizeAsBytes = conf.get(BUFFER_PAGESIZE).getOrElse(default)
+ if (conf.get(MEMORY_ONHEAP_PAGESIZE_OPTIMIZE_ENABLED) &&
+ tungstenMemoryMode == MemoryMode.ON_HEAP &&
+ sizeAsBytes % (1024 * 1024) == 0 &&
+ Utils.isHumongousAllocation(sizeAsBytes)) {
+ sizeAsBytes - Platform.LONG_ARRAY_OFFSET
Review comment:
I revisit here, it's really no need to restrict to only homongous allocation
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #34846:
URL: https://github.com/apache/spark/pull/34846#issuecomment-996498374
**[Test build #146324 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/146324/testReport)** for PR 34846 at commit [`d166097`](https://github.com/apache/spark/commit/d166097a53630d56b5c7e5d7b97840f3fd5eb230).
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34846:
URL: https://github.com/apache/spark/pull/34846#issuecomment-996642898
**[Test build #146326 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/146326/testReport)** for PR 34846 at commit [`0ff2bd9`](https://github.com/apache/spark/commit/0ff2bd9729df3d8f50e3639a70d4995b5b93cdd8).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] WangGuangxin commented on a change in pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
WangGuangxin commented on a change in pull request #34846:
URL: https://github.com/apache/spark/pull/34846#discussion_r772120430
##########
File path: core/src/main/scala/org/apache/spark/memory/MemoryManager.scala
##########
@@ -255,7 +256,17 @@ private[spark] abstract class MemoryManager(
}
val size = ByteArrayMethods.nextPowerOf2(maxTungstenMemory / cores / safetyFactor)
val default = math.min(maxPageSize, math.max(minPageSize, size))
- conf.get(BUFFER_PAGESIZE).getOrElse(default)
+ val sizeAsBytes = conf.get(BUFFER_PAGESIZE).getOrElse(default)
+ // If we are using G1 GC, it's better to take the LONG_ARRAY_OFFSET into consideration
+ // so that the requested memory size is power of 2 and can be divided by G1 region size
+ // to reduce memory waste within one G1 region
+ if (Utils.isG1GarbageCollector &&
+ tungstenMemoryMode == MemoryMode.ON_HEAP &&
+ sizeAsBytes % (1024 * 1024) == 0) {
+ sizeAsBytes - Platform.LONG_ARRAY_OFFSET
Review comment:
@attilapiros Since -XX:G1HeapRegionSize can only be set to N-th power of 2, and the default size calculated here is also N-th power of 2, so it can be guanranteed that `G1HeapRegionSize % (pageSize + Platform.LONG_ARRAY_OFFSET) == 0` or `(pageSize + Platform.LONG_ARRAY_OFFSET) % G1HeapRegionSize == 0`, right?
Such a change has little effect on the current logic.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun edited a comment on pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun edited a comment on pull request #34846:
URL: https://github.com/apache/spark/pull/34846#issuecomment-1002336872
The new proposed code focused only on fitting to JVM G1HeapRegionSize and it claims that it will reduce the Spark's pageSize (humongous) to smaller ones. In this case, there is no guarantee that new defaultPageSizeBytes is better than the previous calculated ones (whatever they were in the production) in terms of the executor time (if no GC occurs because there is enough memory in the extreme cases).
```scala
private lazy val defaultPageSizeBytes = Utils.maybeG1HeapRegionSize match {
case Some(heapRegionSize) if tungstenMemoryMode == MemoryMode.ON_HEAP =>
heapRegionSize - Platform.LONG_ARRAY_OFFSET
```
If the smaller page size always wins, we may want to have a default value, `1MB` (the minimum value of G1HeapRegionSize) - Platform.LONG_ARRAY_OFFSET, for `spark.buffer.pageSize` configuration, instead of this PR.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] srowen commented on pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
srowen commented on pull request #34846:
URL: https://github.com/apache/spark/pull/34846#issuecomment-1002343473
I'm unclear what you mean - that you don't think this causes more mem allocation? It seems like it should be a better idea in a lot of cases. Is the issue with what the default is? like, why not just set the default at (default - epsilon) and leave it at that? What's the perf regression you have in mind?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun edited a comment on pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun edited a comment on pull request #34846:
URL: https://github.com/apache/spark/pull/34846#issuecomment-1002354003
I agreed that this makes Spark utilize more memory without waste.
> that you don't think this causes more mem allocation?
Yes, indeed. I also agree with that. My point is `G1HeapRegionSize - Platform.LONG_ARRAY_OFFSET` is not a silver bullet to guarantee **always-win**.
> It seems like it should be a better idea in a lot of cases.
Let me ask you in this way. Do you think `spark.buffer.pageSize = 1MB(G1HeapRegionSize) - Platform.LONG_ARRAY_OFFSET` is always better than the other values in the production?
> What's the perf regression you have in mind?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] srowen commented on pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
srowen commented on pull request #34846:
URL: https://github.com/apache/spark/pull/34846#issuecomment-1002370194
OK, I was reading: "However, this PR also may cause a performance regression in some jobs which need bigger pageSize without memory limitation"
This change affects the default, and is always overridden by an explicit size, it looks like. What config beyond that do you have in mind?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #34846:
URL: https://github.com/apache/spark/pull/34846#issuecomment-1002356904
To @WangGuangxin , although avoiding `OutOfMemoryError` is good, could you share your performance improvement in your environment?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] WangGuangxin commented on pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
WangGuangxin commented on pull request #34846:
URL: https://github.com/apache/spark/pull/34846#issuecomment-1035911957
@srowen @dongjoon-hyun @attilapiros
Sorry for the late reply. Based on the comments, I made a slightly change on this.
1. If user set page size by `spark.buffer.pageSize`, we just leave it alone
2. Otherwise, we first follow Spark's current algorithm to choose a best page size by available cores and memory. After that we just need to ajust it by minus `LONG_ARRAY_OFFSET` if we are using G1GC, that's enough.
Because both the choosed page size by Spark and the G1GC region size if the power of 2, so choosedPageSize and heapRegionSize must be multiple relations. There are three cases:
- choosedPageSize == heapRegionSize, then `choosedPageSize - LONG_ARRAY_OFFSET` can make sure that every page occupies one G1 region without wast.
- choosedPageSize > heapRegionSize, then `choosedPageSize - LONG_ARRAY_OFFSET` can make sure that every page occupies N G1 regions without wast.
- choosedPageSize < heapRegionSize, then `choosedPageSize - LONG_ARRAY_OFFSET` can make sure that every G1 region can allocation N pages without wast.
Compared to existing defaultPageSize, the diff is only `LONG_ARRAY_OFFSET` bytes, it's safe enough to not cause perf regression under various cases.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] WangGuangxin commented on a change in pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
WangGuangxin commented on a change in pull request #34846:
URL: https://github.com/apache/spark/pull/34846#discussion_r805219114
##########
File path: core/src/main/scala/org/apache/spark/util/Utils.scala
##########
@@ -3232,6 +3232,24 @@ private[spark] object Utils extends Logging {
case _ => math.max(sortedSize(len / 2), 1)
}
}
+
+ /**
+ * Return whether we are using G1GC or not
+ */
+ val isG1GC: Boolean = {
Review comment:
ok, updated
##########
File path: core/src/main/scala/org/apache/spark/memory/MemoryManager.scala
##########
@@ -254,10 +259,16 @@ private[spark] abstract class MemoryManager(
case MemoryMode.OFF_HEAP => offHeapExecutionMemoryPool.poolSize
}
val size = ByteArrayMethods.nextPowerOf2(maxTungstenMemory / cores / safetyFactor)
- val default = math.min(maxPageSize, math.max(minPageSize, size))
- conf.get(BUFFER_PAGESIZE).getOrElse(default)
+ val choosedPageSize = math.min(maxPageSize, math.max(minPageSize, size))
Review comment:
done
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on a change in pull request #34846: [SPARK-37593][CORE] Optimize HeapMemoryAllocator to avoid memory waste when using G1GC
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #34846:
URL: https://github.com/apache/spark/pull/34846#discussion_r806422815
##########
File path: core/src/main/scala/org/apache/spark/memory/MemoryManager.scala
##########
@@ -267,4 +281,22 @@ private[spark] abstract class MemoryManager(
case MemoryMode.OFF_HEAP => MemoryAllocator.UNSAFE
}
}
+
+ /**
+ * Return whether we are using G1GC or not
+ */
+ private[memory] final val isG1GC: Boolean = {
Review comment:
Should we make it `lazy` too to fully leverage `private lazy val defaultPageSizeBytes` you wrote?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org