From 29e696a9931d681ee08cc896da8733621293637d Mon Sep 17 00:00:00 2001 From: Ayres Date: Thu, 7 Jun 2018 13:42:55 -0700 Subject: [PATCH 1/3] Fixes Scala memory leak (#10436) --- .../core/src/main/scala/org/apache/mxnet/FeedForward.scala | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/scala-package/core/src/main/scala/org/apache/mxnet/FeedForward.scala b/scala-package/core/src/main/scala/org/apache/mxnet/FeedForward.scala index 7289df197125..274a68aaf03b 100644 --- a/scala-package/core/src/main/scala/org/apache/mxnet/FeedForward.scala +++ b/scala-package/core/src/main/scala/org/apache/mxnet/FeedForward.scala @@ -230,8 +230,9 @@ class FeedForward private( val padded = batch.pad val realSize = batchSize - padded for ((list, nd) <- outputs zip predExec.outputs) { - list += nd.slice(0, realSize).copy() + list += nd.slice(0, realSize) } + batch.dispose() } // TODO(Yizhi): we can use Symbol.concat to do the same thing. Can it be more efficient? val results = outputs.map(NDArray.concatenate(_)) From 258bd6415339eb044ba834b226cb88c81ed0c534 Mon Sep 17 00:00:00 2001 From: Ayres Date: Thu, 7 Jun 2018 14:39:23 -0700 Subject: [PATCH 2/3] Replaced the copy and disposed of sliced ndArray to resolve memory leak --- .../core/src/main/scala/org/apache/mxnet/FeedForward.scala | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/scala-package/core/src/main/scala/org/apache/mxnet/FeedForward.scala b/scala-package/core/src/main/scala/org/apache/mxnet/FeedForward.scala index 274a68aaf03b..89a79c163a67 100644 --- a/scala-package/core/src/main/scala/org/apache/mxnet/FeedForward.scala +++ b/scala-package/core/src/main/scala/org/apache/mxnet/FeedForward.scala @@ -230,7 +230,9 @@ class FeedForward private( val padded = batch.pad val realSize = batchSize - padded for ((list, nd) <- outputs zip predExec.outputs) { - list += nd.slice(0, realSize) + val ndSliced = nd.slice(0, realSize) + list += ndSliced.copy() + ndSliced.dispose() } batch.dispose() } From a3cf5eb4901cfb45ed6e258bc3433a19b8e0f2af Mon Sep 17 00:00:00 2001 From: Ayres Date: Mon, 11 Jun 2018 11:47:10 -0700 Subject: [PATCH 3/3] Wrapped disposes in a finally to ensure they are called. --- .../scala/org/apache/mxnet/FeedForward.scala | 28 ++++++++++++------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/scala-package/core/src/main/scala/org/apache/mxnet/FeedForward.scala b/scala-package/core/src/main/scala/org/apache/mxnet/FeedForward.scala index 89a79c163a67..87c9bc72be0c 100644 --- a/scala-package/core/src/main/scala/org/apache/mxnet/FeedForward.scala +++ b/scala-package/core/src/main/scala/org/apache/mxnet/FeedForward.scala @@ -224,17 +224,25 @@ class FeedForward private( var i = 0 while (data.hasNext && i != numBatch) { val batch = data.next() - i += 1 - ExecutorManager.loadData(batch, dataArrays) - predExec.forward(isTrain = false) - val padded = batch.pad - val realSize = batchSize - padded - for ((list, nd) <- outputs zip predExec.outputs) { - val ndSliced = nd.slice(0, realSize) - list += ndSliced.copy() - ndSliced.dispose() + try { + i += 1 + ExecutorManager.loadData(batch, dataArrays) + predExec.forward(isTrain = false) + val padded = batch.pad + val realSize = batchSize - padded + for ((list, nd) <- outputs zip predExec.outputs) { + // The slice is being written to a value so that dispose can be called after the copy. + // The one liner nd.slice().copy() leads to leaking the memory of the slice. + val ndSliced = nd.slice(0, realSize) + try { + list += ndSliced.copy() + } finally { + ndSliced.dispose() + } + } + } finally { + batch.dispose() } - batch.dispose() } // TODO(Yizhi): we can use Symbol.concat to do the same thing. Can it be more efficient? val results = outputs.map(NDArray.concatenate(_))