From 97fd4d97637d904c674ee56182cd2d26ec5c61c5 Mon Sep 17 00:00:00 2001 From: Christian Haeubl Date: Fri, 10 Dec 2021 08:49:38 +0100 Subject: [PATCH 1/7] Enable the reference handler thread by default. --- .../src/com/oracle/svm/core/SubstrateOptions.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/SubstrateOptions.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/SubstrateOptions.java index 8b274d5ac6e0..32bd9b4497d0 100644 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/SubstrateOptions.java +++ b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/SubstrateOptions.java @@ -573,7 +573,7 @@ public Boolean getValue(OptionValues values) { /** Use {@link ReferenceHandler#useDedicatedThread()} instead. */ @Option(help = "Populate reference queues in a separate thread rather than after a garbage collection.", type = OptionType.Expert) // - public static final RuntimeOptionKey UseReferenceHandlerThread = new RuntimeOptionKey<>(false); + public static final RuntimeOptionKey UseReferenceHandlerThread = new RuntimeOptionKey<>(true); } @Option(help = "Overwrites the available number of processors provided by the OS. Any value <= 0 means using the processor count from the OS.")// From 482fcd9a5ff8f7ac1b27a8c75710a1f6562a8a94 Mon Sep 17 00:00:00 2001 From: Christian Haeubl Date: Fri, 10 Dec 2021 08:50:02 +0100 Subject: [PATCH 2/7] Remove the field RestrictHeapAccess.overridesCallers. --- .../src/com/oracle/svm/core/annotate/RestrictHeapAccess.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/annotate/RestrictHeapAccess.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/annotate/RestrictHeapAccess.java index 59c0ce22f6b8..9aa51b6642a7 100644 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/annotate/RestrictHeapAccess.java +++ b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/annotate/RestrictHeapAccess.java @@ -47,8 +47,5 @@ public boolean isMoreRestrictiveThan(Access other) { Access access(); - // Unnecessary, will be removed in GR-34779. - boolean overridesCallers() default false; - String reason(); } From 7c1ea1a219d0194126e1d58d67750fb40b5fdd01 Mon Sep 17 00:00:00 2001 From: Christian Haeubl Date: Thu, 16 Dec 2021 18:59:39 +0100 Subject: [PATCH 3/7] CommittedMemoryProvider cleanups. --- .../os/AbstractCommittedMemoryProvider.java | 33 ---------------- .../svm/core/os/CommittedMemoryProvider.java | 38 +------------------ .../core/os/OSCommittedMemoryProvider.java | 37 ++++++++++++++++-- 3 files changed, 35 insertions(+), 73 deletions(-) diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/os/AbstractCommittedMemoryProvider.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/os/AbstractCommittedMemoryProvider.java index 87412abf2579..3acca7f3f2dd 100644 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/os/AbstractCommittedMemoryProvider.java +++ b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/os/AbstractCommittedMemoryProvider.java @@ -48,39 +48,6 @@ public boolean guaranteesHeapPreferredAddressSpaceAlignment() { return SubstrateOptions.SpawnIsolates.getValue() && ImageHeapProvider.get().guaranteesHeapPreferredAddressSpaceAlignment(); } - @Override - public Pointer allocateAlignedChunk(UnsignedWord nbytes, UnsignedWord alignment) { - return allocate(nbytes, alignment, false); - } - - @Override - public Pointer allocateUnalignedChunk(UnsignedWord nbytes) { - return allocate(nbytes, CommittedMemoryProvider.UNALIGNED, false); - } - - @Override - public Pointer allocateExecutableMemory(UnsignedWord nbytes, UnsignedWord alignment) { - return allocate(nbytes, alignment, true); - } - - @Override - @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true) - public void freeAlignedChunk(PointerBase start, UnsignedWord nbytes, UnsignedWord alignment) { - free(start, nbytes, alignment, false); - } - - @Override - @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true) - public void freeUnalignedChunk(PointerBase start, UnsignedWord nbytes) { - free(start, nbytes, CommittedMemoryProvider.UNALIGNED, false); - } - - @Override - @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true) - public void freeExecutableMemory(PointerBase start, UnsignedWord nbytes, UnsignedWord alignment) { - free(start, nbytes, alignment, true); - } - @Uninterruptible(reason = "Still being initialized.") protected static int protectSingleIsolateImageHeap() { assert !SubstrateOptions.SpawnIsolates.getValue() : "Must be handled by ImageHeapProvider when SpawnIsolates is enabled"; diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/os/CommittedMemoryProvider.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/os/CommittedMemoryProvider.java index 1782f18eb5c5..fe50ad8939f1 100644 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/os/CommittedMemoryProvider.java +++ b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/os/CommittedMemoryProvider.java @@ -26,7 +26,6 @@ import java.util.EnumSet; -import com.oracle.svm.core.util.VMError; import org.graalvm.compiler.api.replacements.Fold; import org.graalvm.nativeimage.ImageSingletons; import org.graalvm.nativeimage.c.type.WordPointer; @@ -90,22 +89,6 @@ default UnsignedWord getGranularity() { return VirtualMemoryProvider.get().getGranularity(); } - /** - * Allocate a block of committed memory. - * - * @param nbytes The number of bytes to allocate, which is rounded up to the next multiple of - * the {@linkplain #getGranularity() granularity} if required. - * @param alignment The required alignment of the block start, or {@link #UNALIGNED}. - * @param executable Whether the block must be executable. - * @return The start of the allocated block, or {@link WordFactory#nullPointer()} in case of an - * error. - */ - default Pointer allocate(UnsignedWord nbytes, UnsignedWord alignment, boolean executable) { - // We need this default method temporarily so that we can remove the methods allocate and - // free from all subclasses in GR-34236. - throw VMError.shouldNotReachHere("Subclasses must overwrite this method"); - } - Pointer allocateAlignedChunk(UnsignedWord nbytes, UnsignedWord alignment); Pointer allocateUnalignedChunk(UnsignedWord nbytes); @@ -118,23 +101,6 @@ default Pointer allocate(UnsignedWord nbytes, UnsignedWord alignment, boolean ex */ boolean areUnalignedChunksZeroed(); - /** - * Release a block of committed memory that was allocated with {@link #allocate}, requiring the - * exact same parameter values that were originally passed to {@link #allocate}. - * - * @param start The start of the memory block, as returned by {@link #allocate}. - * @param nbytes The originally requested size in bytes. - * @param alignment The originally requested alignment. - * @param executable Whether the block was requested to be executable. - * @return true on success, or false otherwise. - */ - @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true) - default boolean free(PointerBase start, UnsignedWord nbytes, UnsignedWord alignment, boolean executable) { - // We need this default method temporarily so that we can remove the methods allocate and - // free from all subclasses in GR-34236. - throw VMError.shouldNotReachHere("Subclasses must overwrite this method"); - } - @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true) void freeAlignedChunk(PointerBase start, UnsignedWord nbytes, UnsignedWord alignment); @@ -165,8 +131,8 @@ enum Access { } /** - * Change access permissions for a block of committed memory that was allocated with - * {@link #allocate}. + * Change access permissions for a block of committed memory that was allocated with one of the + * allocation methods. * * @param start The start of the address range to be protected, which must be a multiple of the * {@linkplain #getGranularity() granularity}. diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/os/OSCommittedMemoryProvider.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/os/OSCommittedMemoryProvider.java index 1c2e63e80773..4c440d13a0ba 100644 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/os/OSCommittedMemoryProvider.java +++ b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/os/OSCommittedMemoryProvider.java @@ -77,7 +77,21 @@ public int tearDown() { } @Override - public Pointer allocate(UnsignedWord size, UnsignedWord alignment, boolean executable) { + public Pointer allocateAlignedChunk(UnsignedWord nbytes, UnsignedWord alignment) { + return allocate(nbytes, alignment, false); + } + + @Override + public Pointer allocateUnalignedChunk(UnsignedWord nbytes) { + return allocate(nbytes, WordFactory.unsigned(1), false); + } + + @Override + public Pointer allocateExecutableMemory(UnsignedWord nbytes, UnsignedWord alignment) { + return allocate(nbytes, alignment, true); + } + + private Pointer allocate(UnsignedWord size, UnsignedWord alignment, boolean executable) { int access = VirtualMemoryProvider.Access.READ | VirtualMemoryProvider.Access.WRITE; if (executable) { access |= VirtualMemoryProvider.Access.EXECUTE; @@ -108,12 +122,27 @@ public boolean areUnalignedChunksZeroed() { @Override @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true) - public boolean free(PointerBase start, UnsignedWord nbytes, UnsignedWord alignment, boolean executable) { + public void freeAlignedChunk(PointerBase start, UnsignedWord nbytes, UnsignedWord alignment) { + free(start, nbytes); + } + + @Override + @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true) + public void freeUnalignedChunk(PointerBase start, UnsignedWord nbytes) { + free(start, nbytes); + } + + @Override + @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true) + public void freeExecutableMemory(PointerBase start, UnsignedWord nbytes, UnsignedWord alignment) { + free(start, nbytes); + } + + @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true) + private void free(PointerBase start, UnsignedWord nbytes) { if (VirtualMemoryProvider.get().free(start, nbytes) == 0) { tracker.untrack(nbytes); - return true; } - return false; } private final VirtualMemoryTracker tracker = new VirtualMemoryTracker(); From 9bcea63d8a9c60d932fbab3961df03c70376f5ab Mon Sep 17 00:00:00 2001 From: Christian Haeubl Date: Thu, 27 Jan 2022 13:46:12 +0100 Subject: [PATCH 4/7] Add a mode where the reference handling can be triggered manually. --- substratevm/mx.substratevm/mx_substratevm.py | 4 +- .../oracle/svm/core/genscavenge/GCImpl.java | 52 ++++++------------- .../oracle/svm/core/genscavenge/HeapImpl.java | 8 +++ .../genscavenge/ThreadLocalAllocation.java | 32 ++++++++++-- .../svm/core/IsolateArgumentParser.java | 38 ++++++++++---- .../com/oracle/svm/core/SubstrateOptions.java | 24 ++++++++- .../src/com/oracle/svm/core/heap/Heap.java | 19 ++++++- .../svm/core/heap/ReferenceHandler.java | 24 +++++++-- .../svm/core/heap/ReferenceHandlerMode.java | 46 ++++++++++++++++ .../svm/core/heap/ReferenceHandlerThread.java | 8 +-- .../svm/core/heap/ReferenceInternals.java | 9 ++++ .../svm-driver/native-image.properties | 2 +- .../hotspot/libgraal/LibGraalFeature.java | 13 ++++- .../graal/isolated/IsolatedGraalUtils.java | 9 ++-- .../isolated/IsolateAwareTruffleCompiler.java | 6 +++ 15 files changed, 219 insertions(+), 75 deletions(-) create mode 100644 substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/heap/ReferenceHandlerMode.java diff --git a/substratevm/mx.substratevm/mx_substratevm.py b/substratevm/mx.substratevm/mx_substratevm.py index 5a790299f6cb..bb17e7ef8bb4 100644 --- a/substratevm/mx.substratevm/mx_substratevm.py +++ b/substratevm/mx.substratevm/mx_substratevm.py @@ -990,8 +990,10 @@ def _native_image_launcher_extra_jvm_args(): '-H:+PreserveFramePointer', '-H:-DeleteLocalSymbols', - # No VM-internal threads may be spawned for libgraal. + + # No VM-internal threads may be spawned for libgraal and the reference handler is executed manually. '-H:-AllowVMInternalThreads', + '-R:ReferenceHandlerMode=0', ], ), ], diff --git a/substratevm/src/com.oracle.svm.core.genscavenge/src/com/oracle/svm/core/genscavenge/GCImpl.java b/substratevm/src/com.oracle.svm.core.genscavenge/src/com/oracle/svm/core/genscavenge/GCImpl.java index 374e361323cf..93f3e9567dfb 100644 --- a/substratevm/src/com.oracle.svm.core.genscavenge/src/com/oracle/svm/core/genscavenge/GCImpl.java +++ b/substratevm/src/com.oracle.svm.core.genscavenge/src/com/oracle/svm/core/genscavenge/GCImpl.java @@ -29,6 +29,7 @@ import java.lang.ref.Reference; +import com.oracle.svm.core.thread.JavaThreads; import org.graalvm.compiler.api.replacements.Fold; import org.graalvm.nativeimage.CurrentIsolate; import org.graalvm.nativeimage.IsolateThread; @@ -151,7 +152,7 @@ private void collect(GCCause cause, boolean forceFullGC) { if (outOfMemory) { throw OUT_OF_MEMORY_ERROR; } - doReferenceHandling(); + doReferenceHandlingInRegularThread(); } } @@ -1055,46 +1056,23 @@ private void finishCollection() { collectionInProgress = false; } + // This method will be removed as soon as possible, see GR-36676. + static void doReferenceHandlingInRegularThread() { + if (ReferenceHandler.useRegularJavaThread() && !VMOperation.isInProgress() && JavaThreads.currentJavaThreadInitialized()) { + doReferenceHandling(); + } + } + /** - * NOTE: All code that is transitively reachable from this method may get executed as a side - * effect of a GC or as a side effect of an allocation slow path. To prevent hard to debug - * transient issues, we execute as little code as possible in this method. Multiple threads may - * execute this method concurrently. - * - * Without a dedicated reference handler thread, arbitrary complex code can get executed as a - * side effect of this method. So, allocations of Java objects or an explicitly triggered GC can - * result in a recursive invocation of methods. - * - * This can have the effect that global state changes unexpectedly and may result in issues that - * look similar to races but that can even happen in single-threaded environments, e.g.: - * - * 
-     * {@code
-     * private static Object singleton;
-     *
-     * private static synchronized Object createSingleton() {
-     *     if (singleton == null) {
-     *         Object o = new Object();
-     *         // If the allocation above enters the allocation slow path code, then it is possible
-     *         // that doReferenceHandling() gets executed by the current thread. If the method
-     *         // createSingleton() is called as a side effect of doReferenceHandling(), then the
-     *         // assertion below may fail because the singleton got already initialized by the same
-     *         // thread in the meanwhile.
-     *         assert singleton == null;
-     *         singleton = o;
-     *     }
-     *     return result;
-     * }
-     * }
-     *
+ * Inside a VMOperation, we are not allowed to do certain things, e.g., perform synchronization + * (because it can deadlock when a lock is held outside the VMOperation). Similar restrictions + * apply if we are too early in the attach sequence of a thread. */ static void doReferenceHandling() { - if (ReferenceHandler.useDedicatedThread()) { - return; - } - + assert !VMOperation.isInProgress() : "could result in deadlocks"; + assert JavaThreads.currentJavaThreadInitialized() : "thread is not fully initialized yet"; /* Most of the time, we won't have a pending reference list. So, we do that check first. */ - if (HeapImpl.getHeapImpl().hasReferencePendingListUnsafe() && ReferenceHandler.isReferenceHandlingAllowed()) { + if (HeapImpl.getHeapImpl().hasReferencePendingListUnsafe()) { long startTime = System.nanoTime(); ReferenceHandler.processPendingReferencesInRegularThread(); diff --git a/substratevm/src/com.oracle.svm.core.genscavenge/src/com/oracle/svm/core/genscavenge/HeapImpl.java b/substratevm/src/com.oracle.svm.core.genscavenge/src/com/oracle/svm/core/genscavenge/HeapImpl.java index 89a6cea30b60..f7cde39d4416 100644 --- a/substratevm/src/com.oracle.svm.core.genscavenge/src/com/oracle/svm/core/genscavenge/HeapImpl.java +++ b/substratevm/src/com.oracle.svm.core.genscavenge/src/com/oracle/svm/core/genscavenge/HeapImpl.java @@ -64,6 +64,7 @@ import com.oracle.svm.core.heap.ObjectHeader; import com.oracle.svm.core.heap.ObjectVisitor; import com.oracle.svm.core.heap.PhysicalMemory; +import com.oracle.svm.core.heap.ReferenceHandler; import com.oracle.svm.core.heap.ReferenceHandlerThread; import com.oracle.svm.core.heap.ReferenceInternals; import com.oracle.svm.core.heap.RuntimeCodeInfoGCSupport; @@ -470,6 +471,13 @@ public BarrierSet createBarrierSet(MetaAccessProvider metaAccess) { return RememberedSet.get().createBarrierSet(metaAccess); } + @Override + public void doReferenceHandling() { + if (ReferenceHandler.isExecutedManually()) { + GCImpl.doReferenceHandling(); + } + } + @SuppressFBWarnings(value = "VO_VOLATILE_INCREMENT", justification = "Only the GC increments the volatile field 'refListOfferCounter'.") void addToReferencePendingList(Reference list) { assert VMOperation.isGCInProgress(); diff --git a/substratevm/src/com.oracle.svm.core.genscavenge/src/com/oracle/svm/core/genscavenge/ThreadLocalAllocation.java b/substratevm/src/com.oracle.svm.core.genscavenge/src/com/oracle/svm/core/genscavenge/ThreadLocalAllocation.java index f85a07a9c79e..2837c97a46b4 100644 --- a/substratevm/src/com.oracle.svm.core.genscavenge/src/com/oracle/svm/core/genscavenge/ThreadLocalAllocation.java +++ b/substratevm/src/com.oracle.svm.core.genscavenge/src/com/oracle/svm/core/genscavenge/ThreadLocalAllocation.java @@ -175,13 +175,35 @@ private static Object slowPathNewInstance(Word objectHeader) { } /** - * NOTE: All code that is transitively reachable from this method may get executed as a side - * effect of an allocation slow path. To prevent hard to debug transient issues, we execute as - * little code as possible in this method (see {@link GCImpl#doReferenceHandling()} for more - * details). Multiple threads may execute this method concurrently. + * NOTE: Multiple threads may execute this method concurrently. All code that is transitively + * reachable from this method may get executed as a side effect of an allocation slow path. To + * prevent hard to debug transient issues, we execute as little code as possible in this method. + * + * If the executed code is too complex, then it can happen that we unexpectedly change some + * shared global state as a side effect of an allocation. This may result in issues that look + * similar to races but that can even happen in single-threaded environments, e.g.: + * + * 
+     * {@code
+     * private static Object singleton;
+     *
+     * private static synchronized Object createSingleton() {
+     *     if (singleton == null) {
+     *         Object o = new Object();
+     *         // If the allocation above enters the allocation slow path code, and executes a complex
+     *         // slow path hook, then it is possible that createSingleton() gets recursively execute
+     *         // by the current thread. So, the assertion below may fail because the singleton got
+     *         // already initialized by the same thread in the meanwhile.
+     *         assert singleton == null;
+     *         singleton = o;
+     *     }
+     *     return result;
+     * }
+     * }
+     *
*/ private static void runSlowPathHooks() { - GCImpl.doReferenceHandling(); + GCImpl.doReferenceHandlingInRegularThread(); GCImpl.getPolicy().updateSizeParameters(); } diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/IsolateArgumentParser.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/IsolateArgumentParser.java index fc2cea3f501f..0bbaf980c664 100644 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/IsolateArgumentParser.java +++ b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/IsolateArgumentParser.java @@ -56,7 +56,7 @@ */ public class IsolateArgumentParser { private static final RuntimeOptionKey[] OPTIONS = {SubstrateGCOptions.MinHeapSize, SubstrateGCOptions.MaxHeapSize, SubstrateGCOptions.MaxNewSize, - SubstrateOptions.ConcealedOptions.UseReferenceHandlerThread}; + SubstrateOptions.ConcealedOptions.ReferenceHandlerMode}; private static final int OPTION_COUNT = OPTIONS.length; private static final CGlobalData OPTION_NAMES = CGlobalDataFactory.createBytes(IsolateArgumentParser::createOptionNames); private static final CGlobalData OPTION_NAME_POSITIONS = CGlobalDataFactory.createBytes(IsolateArgumentParser::createOptionNamePosition); @@ -134,8 +134,10 @@ private static byte[] createHostedValues() { private static long toLong(Object value) { if (value instanceof Boolean) { return ((Boolean) value) ? 1 : 0; + } else if (value instanceof Integer) { + return (Integer) value; } else if (value instanceof Long) { - return ((Long) value); + return (Long) value; } else { throw VMError.shouldNotReachHere("Unexpected option value: " + value); } @@ -163,13 +165,13 @@ public static void parse(CEntryPointCreateIsolateParameters parameters, CLongPoi if (arg.isNonNull()) { CCharPointer tail = matchPrefix(arg); if (tail.isNonNull()) { - tail = matchXOption(tail); - if (tail.isNonNull()) { - parseXOption(parsedArgs, numericValue, tail); + CCharPointer xOptionTail = matchXOption(tail); + if (xOptionTail.isNonNull()) { + parseXOption(parsedArgs, numericValue, xOptionTail); } else { - tail = matchXXOption(arg); - if (tail.isNonNull()) { - parseXXOption(parsedArgs, numericValue, tail); + CCharPointer xxOptionTail = matchXXOption(tail); + if (xxOptionTail.isNonNull()) { + parseXXOption(parsedArgs, numericValue, xxOptionTail); } } } @@ -190,16 +192,24 @@ public void verifyOptionValues() { } } + @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true) public static boolean getBooleanOptionValue(int index) { return PARSED_OPTION_VALUES[index] == 1; } + @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true) + public static int getIntOptionValue(int index) { + return (int) PARSED_OPTION_VALUES[index]; + } + private static Object getOptionValue(int index) { Class optionValueType = OPTIONS[index].getDescriptor().getOptionValueType(); long value = PARSED_OPTION_VALUES[index]; if (optionValueType == Boolean.class) { assert value == 0 || value == 1; return value == 1; + } else if (optionValueType == Integer.class) { + return (int) value; } else if (optionValueType == Long.class) { return value; } else { @@ -277,7 +287,7 @@ private static void parseXXOption(CLongPointer parsedArgs, CLongPointer numericV for (int i = 0; i < OPTION_COUNT; i++) { int pos = OPTION_NAME_POSITIONS.get().read(i); CCharPointer optionName = OPTION_NAMES.get().addressOf(pos); - if (OPTION_TYPES.get().read(i) == OptionValueType.Long && parseNumericXXOption(tail, optionName, numericValue)) { + if (OptionValueType.isNumeric(OPTION_TYPES.get().read(i)) && parseNumericXXOption(tail, optionName, numericValue)) { parsedArgs.write(i, numericValue.read()); break; } @@ -411,17 +421,25 @@ public static int getStructSize() { private static class OptionValueType { public static byte Boolean = 1; - public static byte Long = 2; + public static byte Integer = 2; + public static byte Long = 3; public static byte fromClass(Class c) { if (c == Boolean.class) { return Boolean; + } else if (c == Integer.class) { + return Integer; } else if (c == Long.class) { return Long; } else { throw VMError.shouldNotReachHere("Option value has unexpected type: " + c); } } + + @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true) + public static boolean isNumeric(byte optionValueType) { + return optionValueType == Integer || optionValueType == Long; + } } } diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/SubstrateOptions.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/SubstrateOptions.java index 32bd9b4497d0..d7cde4694e5b 100644 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/SubstrateOptions.java +++ b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/SubstrateOptions.java @@ -572,8 +572,28 @@ public Boolean getValue(OptionValues values) { public static final HostedOptionKey UseDedicatedVMOperationThread = new HostedOptionKey<>(false); /** Use {@link ReferenceHandler#useDedicatedThread()} instead. */ - @Option(help = "Populate reference queues in a separate thread rather than after a garbage collection.", type = OptionType.Expert) // - public static final RuntimeOptionKey UseReferenceHandlerThread = new RuntimeOptionKey<>(true); + @Option(help = "Determines how reference handling is done. " + + "0 - reference handling can only be executed manually, " + + "1 - reference handling is done in a dedicated reference handler thread, " + + "2 - (deprecated) reference handling is done in regular Java threads.", type = OptionType.Expert) // + public static final RuntimeOptionKey ReferenceHandlerMode = new ImmutableRuntimeOptionKey<>(null) { + @Override + public Integer getValueOrDefault(UnmodifiableEconomicMap, Object> values) { + if (!values.containsKey(this)) { + if (AllowVMInternalThreads.getValueOrDefault(values)) { + return com.oracle.svm.core.heap.ReferenceHandlerMode.UseDedicatedThread; + } else { + return com.oracle.svm.core.heap.ReferenceHandlerMode.UseRegularJavaThreads; + } + } + return super.getValueOrDefault(values); + } + + @Override + public Integer getValue(OptionValues values) { + return getValueOrDefault(values.getMap()); + } + }; } @Option(help = "Overwrites the available number of processors provided by the OS. Any value <= 0 means using the processor count from the OS.")// diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/heap/Heap.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/heap/Heap.java index c690be5c47e7..22854fb5a00f 100644 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/heap/Heap.java +++ b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/heap/Heap.java @@ -25,6 +25,7 @@ package com.oracle.svm.core.heap; import java.lang.ref.Reference; +import java.lang.ref.ReferenceQueue; import java.util.ArrayList; import java.util.List; @@ -196,9 +197,25 @@ public List> getLoadedClasses() { @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true) public abstract boolean isInPrimaryImageHeap(Pointer objectPtr); + /** + * If the automatic reference handling is disabled (see + * {@link ReferenceHandlerMode#ExecuteManually}), then this method can be called to do the + * reference handling manually. On execution, the current thread will enqueue pending + * {@link Reference}s into their corresponding {@link ReferenceQueue}s and it will execute + * pending cleaners. + * + * This method must not be called from within a VM operation as this could result in deadlocks. + * Furthermore, it is up to the caller to ensure that this method is only called in places where + * neither the reference handling nor the cleaner execution can cause any unexpected side + * effects on the application behavior. + * + * If the automatic reference handling is enabled, then this method is a no-op. + */ + public abstract void doReferenceHandling(); + /** * Determines if the heap currently has {@link Reference} objects that are pending to be - * {@linkplain java.lang.ref.ReferenceQueue enqueued}. + * {@linkplain ReferenceQueue enqueued}. */ public abstract boolean hasReferencePendingList(); diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/heap/ReferenceHandler.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/heap/ReferenceHandler.java index 4e02f2536b77..e49bf4771b5c 100644 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/heap/ReferenceHandler.java +++ b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/heap/ReferenceHandler.java @@ -26,19 +26,33 @@ import java.lang.ref.Reference; +import com.oracle.svm.core.IsolateArgumentParser; +import com.oracle.svm.core.SubstrateOptions; import com.oracle.svm.core.SubstrateUtil; import com.oracle.svm.core.stack.StackOverflowCheck; -import com.oracle.svm.core.thread.PlatformThreads; -import com.oracle.svm.core.thread.VMOperation; import com.oracle.svm.core.util.VMError; public final class ReferenceHandler { public static boolean useDedicatedThread() { - return ReferenceHandlerThread.isSupported() && ReferenceHandlerThread.isEnabled(); + if (ReferenceHandlerThread.isSupported()) { + int optionIndex = IsolateArgumentParser.getOptionIndex(SubstrateOptions.ConcealedOptions.ReferenceHandlerMode); + return IsolateArgumentParser.getIntOptionValue(optionIndex) == ReferenceHandlerMode.UseDedicatedThread; + } + return false; + } + + public static boolean useRegularJavaThread() { + int optionIndex = IsolateArgumentParser.getOptionIndex(SubstrateOptions.ConcealedOptions.ReferenceHandlerMode); + return IsolateArgumentParser.getIntOptionValue(optionIndex) == ReferenceHandlerMode.UseRegularJavaThreads; + } + + public static boolean isExecutedManually() { + int optionIndex = IsolateArgumentParser.getOptionIndex(SubstrateOptions.ConcealedOptions.ReferenceHandlerMode); + return IsolateArgumentParser.getIntOptionValue(optionIndex) == ReferenceHandlerMode.ExecuteManually; } public static void processPendingReferencesInRegularThread() { - assert !useDedicatedThread() && isReferenceHandlingAllowed(); + assert !useDedicatedThread(); /* * We might be running in a user thread that is close to a stack overflow, so enable the @@ -80,7 +94,7 @@ public static boolean isReferenceHandlingAllowed() { * synchronization (because it can deadlock when a lock is held outside the VMOperation). * Similar restrictions apply if we are too early in the attach sequence of a thread. */ - return !VMOperation.isInProgress() && PlatformThreads.isCurrentAssigned(); + return !VMOperation.isInProgress() && JavaThreads.currentJavaThreadInitialized(); } private ReferenceHandler() { diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/heap/ReferenceHandlerMode.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/heap/ReferenceHandlerMode.java new file mode 100644 index 000000000000..9cc9ef43e9ab --- /dev/null +++ b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/heap/ReferenceHandlerMode.java @@ -0,0 +1,46 @@ +/* + * Copyright (c) 2022, 2022, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. Oracle designates this + * particular file as subject to the "Classpath" exception as provided + * by Oracle in the LICENSE file that accompanied this code. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ +package com.oracle.svm.core.heap; + +/** + * Values that can be used for the option + * {@link com.oracle.svm.core.SubstrateOptions.ConcealedOptions#ReferenceHandlerMode}. + */ +public abstract class ReferenceHandlerMode { + /** + * Disables automatic reference handling. If reference handling is needed, then it must be done + * manually by calling {@link Heap#doReferenceHandling()}. + */ + public static final int ExecuteManually = 0; + + /** Uses a dedicated thread to do the reference handling. */ + public static final int UseDedicatedThread = 1; + + /** + * Deprecated, will be removed as soon as possible (see GR-36676): does the reference handling + * in the regular Java threads (e.g., at the end of the allocation slow-path). + */ + public static final int UseRegularJavaThreads = 2; +} diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/heap/ReferenceHandlerThread.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/heap/ReferenceHandlerThread.java index 14ee170050ae..b47d1301bea3 100644 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/heap/ReferenceHandlerThread.java +++ b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/heap/ReferenceHandlerThread.java @@ -24,8 +24,6 @@ */ package com.oracle.svm.core.heap; -import com.oracle.svm.core.IsolateArgumentParser; -import com.oracle.svm.core.SubstrateOptions; import org.graalvm.compiler.api.replacements.Fold; import org.graalvm.nativeimage.CurrentIsolate; import org.graalvm.nativeimage.ImageSingletons; @@ -34,6 +32,7 @@ import org.graalvm.nativeimage.Platforms; import org.graalvm.nativeimage.hosted.Feature; +import com.oracle.svm.core.SubstrateOptions; import com.oracle.svm.core.annotate.AutomaticFeature; import com.oracle.svm.core.thread.ThreadingSupportImpl; import com.oracle.svm.core.thread.VMThreads; @@ -103,11 +102,6 @@ static ReferenceHandlerThread singleton() { static boolean isSupported() { return SubstrateOptions.MultiThreaded.getValue() && SubstrateOptions.AllowVMInternalThreads.getValue(); } - - static boolean isEnabled() { - int optionIndex = IsolateArgumentParser.getOptionIndex(SubstrateOptions.ConcealedOptions.UseReferenceHandlerThread); - return IsolateArgumentParser.getBooleanOptionValue(optionIndex); - } } @AutomaticFeature diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/heap/ReferenceInternals.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/heap/ReferenceInternals.java index f9e1dc9357ce..fb4e0452d7bd 100644 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/heap/ReferenceInternals.java +++ b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/heap/ReferenceInternals.java @@ -220,6 +220,15 @@ public static boolean waitForReferenceProcessing() throws InterruptedException { assert !VMOperation.isInProgress() : "could cause a deadlock"; assert !ReferenceHandlerThread.isReferenceHandlerThread() : "would cause a deadlock"; + if (ReferenceHandler.isExecutedManually()) { + /* + * When the reference handling is executed manually, then we don't know when pending + * references will be processed. So, we must not block when there are pending references + * as this could cause deadlocks. + */ + return false; + } + synchronized (processPendingLock) { if (processPendingActive || Heap.getHeap().hasReferencePendingList()) { processPendingLock.wait(); // Wait for progress, not necessarily completion diff --git a/substratevm/src/com.oracle.svm.driver/resources/META-INF/native-image/com.oracle.substratevm/svm-driver/native-image.properties b/substratevm/src/com.oracle.svm.driver/resources/META-INF/native-image/com.oracle.substratevm/svm-driver/native-image.properties index 99d886e6d913..a3883325fe9c 100644 --- a/substratevm/src/com.oracle.svm.driver/resources/META-INF/native-image/com.oracle.substratevm/svm-driver/native-image.properties +++ b/substratevm/src/com.oracle.svm.driver/resources/META-INF/native-image/com.oracle.substratevm/svm-driver/native-image.properties @@ -1,2 +1,2 @@ ImageName = native-image -Args = -R:+UseReferenceHandlerThread -H:-ParseRuntimeOptions --initialize-at-build-time=com.oracle.svm.driver +Args = -H:-ParseRuntimeOptions --initialize-at-build-time=com.oracle.svm.driver diff --git a/substratevm/src/com.oracle.svm.graal.hotspot.libgraal/src/com/oracle/svm/graal/hotspot/libgraal/LibGraalFeature.java b/substratevm/src/com.oracle.svm.graal.hotspot.libgraal/src/com/oracle/svm/graal/hotspot/libgraal/LibGraalFeature.java index 29dfd3094de2..315122eaf2d8 100644 --- a/substratevm/src/com.oracle.svm.graal.hotspot.libgraal/src/com/oracle/svm/graal/hotspot/libgraal/LibGraalFeature.java +++ b/substratevm/src/com.oracle.svm.graal.hotspot.libgraal/src/com/oracle/svm/graal/hotspot/libgraal/LibGraalFeature.java @@ -52,6 +52,7 @@ import java.util.function.Supplier; import java.util.stream.Collectors; +import com.oracle.svm.core.heap.Heap; import org.graalvm.collections.EconomicMap; import org.graalvm.collections.MapCursor; import org.graalvm.compiler.code.DisassemblerProvider; @@ -656,8 +657,16 @@ private static CompilationRequestResult compileMethod(HotSpotGraalCompiler compi // This scope is required to allow Graal compilations of host methods to call methods // on the TruffleCompilerRuntime. This is, for example, required to find out about // Truffle-specific method annotations. - try (JNIMethodScope scope = LibGraalUtil.openScope("", env)) { - return compiler.compileMethod(request, true, compiler.getGraalRuntime().getOptions()); + try { + try (JNIMethodScope scope = LibGraalUtil.openScope("", env)) { + return compiler.compileMethod(request, true, compiler.getGraalRuntime().getOptions()); + } + } finally { + /* + * libgraal doesn't use a dedicated reference handler thread, so we trigger the + * reference handling manually when a compilation finishes. + */ + Heap.getHeap().doReferenceHandling(); } } } diff --git a/substratevm/src/com.oracle.svm.graal/src/com/oracle/svm/graal/isolated/IsolatedGraalUtils.java b/substratevm/src/com.oracle.svm.graal/src/com/oracle/svm/graal/isolated/IsolatedGraalUtils.java index 8918b3b77ef7..1ba0f9a261c4 100644 --- a/substratevm/src/com.oracle.svm.graal/src/com/oracle/svm/graal/isolated/IsolatedGraalUtils.java +++ b/substratevm/src/com.oracle.svm.graal/src/com/oracle/svm/graal/isolated/IsolatedGraalUtils.java @@ -27,6 +27,7 @@ import java.lang.reflect.Array; import java.nio.ByteBuffer; +import com.oracle.svm.core.heap.ReferenceHandlerMode; import org.graalvm.collections.EconomicMap; import org.graalvm.collections.UnmodifiableMapCursor; import org.graalvm.compiler.code.CompilationResult; @@ -66,8 +67,8 @@ public static CompilerIsolateThread createCompilationIsolate() { if (addressSpaceSize > 0) { builder.reservedAddressSpaceSize(WordFactory.signed(addressSpaceSize)); } - // Compilation isolates never need a dedicated reference handler thread. - builder.appendArgument(getOptionString(SubstrateOptions.ConcealedOptions.UseReferenceHandlerThread, false)); + // Compilation isolates do the reference handling manually to avoid the extra thread. + builder.appendArgument(getOptionString(SubstrateOptions.ConcealedOptions.ReferenceHandlerMode, ReferenceHandlerMode.ExecuteManually)); CreateIsolateParameters params = builder.build(); CompilerIsolateThread isolate = (CompilerIsolateThread) Isolates.createIsolate(params); initializeCompilationIsolate(isolate); @@ -192,8 +193,8 @@ public static void applyClientRuntimeOptionValues(PointerBase encodedOptionsPtr, RuntimeOptionValues.singleton().update(options); } - private static String getOptionString(RuntimeOptionKey option, boolean value) { - return "-XX:" + (value ? "+" : "-") + option.getName(); + private static String getOptionString(RuntimeOptionKey option, int value) { + return "-XX:" + option.getName() + "=" + value; } private IsolatedGraalUtils() { diff --git a/substratevm/src/com.oracle.svm.truffle/src/com/oracle/svm/truffle/isolated/IsolateAwareTruffleCompiler.java b/substratevm/src/com.oracle.svm.truffle/src/com/oracle/svm/truffle/isolated/IsolateAwareTruffleCompiler.java index d1f016798a71..48223611dc88 100644 --- a/substratevm/src/com.oracle.svm.truffle/src/com/oracle/svm/truffle/isolated/IsolateAwareTruffleCompiler.java +++ b/substratevm/src/com.oracle.svm.truffle/src/com/oracle/svm/truffle/isolated/IsolateAwareTruffleCompiler.java @@ -30,6 +30,7 @@ import java.util.Map; import java.util.concurrent.atomic.AtomicBoolean; +import com.oracle.svm.core.heap.Heap; import org.graalvm.compiler.api.replacements.SnippetReflectionProvider; import org.graalvm.compiler.core.common.SuppressFBWarnings; import org.graalvm.compiler.nodes.PauseNode; @@ -223,6 +224,11 @@ private static ClientHandle doCompile0(@SuppressWarnings("unused") @CEnt t.printStackTrace(new PrintWriter(writer)); return IsolatedCompileContext.get().createStringInClient(writer.toString()); } finally { + /* + * Compilation isolate do not use a dedicated reference handler thread, so we trigger + * the reference handling manually when a compilation finishes. + */ + Heap.getHeap().doReferenceHandling(); IsolatedCompileContext.set(null); } } From 7ed3399cc760f1fe4a8756f569f35dab0ec27ddd Mon Sep 17 00:00:00 2001 From: Christian Haeubl Date: Wed, 2 Feb 2022 15:25:16 +0100 Subject: [PATCH 5/7] Changed the name of the option that is used to disable automatic reference handling. --- substratevm/mx.substratevm/mx_substratevm.py | 4 +- .../svm/core/IsolateArgumentParser.java | 2 +- .../com/oracle/svm/core/SubstrateOptions.java | 26 ++--------- .../src/com/oracle/svm/core/heap/Heap.java | 9 ++-- .../svm/core/heap/ReferenceHandler.java | 15 +++--- .../svm/core/heap/ReferenceHandlerMode.java | 46 ------------------- .../hotspot/libgraal/LibGraalEntryPoints.java | 7 +++ .../graal/isolated/IsolatedGraalUtils.java | 7 ++- 8 files changed, 32 insertions(+), 84 deletions(-) delete mode 100644 substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/heap/ReferenceHandlerMode.java diff --git a/substratevm/mx.substratevm/mx_substratevm.py b/substratevm/mx.substratevm/mx_substratevm.py index bb17e7ef8bb4..f7a1f9a527db 100644 --- a/substratevm/mx.substratevm/mx_substratevm.py +++ b/substratevm/mx.substratevm/mx_substratevm.py @@ -991,9 +991,9 @@ def _native_image_launcher_extra_jvm_args(): '-H:-DeleteLocalSymbols', - # No VM-internal threads may be spawned for libgraal and the reference handler is executed manually. + # No VM-internal threads may be spawned for libgraal and the reference handling is executed manually. '-H:-AllowVMInternalThreads', - '-R:ReferenceHandlerMode=0', + '-R:-AutomaticReferenceHandling', ], ), ], diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/IsolateArgumentParser.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/IsolateArgumentParser.java index 0bbaf980c664..d79bc41e2d1c 100644 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/IsolateArgumentParser.java +++ b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/IsolateArgumentParser.java @@ -56,7 +56,7 @@ */ public class IsolateArgumentParser { private static final RuntimeOptionKey[] OPTIONS = {SubstrateGCOptions.MinHeapSize, SubstrateGCOptions.MaxHeapSize, SubstrateGCOptions.MaxNewSize, - SubstrateOptions.ConcealedOptions.ReferenceHandlerMode}; + SubstrateOptions.ConcealedOptions.UseReferenceHandlerThread, SubstrateOptions.ConcealedOptions.AutomaticReferenceHandling}; private static final int OPTION_COUNT = OPTIONS.length; private static final CGlobalData OPTION_NAMES = CGlobalDataFactory.createBytes(IsolateArgumentParser::createOptionNames); private static final CGlobalData OPTION_NAME_POSITIONS = CGlobalDataFactory.createBytes(IsolateArgumentParser::createOptionNamePosition); diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/SubstrateOptions.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/SubstrateOptions.java index d7cde4694e5b..ad0edcf0d9a0 100644 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/SubstrateOptions.java +++ b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/SubstrateOptions.java @@ -572,28 +572,12 @@ public Boolean getValue(OptionValues values) { public static final HostedOptionKey UseDedicatedVMOperationThread = new HostedOptionKey<>(false); /** Use {@link ReferenceHandler#useDedicatedThread()} instead. */ - @Option(help = "Determines how reference handling is done. " + - "0 - reference handling can only be executed manually, " + - "1 - reference handling is done in a dedicated reference handler thread, " + - "2 - (deprecated) reference handling is done in regular Java threads.", type = OptionType.Expert) // - public static final RuntimeOptionKey ReferenceHandlerMode = new ImmutableRuntimeOptionKey<>(null) { - @Override - public Integer getValueOrDefault(UnmodifiableEconomicMap, Object> values) { - if (!values.containsKey(this)) { - if (AllowVMInternalThreads.getValueOrDefault(values)) { - return com.oracle.svm.core.heap.ReferenceHandlerMode.UseDedicatedThread; - } else { - return com.oracle.svm.core.heap.ReferenceHandlerMode.UseRegularJavaThreads; - } - } - return super.getValueOrDefault(values); - } + @Option(help = "Populate reference queues in a separate thread rather than after a garbage collection.", type = OptionType.Expert) // + public static final RuntimeOptionKey UseReferenceHandlerThread = new ImmutableRuntimeOptionKey<>(true); - @Override - public Integer getValue(OptionValues values) { - return getValueOrDefault(values.getMap()); - } - }; + /** Use {@link ReferenceHandler#isExecutedManually()} instead. */ + @Option(help = "Determines if the reference handling is executed automatically or manually.", type = OptionType.Expert) // + public static final RuntimeOptionKey AutomaticReferenceHandling = new ImmutableRuntimeOptionKey<>(true); } @Option(help = "Overwrites the available number of processors provided by the OS. Any value <= 0 means using the processor count from the OS.")// diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/heap/Heap.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/heap/Heap.java index 22854fb5a00f..50df84eac429 100644 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/heap/Heap.java +++ b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/heap/Heap.java @@ -38,6 +38,7 @@ import org.graalvm.word.Pointer; import org.graalvm.word.UnsignedWord; +import com.oracle.svm.core.SubstrateOptions; import com.oracle.svm.core.annotate.Uninterruptible; import com.oracle.svm.core.hub.DynamicHub; import com.oracle.svm.core.hub.PredefinedClassesSupport; @@ -199,10 +200,10 @@ public List> getLoadedClasses() { /** * If the automatic reference handling is disabled (see - * {@link ReferenceHandlerMode#ExecuteManually}), then this method can be called to do the - * reference handling manually. On execution, the current thread will enqueue pending - * {@link Reference}s into their corresponding {@link ReferenceQueue}s and it will execute - * pending cleaners. + * {@link SubstrateOptions.ConcealedOptions#AutomaticReferenceHandling}), then this method can + * be called to do the reference handling manually. On execution, the current thread will + * enqueue pending {@link Reference}s into their corresponding {@link ReferenceQueue}s and it + * will execute pending cleaners. * * This method must not be called from within a VM operation as this could result in deadlocks. * Furthermore, it is up to the caller to ensure that this method is only called in places where diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/heap/ReferenceHandler.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/heap/ReferenceHandler.java index e49bf4771b5c..60c1a2a554f1 100644 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/heap/ReferenceHandler.java +++ b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/heap/ReferenceHandler.java @@ -35,20 +35,23 @@ public final class ReferenceHandler { public static boolean useDedicatedThread() { if (ReferenceHandlerThread.isSupported()) { - int optionIndex = IsolateArgumentParser.getOptionIndex(SubstrateOptions.ConcealedOptions.ReferenceHandlerMode); - return IsolateArgumentParser.getIntOptionValue(optionIndex) == ReferenceHandlerMode.UseDedicatedThread; + int useReferenceHandlerThread = IsolateArgumentParser.getOptionIndex(SubstrateOptions.ConcealedOptions.UseReferenceHandlerThread); + int automaticReferenceHandling = IsolateArgumentParser.getOptionIndex(SubstrateOptions.ConcealedOptions.AutomaticReferenceHandling); + return IsolateArgumentParser.getBooleanOptionValue(useReferenceHandlerThread) && IsolateArgumentParser.getBooleanOptionValue(automaticReferenceHandling); } return false; } public static boolean useRegularJavaThread() { - int optionIndex = IsolateArgumentParser.getOptionIndex(SubstrateOptions.ConcealedOptions.ReferenceHandlerMode); - return IsolateArgumentParser.getIntOptionValue(optionIndex) == ReferenceHandlerMode.UseRegularJavaThreads; + int useReferenceHandlerThread = IsolateArgumentParser.getOptionIndex(SubstrateOptions.ConcealedOptions.UseReferenceHandlerThread); + int automaticReferenceHandling = IsolateArgumentParser.getOptionIndex(SubstrateOptions.ConcealedOptions.AutomaticReferenceHandling); + return (!ReferenceHandlerThread.isSupported() || !IsolateArgumentParser.getBooleanOptionValue(useReferenceHandlerThread)) && + IsolateArgumentParser.getBooleanOptionValue(automaticReferenceHandling); } public static boolean isExecutedManually() { - int optionIndex = IsolateArgumentParser.getOptionIndex(SubstrateOptions.ConcealedOptions.ReferenceHandlerMode); - return IsolateArgumentParser.getIntOptionValue(optionIndex) == ReferenceHandlerMode.ExecuteManually; + int automaticReferenceHandling = IsolateArgumentParser.getOptionIndex(SubstrateOptions.ConcealedOptions.AutomaticReferenceHandling); + return !IsolateArgumentParser.getBooleanOptionValue(automaticReferenceHandling); } public static void processPendingReferencesInRegularThread() { diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/heap/ReferenceHandlerMode.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/heap/ReferenceHandlerMode.java deleted file mode 100644 index 9cc9ef43e9ab..000000000000 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/heap/ReferenceHandlerMode.java +++ /dev/null @@ -1,46 +0,0 @@ -/* - * Copyright (c) 2022, 2022, Oracle and/or its affiliates. All rights reserved. - * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. - * - * This code is free software; you can redistribute it and/or modify it - * under the terms of the GNU General Public License version 2 only, as - * published by the Free Software Foundation. Oracle designates this - * particular file as subject to the "Classpath" exception as provided - * by Oracle in the LICENSE file that accompanied this code. - * - * This code is distributed in the hope that it will be useful, but WITHOUT - * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or - * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License - * version 2 for more details (a copy is included in the LICENSE file that - * accompanied this code). - * - * You should have received a copy of the GNU General Public License version - * 2 along with this work; if not, write to the Free Software Foundation, - * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. - * - * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA - * or visit www.oracle.com if you need additional information or have any - * questions. - */ -package com.oracle.svm.core.heap; - -/** - * Values that can be used for the option - * {@link com.oracle.svm.core.SubstrateOptions.ConcealedOptions#ReferenceHandlerMode}. - */ -public abstract class ReferenceHandlerMode { - /** - * Disables automatic reference handling. If reference handling is needed, then it must be done - * manually by calling {@link Heap#doReferenceHandling()}. - */ - public static final int ExecuteManually = 0; - - /** Uses a dedicated thread to do the reference handling. */ - public static final int UseDedicatedThread = 1; - - /** - * Deprecated, will be removed as soon as possible (see GR-36676): does the reference handling - * in the regular Java threads (e.g., at the end of the allocation slow-path). - */ - public static final int UseRegularJavaThreads = 2; -} diff --git a/substratevm/src/com.oracle.svm.graal.hotspot.libgraal/src/com/oracle/svm/graal/hotspot/libgraal/LibGraalEntryPoints.java b/substratevm/src/com.oracle.svm.graal.hotspot.libgraal/src/com/oracle/svm/graal/hotspot/libgraal/LibGraalEntryPoints.java index 90d9a658186a..cc08c31a4358 100644 --- a/substratevm/src/com.oracle.svm.graal.hotspot.libgraal/src/com/oracle/svm/graal/hotspot/libgraal/LibGraalEntryPoints.java +++ b/substratevm/src/com.oracle.svm.graal.hotspot.libgraal/src/com/oracle/svm/graal/hotspot/libgraal/LibGraalEntryPoints.java @@ -32,6 +32,7 @@ import java.util.Arrays; import java.util.Map; +import com.oracle.svm.core.heap.Heap; import org.graalvm.collections.EconomicMap; import org.graalvm.compiler.debug.GlobalMetrics; import org.graalvm.compiler.hotspot.CompilationContext; @@ -248,6 +249,12 @@ private static long compileMethod(PointerBase jniEnv, UNSAFE.putInt(stackTraceAddress, length); UNSAFE.copyMemory(stackTrace, ARRAY_BYTE_BASE_OFFSET, null, stackTraceAddress + Integer.BYTES, length); return 0L; + } finally { + /* + * libgraal doesn't use a dedicated reference handler thread, so we trigger the + * reference handling manually when a compilation finishes. + */ + Heap.getHeap().doReferenceHandling(); } } } diff --git a/substratevm/src/com.oracle.svm.graal/src/com/oracle/svm/graal/isolated/IsolatedGraalUtils.java b/substratevm/src/com.oracle.svm.graal/src/com/oracle/svm/graal/isolated/IsolatedGraalUtils.java index 1ba0f9a261c4..401853a186c4 100644 --- a/substratevm/src/com.oracle.svm.graal/src/com/oracle/svm/graal/isolated/IsolatedGraalUtils.java +++ b/substratevm/src/com.oracle.svm.graal/src/com/oracle/svm/graal/isolated/IsolatedGraalUtils.java @@ -27,7 +27,6 @@ import java.lang.reflect.Array; import java.nio.ByteBuffer; -import com.oracle.svm.core.heap.ReferenceHandlerMode; import org.graalvm.collections.EconomicMap; import org.graalvm.collections.UnmodifiableMapCursor; import org.graalvm.compiler.code.CompilationResult; @@ -68,7 +67,7 @@ public static CompilerIsolateThread createCompilationIsolate() { builder.reservedAddressSpaceSize(WordFactory.signed(addressSpaceSize)); } // Compilation isolates do the reference handling manually to avoid the extra thread. - builder.appendArgument(getOptionString(SubstrateOptions.ConcealedOptions.ReferenceHandlerMode, ReferenceHandlerMode.ExecuteManually)); + builder.appendArgument(getOptionString(SubstrateOptions.ConcealedOptions.AutomaticReferenceHandling, false)); CreateIsolateParameters params = builder.build(); CompilerIsolateThread isolate = (CompilerIsolateThread) Isolates.createIsolate(params); initializeCompilationIsolate(isolate); @@ -193,8 +192,8 @@ public static void applyClientRuntimeOptionValues(PointerBase encodedOptionsPtr, RuntimeOptionValues.singleton().update(options); } - private static String getOptionString(RuntimeOptionKey option, int value) { - return "-XX:" + option.getName() + "=" + value; + private static String getOptionString(RuntimeOptionKey option, boolean value) { + return "-XX:" + (value ? "+" : "-") + option.getName(); } private IsolatedGraalUtils() { From dd7e8b1b872ce57bc63495ae36f40b20bfb002dc Mon Sep 17 00:00:00 2001 From: Christian Haeubl Date: Thu, 3 Feb 2022 12:12:52 +0100 Subject: [PATCH 6/7] Trigger the reference handling after a Truffle compilation finishes in libgraal. --- .../hotspot/libgraal/TruffleToLibGraalEntryPoints.java | 5 +++++ .../src/com/oracle/svm/core/heap/Heap.java | 9 ++++----- .../svm/graal/hotspot/libgraal/LibGraalFeature.java | 9 +++++++++ 3 files changed, 18 insertions(+), 5 deletions(-) diff --git a/compiler/src/org.graalvm.compiler.truffle.compiler.hotspot.libgraal/src/org/graalvm/compiler/truffle/compiler/hotspot/libgraal/TruffleToLibGraalEntryPoints.java b/compiler/src/org.graalvm.compiler.truffle.compiler.hotspot.libgraal/src/org/graalvm/compiler/truffle/compiler/hotspot/libgraal/TruffleToLibGraalEntryPoints.java index 0aec6c904263..a7ce46aace7b 100644 --- a/compiler/src/org.graalvm.compiler.truffle.compiler.hotspot.libgraal/src/org/graalvm/compiler/truffle/compiler/hotspot/libgraal/TruffleToLibGraalEntryPoints.java +++ b/compiler/src/org.graalvm.compiler.truffle.compiler.hotspot.libgraal/src/org/graalvm/compiler/truffle/compiler/hotspot/libgraal/TruffleToLibGraalEntryPoints.java @@ -274,12 +274,17 @@ public static void closeCompilation(JNIEnv env, JClass hsClazz, @CEntryPoint.Iso } finally { compilable.release(env); HSObject.cleanHandles(env); + doReferenceHandling(); } } catch (Throwable t) { JNIExceptionWrapper.throwInHotSpot(env, t); } } + private static void doReferenceHandling() { + // Substituted in LibGraalFeature. + } + @TruffleToLibGraal(Shutdown) @SuppressWarnings({"unused", "try"}) @CEntryPoint(name = "Java_org_graalvm_compiler_truffle_runtime_hotspot_libgraal_TruffleToLibGraalCalls_shutdown") diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/heap/Heap.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/heap/Heap.java index 50df84eac429..9ced149f7351 100644 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/heap/Heap.java +++ b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/heap/Heap.java @@ -38,7 +38,6 @@ import org.graalvm.word.Pointer; import org.graalvm.word.UnsignedWord; -import com.oracle.svm.core.SubstrateOptions; import com.oracle.svm.core.annotate.Uninterruptible; import com.oracle.svm.core.hub.DynamicHub; import com.oracle.svm.core.hub.PredefinedClassesSupport; @@ -200,10 +199,10 @@ public List> getLoadedClasses() { /** * If the automatic reference handling is disabled (see - * {@link SubstrateOptions.ConcealedOptions#AutomaticReferenceHandling}), then this method can - * be called to do the reference handling manually. On execution, the current thread will - * enqueue pending {@link Reference}s into their corresponding {@link ReferenceQueue}s and it - * will execute pending cleaners. + * {@link com.oracle.svm.core.SubstrateOptions.ConcealedOptions#AutomaticReferenceHandling}), + * then this method can be called to do the reference handling manually. On execution, the + * current thread will enqueue pending {@link Reference}s into their corresponding + * {@link ReferenceQueue}s and it will execute pending cleaners. * * This method must not be called from within a VM operation as this could result in deadlocks. * Furthermore, it is up to the caller to ensure that this method is only called in places where diff --git a/substratevm/src/com.oracle.svm.graal.hotspot.libgraal/src/com/oracle/svm/graal/hotspot/libgraal/LibGraalFeature.java b/substratevm/src/com.oracle.svm.graal.hotspot.libgraal/src/com/oracle/svm/graal/hotspot/libgraal/LibGraalFeature.java index 315122eaf2d8..4d6e519450a0 100644 --- a/substratevm/src/com.oracle.svm.graal.hotspot.libgraal/src/com/oracle/svm/graal/hotspot/libgraal/LibGraalFeature.java +++ b/substratevm/src/com.oracle.svm.graal.hotspot.libgraal/src/com/oracle/svm/graal/hotspot/libgraal/LibGraalFeature.java @@ -863,3 +863,12 @@ private static void notifyCrash(String crashMessage) { @Delete("shouldn't appear in libgraal") final class Target_org_graalvm_compiler_hotspot_SymbolicSnippetEncoder { } + +@TargetClass(className = "org.graalvm.compiler.truffle.compiler.hotspot.libgraal.TruffleToLibGraalEntryPoints", onlyWith = LibGraalFeature.IsEnabled.class) +final class Target_org_graalvm_compiler_truffle_compiler_hotspot_libgraal_TruffleToLibGraalEntryPoints { + @SuppressWarnings("unused") + @Substitute + private static void doReferenceHandling() { + Heap.getHeap().doReferenceHandling(); + } +} From 9459b86f919d03449b598e259f91f9c48d156ab4 Mon Sep 17 00:00:00 2001 From: Christian Haeubl Date: Mon, 7 Feb 2022 09:56:32 +0100 Subject: [PATCH 7/7] Fixes after rebasing. --- .../src/com/oracle/svm/core/genscavenge/GCImpl.java | 6 +++--- .../src/com/oracle/svm/core/heap/ReferenceHandler.java | 9 --------- 2 files changed, 3 insertions(+), 12 deletions(-) diff --git a/substratevm/src/com.oracle.svm.core.genscavenge/src/com/oracle/svm/core/genscavenge/GCImpl.java b/substratevm/src/com.oracle.svm.core.genscavenge/src/com/oracle/svm/core/genscavenge/GCImpl.java index 93f3e9567dfb..68f5e386a55b 100644 --- a/substratevm/src/com.oracle.svm.core.genscavenge/src/com/oracle/svm/core/genscavenge/GCImpl.java +++ b/substratevm/src/com.oracle.svm.core.genscavenge/src/com/oracle/svm/core/genscavenge/GCImpl.java @@ -29,7 +29,6 @@ import java.lang.ref.Reference; -import com.oracle.svm.core.thread.JavaThreads; import org.graalvm.compiler.api.replacements.Fold; import org.graalvm.nativeimage.CurrentIsolate; import org.graalvm.nativeimage.IsolateThread; @@ -85,6 +84,7 @@ import com.oracle.svm.core.thread.JavaVMOperation; import com.oracle.svm.core.thread.NativeVMOperation; import com.oracle.svm.core.thread.NativeVMOperationData; +import com.oracle.svm.core.thread.PlatformThreads; import com.oracle.svm.core.thread.VMOperation; import com.oracle.svm.core.thread.VMThreads; import com.oracle.svm.core.util.TimeUtils; @@ -1058,7 +1058,7 @@ private void finishCollection() { // This method will be removed as soon as possible, see GR-36676. static void doReferenceHandlingInRegularThread() { - if (ReferenceHandler.useRegularJavaThread() && !VMOperation.isInProgress() && JavaThreads.currentJavaThreadInitialized()) { + if (ReferenceHandler.useRegularJavaThread() && !VMOperation.isInProgress() && PlatformThreads.isCurrentAssigned()) { doReferenceHandling(); } } @@ -1070,7 +1070,7 @@ static void doReferenceHandlingInRegularThread() { */ static void doReferenceHandling() { assert !VMOperation.isInProgress() : "could result in deadlocks"; - assert JavaThreads.currentJavaThreadInitialized() : "thread is not fully initialized yet"; + assert PlatformThreads.isCurrentAssigned() : "thread is not fully initialized yet"; /* Most of the time, we won't have a pending reference list. So, we do that check first. */ if (HeapImpl.getHeapImpl().hasReferencePendingListUnsafe()) { long startTime = System.nanoTime(); diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/heap/ReferenceHandler.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/heap/ReferenceHandler.java index 60c1a2a554f1..d6d28324446b 100644 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/heap/ReferenceHandler.java +++ b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/heap/ReferenceHandler.java @@ -91,15 +91,6 @@ static void processCleaners() { } } - public static boolean isReferenceHandlingAllowed() { - /* - * Inside a VMOperation, we are not allowed to do certain things, e.g., perform - * synchronization (because it can deadlock when a lock is held outside the VMOperation). - * Similar restrictions apply if we are too early in the attach sequence of a thread. - */ - return !VMOperation.isInProgress() && JavaThreads.currentJavaThreadInitialized(); - } - private ReferenceHandler() { } }