From 55496ef97802128ffe35dd6971c213bc2e4babaa Mon Sep 17 00:00:00 2001 From: Christian Haeubl Date: Tue, 4 Feb 2025 12:47:25 +0100 Subject: [PATCH 1/5] Remove SystemPropertiesSupport.tmpdirValue() and javaLibraryPathValue(). --- .../svm/core/jdk/SystemPropertiesSupport.java | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jdk/SystemPropertiesSupport.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jdk/SystemPropertiesSupport.java index da10e7654bb3..6c78e2da84c6 100644 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jdk/SystemPropertiesSupport.java +++ b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jdk/SystemPropertiesSupport.java @@ -335,20 +335,9 @@ private synchronized void initializeProperty(LazySystemProperty property) { protected abstract String osVersionValue(); - protected String javaIoTmpdirValue() { - return tmpdirValue(); - } - - /* Should be removed, see GR-61420. */ - protected String tmpdirValue() { - throw VMError.shouldNotReachHere("Subclasses must either implement javaIoTmpdirValue() or tmpdirValue()."); - } + protected abstract String javaIoTmpdirValue(); - /* Should be removed, see GR-61420. */ - protected String javaLibraryPathValue() { - /* Fallback for platforms that don't implement this method. */ - return ""; - } + protected abstract String javaLibraryPathValue(); private static class LazySystemProperty { private final String key; From 471af76d2a5b9616663ccd376529ffab9fdfe237 Mon Sep 17 00:00:00 2001 From: Christian Haeubl Date: Tue, 4 Feb 2025 12:52:26 +0100 Subject: [PATCH 2/5] Remove pre-allocated OutOfMemoryErrors in HeapChunkProvider. --- .../svm/core/genscavenge/AlignedHeapChunk.java | 1 + .../svm/core/genscavenge/HeapChunkProvider.java | 16 ++-------------- .../svm/core/genscavenge/UnalignedHeapChunk.java | 1 + .../os/ChunkBasedCommittedMemoryProvider.java | 4 +++- 4 files changed, 7 insertions(+), 15 deletions(-) diff --git a/substratevm/src/com.oracle.svm.core.genscavenge/src/com/oracle/svm/core/genscavenge/AlignedHeapChunk.java b/substratevm/src/com.oracle.svm.core.genscavenge/src/com/oracle/svm/core/genscavenge/AlignedHeapChunk.java index 8b0da4008afd..e62acc1e462f 100644 --- a/substratevm/src/com.oracle.svm.core.genscavenge/src/com/oracle/svm/core/genscavenge/AlignedHeapChunk.java +++ b/substratevm/src/com.oracle.svm.core.genscavenge/src/com/oracle/svm/core/genscavenge/AlignedHeapChunk.java @@ -85,6 +85,7 @@ public interface AlignedHeader extends HeapChunk.Header { @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true) public static void initialize(AlignedHeader chunk, UnsignedWord chunkSize) { + assert chunk.isNonNull(); assert chunkSize.equal(HeapParameters.getAlignedHeapChunkSize()) : "expecting all aligned chunks to be the same size"; HeapChunk.initialize(chunk, AlignedHeapChunk.getObjectsStart(chunk), chunkSize); chunk.setShouldSweepInsteadOfCompact(false); diff --git a/substratevm/src/com.oracle.svm.core.genscavenge/src/com/oracle/svm/core/genscavenge/HeapChunkProvider.java b/substratevm/src/com.oracle.svm.core.genscavenge/src/com/oracle/svm/core/genscavenge/HeapChunkProvider.java index c242551a780e..fa6abfd0dd6b 100644 --- a/substratevm/src/com.oracle.svm.core.genscavenge/src/com/oracle/svm/core/genscavenge/HeapChunkProvider.java +++ b/substratevm/src/com.oracle.svm.core.genscavenge/src/com/oracle/svm/core/genscavenge/HeapChunkProvider.java @@ -24,7 +24,6 @@ */ package com.oracle.svm.core.genscavenge; -import jdk.graal.compiler.word.Word; import org.graalvm.nativeimage.Platform; import org.graalvm.nativeimage.Platforms; import org.graalvm.word.Pointer; @@ -36,7 +35,6 @@ import com.oracle.svm.core.genscavenge.AlignedHeapChunk.AlignedHeader; import com.oracle.svm.core.genscavenge.HeapChunk.Header; import com.oracle.svm.core.genscavenge.UnalignedHeapChunk.UnalignedHeader; -import com.oracle.svm.core.heap.OutOfMemoryUtil; import com.oracle.svm.core.jdk.UninterruptibleUtils; import com.oracle.svm.core.jdk.UninterruptibleUtils.AtomicUnsigned; import com.oracle.svm.core.log.Log; @@ -45,6 +43,8 @@ import com.oracle.svm.core.thread.VMThreads; import com.oracle.svm.core.util.UnsignedUtils; +import jdk.graal.compiler.word.Word; + /** * Allocates and frees the memory for aligned and unaligned heap chunks. The methods are * thread-safe, so no locking is necessary when calling them. @@ -54,10 +54,6 @@ * list. Memory for unaligned chunks is released immediately. */ final class HeapChunkProvider { - /** These {@link OutOfMemoryError}s are only needed for legacy code, see GR-59639. */ - private static final OutOfMemoryError ALIGNED_OUT_OF_MEMORY_ERROR = new OutOfMemoryError("Could not allocate an aligned heap chunk"); - private static final OutOfMemoryError UNALIGNED_OUT_OF_MEMORY_ERROR = new OutOfMemoryError("Could not allocate an unaligned heap chunk"); - /** * The head of the linked list of unused aligned chunks. Chunks are chained using * {@link HeapChunk#getNext}. @@ -90,10 +86,6 @@ AlignedHeader produceAlignedChunk() { if (result.isNull()) { /* Unused list was empty, need to allocate memory. */ result = (AlignedHeader) ChunkBasedCommittedMemoryProvider.get().allocateAlignedChunk(chunkSize, HeapParameters.getAlignedHeapChunkAlignment()); - if (result.isNull()) { - throw OutOfMemoryUtil.reportOutOfMemoryError(ALIGNED_OUT_OF_MEMORY_ERROR); - } - AlignedHeapChunk.initialize(result, chunkSize); } assert HeapChunk.getTopOffset(result).equal(AlignedHeapChunk.getObjectsStartOffset()); @@ -241,10 +233,6 @@ UnalignedHeader produceUnalignedChunk(UnsignedWord objectSize) { UnsignedWord chunkSize = UnalignedHeapChunk.getChunkSizeForObject(objectSize); UnalignedHeader result = (UnalignedHeader) ChunkBasedCommittedMemoryProvider.get().allocateUnalignedChunk(chunkSize); - if (result.isNull()) { - throw OutOfMemoryUtil.reportOutOfMemoryError(UNALIGNED_OUT_OF_MEMORY_ERROR); - } - UnalignedHeapChunk.initialize(result, chunkSize); assert objectSize.belowOrEqual(HeapChunk.availableObjectMemory(result)) : "UnalignedHeapChunk insufficient for requested object"; diff --git a/substratevm/src/com.oracle.svm.core.genscavenge/src/com/oracle/svm/core/genscavenge/UnalignedHeapChunk.java b/substratevm/src/com.oracle.svm.core.genscavenge/src/com/oracle/svm/core/genscavenge/UnalignedHeapChunk.java index e0825c0f56d5..b46e32edbf4c 100644 --- a/substratevm/src/com.oracle.svm.core.genscavenge/src/com/oracle/svm/core/genscavenge/UnalignedHeapChunk.java +++ b/substratevm/src/com.oracle.svm.core.genscavenge/src/com/oracle/svm/core/genscavenge/UnalignedHeapChunk.java @@ -89,6 +89,7 @@ public interface UnalignedHeader extends HeapChunk.Header { } public static void initialize(UnalignedHeader chunk, UnsignedWord chunkSize) { + assert chunk.isNonNull(); HeapChunk.initialize(chunk, UnalignedHeapChunk.getObjectStart(chunk), chunkSize); } diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/os/ChunkBasedCommittedMemoryProvider.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/os/ChunkBasedCommittedMemoryProvider.java index 77ef55d28e94..369d027a1ccc 100644 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/os/ChunkBasedCommittedMemoryProvider.java +++ b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/os/ChunkBasedCommittedMemoryProvider.java @@ -24,7 +24,6 @@ */ package com.oracle.svm.core.os; -import jdk.graal.compiler.word.Word; import org.graalvm.nativeimage.ImageSingletons; import org.graalvm.word.Pointer; import org.graalvm.word.PointerBase; @@ -37,6 +36,7 @@ import com.oracle.svm.core.thread.VMOperation; import jdk.graal.compiler.api.replacements.Fold; +import jdk.graal.compiler.word.Word; public abstract class ChunkBasedCommittedMemoryProvider extends AbstractCommittedMemoryProvider { private static final OutOfMemoryError ALIGNED_OUT_OF_MEMORY_ERROR = new OutOfMemoryError("Could not allocate an aligned heap chunk. " + @@ -49,6 +49,7 @@ public static ChunkBasedCommittedMemoryProvider get() { return (ChunkBasedCommittedMemoryProvider) ImageSingletons.lookup(CommittedMemoryProvider.class); } + /** Returns a non-null value or throws a pre-allocated exception. */ @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true) public Pointer allocateAlignedChunk(UnsignedWord nbytes, UnsignedWord alignment) { Pointer result = allocate(nbytes, alignment, false, NmtCategory.JavaHeap); @@ -58,6 +59,7 @@ public Pointer allocateAlignedChunk(UnsignedWord nbytes, UnsignedWord alignment) return result; } + /** Returns a non-null value or throws a pre-allocated exception. */ public Pointer allocateUnalignedChunk(UnsignedWord nbytes) { Pointer result = allocate(nbytes, getAlignmentForUnalignedChunks(), false, NmtCategory.JavaHeap); if (result.isNull()) { From 1674bdd5210ea0199108016041f4640b24a6249d Mon Sep 17 00:00:00 2001 From: Christian Haeubl Date: Tue, 4 Feb 2025 13:44:41 +0100 Subject: [PATCH 3/5] Temporarily disable a few checks regarding @Uninterruptible(callerMustBe = true). --- .../svm/hosted/code/UninterruptibleAnnotationChecker.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/code/UninterruptibleAnnotationChecker.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/code/UninterruptibleAnnotationChecker.java index f5ccebf38230..10f4621de57e 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/code/UninterruptibleAnnotationChecker.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/code/UninterruptibleAnnotationChecker.java @@ -184,7 +184,9 @@ private void checkOverrides(HostedMethod method, Uninterruptible methodAnnotatio Uninterruptible implAnnotation = Uninterruptible.Utils.getAnnotation(impl); if (implAnnotation != null) { if (methodAnnotation.callerMustBe() != implAnnotation.callerMustBe()) { - violations.add("callerMustBe: " + method.format("%H.%n(%p):%r") + " != " + impl.format("%H.%n(%p):%r")); + // GR-45784: temporarily disabled so that we can remove legacy code + // violations.add("callerMustBe: " + method.format("%H.%n(%p):%r") + " != " + + // impl.format("%H.%n(%p):%r")); } if (methodAnnotation.calleeMustBe() != implAnnotation.calleeMustBe()) { violations.add("calleeMustBe: " + method.format("%H.%n(%p):%r") + " != " + impl.format("%H.%n(%p):%r")); From e86d09e40e069f126ef437253ebdc86f5818766a Mon Sep 17 00:00:00 2001 From: Christian Haeubl Date: Mon, 11 Nov 2024 17:58:43 +0100 Subject: [PATCH 4/5] Annotate VMMutex.unlockNoTransitionUnspecifiedOwner() with @Uninterruptible(callerMustBe = true). --- .../oracle/svm/core/posix/pthread/PthreadVMLockSupport.java | 2 +- .../com/oracle/svm/core/windows/WindowsVMLockSupport.java | 2 +- .../src/com/oracle/svm/core/locks/VMMutex.java | 6 +----- 3 files changed, 3 insertions(+), 7 deletions(-) diff --git a/substratevm/src/com.oracle.svm.core.posix/src/com/oracle/svm/core/posix/pthread/PthreadVMLockSupport.java b/substratevm/src/com.oracle.svm.core.posix/src/com/oracle/svm/core/posix/pthread/PthreadVMLockSupport.java index e2a48f6ef6ef..612b5753b375 100644 --- a/substratevm/src/com.oracle.svm.core.posix/src/com/oracle/svm/core/posix/pthread/PthreadVMLockSupport.java +++ b/substratevm/src/com.oracle.svm.core.posix/src/com/oracle/svm/core/posix/pthread/PthreadVMLockSupport.java @@ -142,7 +142,7 @@ public void unlock() { } @Override - @Uninterruptible(reason = "Whole critical section needs to be uninterruptible.") + @Uninterruptible(reason = "Whole critical section needs to be uninterruptible.", callerMustBe = true) public void unlockNoTransitionUnspecifiedOwner() { clearUnspecifiedOwner(); PthreadVMLockSupport.checkResult(Pthread.pthread_mutex_unlock(getStructPointer()), "pthread_mutex_unlock"); diff --git a/substratevm/src/com.oracle.svm.core.windows/src/com/oracle/svm/core/windows/WindowsVMLockSupport.java b/substratevm/src/com.oracle.svm.core.windows/src/com/oracle/svm/core/windows/WindowsVMLockSupport.java index f6957a1b47a6..3df7f11fb602 100644 --- a/substratevm/src/com.oracle.svm.core.windows/src/com/oracle/svm/core/windows/WindowsVMLockSupport.java +++ b/substratevm/src/com.oracle.svm.core.windows/src/com/oracle/svm/core/windows/WindowsVMLockSupport.java @@ -153,7 +153,7 @@ public void unlock() { } @Override - @Uninterruptible(reason = "Whole critical section needs to be uninterruptible.") + @Uninterruptible(reason = "Whole critical section needs to be uninterruptible.", callerMustBe = true) public void unlockNoTransitionUnspecifiedOwner() { clearUnspecifiedOwner(); Process.NoTransitions.LeaveCriticalSection(getStructPointer()); diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/locks/VMMutex.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/locks/VMMutex.java index 903b5f5f3a69..09fa86da8396 100644 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/locks/VMMutex.java +++ b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/locks/VMMutex.java @@ -125,12 +125,8 @@ public void unlock() { * Like {@linkplain #unlock()}. Only use this method if the lock was acquired via * {@linkplain #lockNoTransitionUnspecifiedOwner()}. */ - @Uninterruptible(reason = "Whole critical section needs to be uninterruptible.") + @Uninterruptible(reason = "Whole critical section needs to be uninterruptible.", callerMustBe = true) public void unlockNoTransitionUnspecifiedOwner() { - /* - * Ideally, this method would be annotated with @Uninterruptible(callerMustBe = true) but - * this isn't possible because of legacy code, see GR-45784. - */ throw VMError.shouldNotReachHere("Lock cannot be used during native image generation"); } From 5aa4840d251bcc2f43629a6a26f94fc5a3c2ba84 Mon Sep 17 00:00:00 2001 From: Christian Haeubl Date: Tue, 4 Feb 2025 14:23:11 +0100 Subject: [PATCH 5/5] Fix a transient failure in TestThreadCPULoadEvent. --- .../svm/test/jfr/TestThreadCPULoadEvent.java | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/substratevm/src/com.oracle.svm.test/src/com/oracle/svm/test/jfr/TestThreadCPULoadEvent.java b/substratevm/src/com.oracle.svm.test/src/com/oracle/svm/test/jfr/TestThreadCPULoadEvent.java index 2e57f07c3677..7b8846c54e64 100644 --- a/substratevm/src/com.oracle.svm.test/src/com/oracle/svm/test/jfr/TestThreadCPULoadEvent.java +++ b/substratevm/src/com.oracle.svm.test/src/com/oracle/svm/test/jfr/TestThreadCPULoadEvent.java @@ -34,6 +34,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.concurrent.CountDownLatch; import org.junit.Assert; import org.junit.Test; @@ -45,7 +46,7 @@ import jdk.jfr.consumer.RecordedEvent; public class TestThreadCPULoadEvent extends JfrRecordingTest { - private static final int TIMEOUT = 10000; + private static final int TIMEOUT = 30000; private static final String THREAD_NAME_1 = "Thread-1"; private static final String THREAD_NAME_2 = "Thread-2"; private static final String THREAD_NAME_3 = "Thread-3"; @@ -55,9 +56,13 @@ public void test() throws Throwable { String[] events = new String[]{JfrEvent.ThreadCPULoad.getName()}; Recording recording = startRecording(events); - WeakReference thread1 = createAndStartBusyWaitThread(THREAD_NAME_1, 10, 250); - WeakReference thread2 = createAndStartBusyWaitThread(THREAD_NAME_2, 250, 10); - Thread thread3 = createAndStartBusyWaitThread(THREAD_NAME_3, 20, TIMEOUT).get(); + CountDownLatch threadsStarted = new CountDownLatch(3); + WeakReference thread1 = new WeakReference<>(createAndStartBusyWaitThread(THREAD_NAME_1, 10, 250, threadsStarted)); + WeakReference thread2 = new WeakReference<>(createAndStartBusyWaitThread(THREAD_NAME_2, 250, 10, threadsStarted)); + Thread thread3 = createAndStartBusyWaitThread(THREAD_NAME_3, 20, TIMEOUT, threadsStarted); + + /* Wait until all threads are started. */ + threadsStarted.await(); /* For threads 1 and 2, the event is emitted when the thread exits. */ waitUntilCollected(thread1); @@ -68,6 +73,7 @@ public void test() throws Throwable { Assert.assertTrue(thread3.isAlive()); thread3.interrupt(); + thread3.join(); } private static void validateEvents(List events) { @@ -88,14 +94,15 @@ private static void validateEvents(List events) { assertTrue(cpuTimes.get(THREAD_NAME_1) < cpuTimes.get(THREAD_NAME_2)); } - private static WeakReference createAndStartBusyWaitThread(String name, int busyMs, int idleMs) { + private static Thread createAndStartBusyWaitThread(String name, int busyMs, int idleMs, CountDownLatch threadsStarted) { Thread thread = new Thread(() -> { + threadsStarted.countDown(); busyWait(busyMs); sleep(idleMs); }); thread.setName(name); thread.start(); - return new WeakReference<>(thread); + return thread; } private static void busyWait(long waitMs) {