Skip to content

[SPARK-24421][CORE][FOLLOWUP] Use normal direct ByteBuffer allocation if Cleaner can't be set #23424

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
srowen wants to merge 3 commits into from
Closed

[SPARK-24421][CORE][FOLLOWUP] Use normal direct ByteBuffer allocation if Cleaner can't be set #23424

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2f267a5
Use normal direct ByteBuffer allocation if Cleaner can't be set
srowen Jan 2, 2019
ee0cd3c
Add more info to OutOfMemoryError when allocating direct buffer
srowen Jan 3, 2019
82a887c
Fix checkstyle
srowen Jan 3, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 14 additions & 10 deletions common/unsafe/src/main/java/org/apache/spark/unsafe/Platform.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,21 +209,25 @@ public static long reallocateMemory(long address, long oldSize, long newSize) {
}

/**
* 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 {
if (CLEANER_CREATE_METHOD == null) {
// Can't set a Cleaner (see comments on field), so need to allocate via normal Java APIs
return ByteBuffer.allocateDirect(size);
Copy link
Member

@felixcheung felixcheung Jan 2, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

try /catch OOM and log a message on setting MaxDirectMemorySize?

Copy link
Member Author

@srowen srowen Jan 3, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, it does look like it will throw OutOfMemoryError here if off-heap allocation fails. That's a decent hint, yes.

}
// 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);
if (CLEANER_CREATE_METHOD != null) {
try {
DBB_CLEANER_FIELD.set(buffer,
CLEANER_CREATE_METHOD.invoke(null, buffer, (Runnable) () -> freeMemory(memory)));
} catch (IllegalAccessException | InvocationTargetException e) {
throw new IllegalStateException(e);
}
try {
DBB_CLEANER_FIELD.set(buffer,
CLEANER_CREATE_METHOD.invoke(null, buffer, (Runnable) () -> freeMemory(memory)));
} catch (IllegalAccessException | InvocationTargetException e) {
freeMemory(memory);
Copy link
Member Author

@srowen srowen Jan 2, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to be totally safe, free the memory that was allocated but can't be used now in this case.

throw new IllegalStateException(e);
}
return buffer;
} catch (Exception e) {
Expand Down