[spark] branch master updated: [SPARK-24421][CORE][FOLLOWUP] Use normal direct ByteBuffer allocation if Cleaner can't be set

[srowen]

This is an automated email from the ASF dual-hosted git repository.

srowen pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/spark.git


The following commit(s) were added to refs/heads/master by this push:
     new 89cebf4  [SPARK-24421][CORE][FOLLOWUP] Use normal direct ByteBuffer 
allocation if Cleaner can't be set
89cebf4 is described below

commit 89cebf4932ff966cc876ba8a9ecd9d9c034fb071
Author: Sean Owen <sean.o...@databricks.com>
AuthorDate: Fri Jan 4 15:37:09 2019 -0600

    [SPARK-24421][CORE][FOLLOWUP] Use normal direct ByteBuffer allocation if 
Cleaner can't be set
    
    ## What changes were proposed in this pull request?
    
    In Java 9+ we can't use sun.misc.Cleaner by default anymore, and this was 
largely handled in https://github.com/apache/spark/pull/22993 However I think 
the change there left a significant problem.
    
    If a DirectByteBuffer is allocated using the reflective hack in Platform, 
now, we by default can't set a Cleaner. But I believe this means the memory 
isn't freed promptly or possibly at all. If a Cleaner can't be set, I think we 
need to use normal APIs to allocate the direct ByteBuffer.
    
    According to comments in the code, the downside is simply that the normal 
APIs will check and impose limits on how much off-heap memory can be allocated. 
Per the original review on https://github.com/apache/spark/pull/22993 this much 
seems fine, as either way in this case the user would have to add a JVM setting 
(increase max, or allow the reflective access).
    
    ## How was this patch tested?
    
    Existing tests. This resolved an OutOfMemoryError in Java 11 from TimSort 
tests without increasing test heap size. (See 
https://github.com/apache/spark/pull/23419#issuecomment-450772125 ) This 
suggests there is a problem and that this resolves it.
    
    Closes #23424 from srowen/SPARK-24421.2.
    
    Authored-by: Sean Owen <sean.o...@databricks.com>
    Signed-off-by: Sean Owen <sean.o...@databricks.com>
---
 .../java/org/apache/spark/unsafe/Platform.java     | 31 +++++++++++++++-------
 1 file changed, 21 insertions(+), 10 deletions(-)

diff --git a/common/unsafe/src/main/java/org/apache/spark/unsafe/Platform.java 
b/common/unsafe/src/main/java/org/apache/spark/unsafe/Platform.java
index 076b693..1adf7ab 100644
--- a/common/unsafe/src/main/java/org/apache/spark/unsafe/Platform.java
+++ b/common/unsafe/src/main/java/org/apache/spark/unsafe/Platform.java
@@ -209,22 +209,33 @@ public final class Platform {
   }
 
   /**
-   * Uses internal JDK APIs to allocate a DirectByteBuffer while ignoring the 
JVM's
-   * MaxDirectMemorySize limit (the default limit is too low and we do not 
want to require users
-   * to increase it).
+   * Allocate a DirectByteBuffer, potentially bypassing the JVM's 
MaxDirectMemorySize limit.
    */
   public static ByteBuffer allocateDirectBuffer(int size) {
     try {
-      long memory = allocateMemory(size);
-      ByteBuffer buffer = (ByteBuffer) DBB_CONSTRUCTOR.newInstance(memory, 
size);
-      if (CLEANER_CREATE_METHOD != null) {
+      if (CLEANER_CREATE_METHOD == null) {
+        // Can't set a Cleaner (see comments on field), so need to allocate 
via normal Java APIs
         try {
-          DBB_CLEANER_FIELD.set(buffer,
-              CLEANER_CREATE_METHOD.invoke(null, buffer, (Runnable) () -> 
freeMemory(memory)));
-        } catch (IllegalAccessException | InvocationTargetException e) {
-          throw new IllegalStateException(e);
+          return ByteBuffer.allocateDirect(size);
+        } catch (OutOfMemoryError oome) {
+          // checkstyle.off: RegexpSinglelineJava
+          throw new OutOfMemoryError("Failed to allocate direct buffer (" + 
oome.getMessage() +
+              "); try increasing -XX:MaxDirectMemorySize=... to, for example, 
your heap size");
+          // checkstyle.on: RegexpSinglelineJava
         }
       }
+      // Otherwise, use internal JDK APIs to allocate a DirectByteBuffer while 
ignoring the JVM's
+      // MaxDirectMemorySize limit (the default limit is too low and we do not 
want to
+      // require users to increase it).
+      long memory = allocateMemory(size);
+      ByteBuffer buffer = (ByteBuffer) DBB_CONSTRUCTOR.newInstance(memory, 
size);
+      try {
+        DBB_CLEANER_FIELD.set(buffer,
+            CLEANER_CREATE_METHOD.invoke(null, buffer, (Runnable) () -> 
freeMemory(memory)));
+      } catch (IllegalAccessException | InvocationTargetException e) {
+        freeMemory(memory);
+        throw new IllegalStateException(e);
+      }
       return buffer;
     } catch (Exception e) {
       throwException(e);


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@spark.apache.org
For additional commands, e-mail: commits-h...@spark.apache.org