[GitHub] spark pull request #20862: [SPARK-23744][CORE]Fix memory leak in ReadableCha...
Github user 10110346 closed the pull request at: https://github.com/apache/spark/pull/20862 --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #20862: [SPARK-23744][CORE]Fix memory leak in ReadableCha...
Github user 10110346 commented on a diff in the pull request:
https://github.com/apache/spark/pull/20862#discussion_r178688855
--- Diff: core/src/main/scala/org/apache/spark/storage/DiskStore.scala ---
@@ -301,7 +301,10 @@ private class ReadableChannelFileRegion(source:
ReadableByteChannel, blockSize:
written
}
- override def deallocate(): Unit = source.close()
+ override def deallocate() {
+source.close()
+StorageUtils.dispose(buffer)
--- End diff --
I see,thanks
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #20862: [SPARK-23744][CORE]Fix memory leak in ReadableCha...
Github user kiszk commented on a diff in the pull request:
https://github.com/apache/spark/pull/20862#discussion_r178626148
--- Diff: core/src/main/scala/org/apache/spark/storage/DiskStore.scala ---
@@ -301,7 +301,10 @@ private class ReadableChannelFileRegion(source:
ReadableByteChannel, blockSize:
written
}
- override def deallocate(): Unit = source.close()
+ override def deallocate() {
+source.close()
+StorageUtils.dispose(buffer)
--- End diff --
In other words, I think that the off-heap in `buffer` will not be leaked if
`buffer` will be reclaimed.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #20862: [SPARK-23744][CORE]Fix memory leak in ReadableCha...
Github user kiszk commented on a diff in the pull request:
https://github.com/apache/spark/pull/20862#discussion_r17862
--- Diff: core/src/main/scala/org/apache/spark/storage/DiskStore.scala ---
@@ -301,7 +301,10 @@ private class ReadableChannelFileRegion(source:
ReadableByteChannel, blockSize:
written
}
- override def deallocate(): Unit = source.close()
+ override def deallocate() {
+source.close()
+StorageUtils.dispose(buffer)
--- End diff --
Sorry for being late since I overlooked.
While `ByteBuffer.allocateDirect` allocates memory in off-heap memory, the
memory will be also reclaimed when the ByteBuffer object will be reclaimed.
Regarding `-XX:+DisableExplicitGC`, as described
[here](http://www.oracle.com/technetwork/java/javase/tech/vmoptions-jsp-140102.html),
this option just disables `System.gc()`. Other GC operations may reclaim the
memory in off-heap allocated by `ByteBuffer.allocateDirect`.
>By default calls to System.gc() are enabled (-XX:-DisableExplicitGC). Use
-XX:+DisableExplicitGC to disable calls to System.gc(). Note that the JVM still
performs garbage collection when necessary.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #20862: [SPARK-23744][CORE]Fix memory leak in ReadableCha...
Github user 10110346 commented on a diff in the pull request:
https://github.com/apache/spark/pull/20862#discussion_r177993186
--- Diff: core/src/test/scala/org/apache/spark/storage/DiskStoreSuite.scala
---
@@ -197,7 +197,7 @@ class DiskStoreSuite extends SparkFunSuite {
while (region.transfered() < region.count()) {
region.transferTo(byteChannel, region.transfered())
}
-
+region.release()
--- End diff --
I think we should release the direct memory in unit test
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #20862: [SPARK-23744][CORE]Fix memory leak in ReadableCha...
Github user 10110346 commented on a diff in the pull request:
https://github.com/apache/spark/pull/20862#discussion_r177992858
--- Diff: core/src/main/scala/org/apache/spark/storage/DiskStore.scala ---
@@ -301,7 +301,10 @@ private class ReadableChannelFileRegion(source:
ReadableByteChannel, blockSize:
written
}
- override def deallocate(): Unit = source.close()
+ override def deallocate() {
+source.close()
+StorageUtils.dispose(buffer)
--- End diff --
In fact, direct memory is also off-heap memory,
if we config this parameter "-XX:+DisableExplicitGC",the memory will not be
reclaimed
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #20862: [SPARK-23744][CORE]Fix memory leak in ReadableCha...
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/20862#discussion_r177986789
--- Diff: core/src/test/scala/org/apache/spark/storage/DiskStoreSuite.scala
---
@@ -197,7 +197,7 @@ class DiskStoreSuite extends SparkFunSuite {
while (region.transfered() < region.count()) {
region.transferTo(byteChannel, region.transfered())
}
-
+region.release()
--- End diff --
why this change?
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #20862: [SPARK-23744][CORE]Fix memory leak in ReadableCha...
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/20862#discussion_r177986757
--- Diff: core/src/main/scala/org/apache/spark/storage/DiskStore.scala ---
@@ -301,7 +301,10 @@ private class ReadableChannelFileRegion(source:
ReadableByteChannel, blockSize:
written
}
- override def deallocate(): Unit = source.close()
+ override def deallocate() {
+source.close()
+StorageUtils.dispose(buffer)
--- End diff --
IIRC direct byte buffer can be GCed, cc @kiszk
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #20862: [SPARK-23744][CORE]Fix memory leak in ReadableCha...
GitHub user 10110346 opened a pull request: https://github.com/apache/spark/pull/20862 [SPARK-23744][CORE]Fix memory leak in ReadableChannelFileRegion ## What changes were proposed in this pull request? In the class `ReadableChannelFileRegion`, the `buffer` is direct memory, we should modify `deallocate` to free it, and `deallocate` will be called by `release` ## How was this patch tested? existing unit test You can merge this pull request into a Git repository by running: $ git pull https://github.com/10110346/spark leakmem Alternatively you can review and apply these changes as the patch at: https://github.com/apache/spark/pull/20862.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #20862 commit 5b58c57607551328c893a3857717e4b159ecf841 Author: liuxian Date: 2018-03-20T03:19:59Z fix --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
