Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,15 @@
*/
package com.oracle.svm.core.heap;

import java.util.concurrent.locks.ReentrantLock;

import org.graalvm.nativeimage.ImageSingletons;
import org.graalvm.word.UnsignedWord;
import org.graalvm.word.WordFactory;

import com.oracle.svm.core.Containers;
import com.oracle.svm.core.SubstrateOptions;
import com.oracle.svm.core.Uninterruptible;
import com.oracle.svm.core.jdk.UninterruptibleUtils.AtomicInteger;
import com.oracle.svm.core.stack.StackOverflowCheck;
import com.oracle.svm.core.thread.PlatformThreads;
import com.oracle.svm.core.thread.VMOperation;
Expand All @@ -55,7 +56,7 @@ default boolean hasSize() {
UnsignedWord size();
}

private static final AtomicInteger INITIALIZING = new AtomicInteger(0);
private static final ReentrantLock LOCK = new ReentrantLock();
private static final UnsignedWord UNSET_SENTINEL = UnsignedUtils.MAX_VALUE;
private static UnsignedWord cachedSize = UNSET_SENTINEL;

Expand All @@ -64,6 +65,10 @@ public static boolean isInitialized() {
return cachedSize != UNSET_SENTINEL;
}

public static boolean isInitializationInProgress() {
return LOCK.isHeldByCurrentThread();
}

/**
* Returns the size of physical memory in bytes, querying it from the OS if it has not been
* initialized yet.
Expand All @@ -81,23 +86,21 @@ public static UnsignedWord size() {
}

if (!isInitialized()) {
/*
* Multiple threads can race to initialize the cache. This is OK because all of them
* will (most-likely) compute the same value.
*/
INITIALIZING.incrementAndGet();
try {
long memoryLimit = SubstrateOptions.MaxRAM.getValue();
if (memoryLimit > 0) {
cachedSize = WordFactory.unsigned(memoryLimit);
} else {
memoryLimit = Containers.memoryLimitInBytes();
cachedSize = memoryLimit > 0
? WordFactory.unsigned(memoryLimit)
: ImageSingletons.lookup(PhysicalMemorySupport.class).size();
long memoryLimit = SubstrateOptions.MaxRAM.getValue();
if (memoryLimit > 0) {
cachedSize = WordFactory.unsigned(memoryLimit);
} else {
LOCK.lock();
try {
if (!isInitialized()) {
memoryLimit = Containers.memoryLimitInBytes();
cachedSize = memoryLimit > 0
? WordFactory.unsigned(memoryLimit)
: ImageSingletons.lookup(PhysicalMemorySupport.class).size();
}
} finally {
LOCK.unlock();
}
} finally {
INITIALIZING.decrementAndGet();
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,11 @@
import java.util.concurrent.atomic.AtomicLong;

import com.oracle.svm.core.annotate.Alias;
import com.oracle.svm.core.annotate.InjectAccessors;
import com.oracle.svm.core.annotate.RecomputeFieldValue;
import com.oracle.svm.core.annotate.RecomputeFieldValue.Kind;
import com.oracle.svm.core.annotate.TargetClass;
import com.oracle.svm.core.heap.PhysicalMemory;

@TargetClass(className = "java.nio.Bits")
final class Target_java_nio_Bits {
Expand All @@ -40,9 +42,9 @@ final class Target_java_nio_Bits {
private static int PAGE_SIZE = -1;

@Alias @RecomputeFieldValue(kind = Kind.FromAlias) //
private static boolean MEMORY_LIMIT_SET = false;
@Alias @RecomputeFieldValue(kind = Kind.FromAlias) //
private static long MAX_MEMORY = -1;
private static boolean MEMORY_LIMIT_SET = true;
@Alias @InjectAccessors(MaxMemoryAccessor.class) //
private static long MAX_MEMORY;

@Alias @RecomputeFieldValue(kind = Kind.FromAlias) //
private static AtomicLong RESERVED_MEMORY = new AtomicLong();
Expand All @@ -53,3 +55,23 @@ final class Target_java_nio_Bits {

// Checkstyle: resume
}

/**
* {@code java.nio.Bits} caches the max. direct memory size in the field {@code MAX_MEMORY}. We
* disable this cache and always call {@link DirectMemoryAccessors#getDirectMemory()} instead, which
* uses our own cache. Otherwise, it could happen that {@code MAX_MEMORY} caches a temporary value
* that is used during early VM startup, before {@link PhysicalMemory} is fully initialized.
*/
final class MaxMemoryAccessor {
// Checkstyle: stop

static long getMAX_MEMORY() {
return DirectMemoryAccessors.getDirectMemory();
}

static void setMAX_MEMORY(long value) {
/* Nothing to do. */
}

// Checkstyle: resume
}
Comment on lines +59 to +77
Copy link
Collaborator

@jerboaa jerboaa Oct 6, 2023

Choose a reason for hiding this comment

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

Instead of going through the accessor an alternative could be to leaverage the init hooks in JDK code and actually set the init level to 1 once we know the real MaxDirectMemorySize (based on the heap size). For example here: https://github.com/oracle/graal/pull/7553/files#diff-db72af86eee732c8a7e3e53319b32fdd92ee0ec00735f413a02858c9dc0aff4dR126. That would mimic what HotSpot itself is doing. FWIW, that's what my original code was intending to do.

Both approaches work.

Copy link
Member

Choose a reason for hiding this comment

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

The problem is that Native Image is already multi-threaded when this code is executed. Modifying the init level may have unexpected effects on other threads.

Copy link
Collaborator

Choose a reason for hiding this comment

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

OK, thanks.

Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import com.oracle.svm.core.annotate.RecomputeFieldValue.Kind;
import com.oracle.svm.core.annotate.Substitute;
import com.oracle.svm.core.annotate.TargetClass;
import com.oracle.svm.core.heap.PhysicalMemory;
import com.oracle.svm.core.snippets.KnownIntrinsics;

import jdk.internal.misc.Unsafe;
Expand Down Expand Up @@ -67,36 +68,28 @@ public static ClassLoader latestUserDefinedLoader0() {

@Alias @InjectAccessors(DirectMemoryAccessors.class) //
private static long directMemory;
@Alias @InjectAccessors(DirectMemoryAccessors.class) //
@Alias @InjectAccessors(PageAlignDirectMemoryAccessors.class) //
private static boolean pageAlignDirectMemory;
}

final class DirectMemoryAccessors {
private static final long DIRECT_MEMORY_DURING_INITIALIZATION = 25 * 1024 * 1024;

/*
* Not volatile to avoid a memory barrier when reading the values. Instead, an explicit barrier
* is inserted when writing the values.
*/
private static boolean initialized;

private static long directMemory;
private static boolean pageAlignDirectMemory;

static long getDirectMemory() {
if (!initialized) {
initialize();
return tryInitialize();
}
return directMemory;
}

static boolean getPageAlignDirectMemory() {
if (!initialized) {
initialize();
}
return pageAlignDirectMemory;
}

private static void initialize() {
private static long tryInitialize() {
/*
* The JDK method VM.saveAndRemoveProperties looks at the system property
* "sun.nio.MaxDirectMemorySize". However, that property is always set by the Java HotSpot
Expand All @@ -109,6 +102,15 @@ private static void initialize() {
* No value explicitly specified. The default in the JDK in this case is the maximum
* heap size.
*/
if (PhysicalMemory.isInitializationInProgress()) {
/*
* When initializing PhysicalMemory, we use NIO/cgroups code that calls
* VM.getDirectMemory(). When this initialization is in progress, we need to prevent
* that Runtime.maxMemory() is called below because it would trigger a recursive
* initialization of PhysicalMemory. So, we return a temporary value.
*/
return DIRECT_MEMORY_DURING_INITIALIZATION;
}
newDirectMemory = Runtime.getRuntime().maxMemory();
}

Expand All @@ -118,6 +120,31 @@ private static void initialize() {
* possible but not a case we care about.
*/
directMemory = newDirectMemory;

/* Ensure values are published to other threads before marking fields as initialized. */
Unsafe.getUnsafe().storeFence();
initialized = true;

return newDirectMemory;
}
}

final class PageAlignDirectMemoryAccessors {
/*
* Not volatile to avoid a memory barrier when reading the values. Instead, an explicit barrier
* is inserted when writing the values.
*/
private static boolean initialized;
private static boolean pageAlignDirectMemory;

static boolean getPageAlignDirectMemory() {
if (!initialized) {
initialize();
}
return pageAlignDirectMemory;
}

private static void initialize() {
pageAlignDirectMemory = Boolean.getBoolean("sun.nio.PageAlignDirectMemory");

/* Ensure values are published to other threads before marking fields as initialized. */
Expand Down