Skip to content

Commit 003ae1f

Browse files
srowenJackey Lee
authored andcommitted
[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 apache#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 apache#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 apache#23419 (comment) ) This suggests there is a problem and that this resolves it. Closes apache#23424 from srowen/SPARK-24421.2. Authored-by: Sean Owen <[email protected]> Signed-off-by: Sean Owen <[email protected]>
1 parent 2d59f0a commit 003ae1f

File tree

1 file changed

+21
-10
lines changed

1 file changed

+21
-10
lines changed

common/unsafe/src/main/java/org/apache/spark/unsafe/Platform.java

Lines changed: 21 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -209,22 +209,33 @@ public static long reallocateMemory(long address, long oldSize, long newSize) {
209209
}
210210

211211
/**
212-
* Uses internal JDK APIs to allocate a DirectByteBuffer while ignoring the JVM's
213-
* MaxDirectMemorySize limit (the default limit is too low and we do not want to require users
214-
* to increase it).
212+
* Allocate a DirectByteBuffer, potentially bypassing the JVM's MaxDirectMemorySize limit.
215213
*/
216214
public static ByteBuffer allocateDirectBuffer(int size) {
217215
try {
218-
long memory = allocateMemory(size);
219-
ByteBuffer buffer = (ByteBuffer) DBB_CONSTRUCTOR.newInstance(memory, size);
220-
if (CLEANER_CREATE_METHOD != null) {
216+
if (CLEANER_CREATE_METHOD == null) {
217+
// Can't set a Cleaner (see comments on field), so need to allocate via normal Java APIs
221218
try {
222-
DBB_CLEANER_FIELD.set(buffer,
223-
CLEANER_CREATE_METHOD.invoke(null, buffer, (Runnable) () -> freeMemory(memory)));
224-
} catch (IllegalAccessException | InvocationTargetException e) {
225-
throw new IllegalStateException(e);
219+
return ByteBuffer.allocateDirect(size);
220+
} catch (OutOfMemoryError oome) {
221+
// checkstyle.off: RegexpSinglelineJava
222+
throw new OutOfMemoryError("Failed to allocate direct buffer (" + oome.getMessage() +
223+
"); try increasing -XX:MaxDirectMemorySize=... to, for example, your heap size");
224+
// checkstyle.on: RegexpSinglelineJava
226225
}
227226
}
227+
// Otherwise, use internal JDK APIs to allocate a DirectByteBuffer while ignoring the JVM's
228+
// MaxDirectMemorySize limit (the default limit is too low and we do not want to
229+
// require users to increase it).
230+
long memory = allocateMemory(size);
231+
ByteBuffer buffer = (ByteBuffer) DBB_CONSTRUCTOR.newInstance(memory, size);
232+
try {
233+
DBB_CLEANER_FIELD.set(buffer,
234+
CLEANER_CREATE_METHOD.invoke(null, buffer, (Runnable) () -> freeMemory(memory)));
235+
} catch (IllegalAccessException | InvocationTargetException e) {
236+
freeMemory(memory);
237+
throw new IllegalStateException(e);
238+
}
228239
return buffer;
229240
} catch (Exception e) {
230241
throwException(e);

0 commit comments

Comments
 (0)