[GR-41839] [GR-41851] [GR-42793] [GR-43145] Various fixes and improvements for JFR.

substratevm/mx.substratevm/suite.py

@@ -620,6 +620,7 @@
             ],
             "requiresConcealed" : {
                 "java.base" : [
+                    "jdk.internal.event",
                     "jdk.internal.misc",
                     "jdk.internal.vm.annotation",
                     "jdk.internal.org.objectweb.asm",

substratevm/src/com.oracle.svm.core.genscavenge/src/com/oracle/svm/core/genscavenge/JfrGCEventSupport.java

@@ -42,7 +42,6 @@
 import com.oracle.svm.core.jfr.JfrNativeEventWriterData;
 import com.oracle.svm.core.jfr.JfrNativeEventWriterDataAccess;
 import com.oracle.svm.core.jfr.JfrTicks;
-import com.oracle.svm.core.jfr.SubstrateJVM;
 import com.oracle.svm.core.util.VMError;
 
 class JfrGCEventSupport {
@@ -66,7 +65,7 @@ public int stopGCPhasePause() {
 
     @Uninterruptible(reason = "Accesses a JFR buffer.")
     public void emitGarbageCollectionEvent(UnsignedWord gcEpoch, GCCause cause, long start) {
-        if (SubstrateJVM.isRecording() && SubstrateJVM.get().isEnabled(JfrEvent.GarbageCollection)) {
+        if (JfrEvent.GarbageCollection.shouldEmit()) {
             long pauseTime = JfrTicks.elapsedTicks() - start;
 
             JfrNativeEventWriterData data = StackValue.get(JfrNativeEventWriterData.class);
@@ -87,7 +86,7 @@ public void emitGarbageCollectionEvent(UnsignedWord gcEpoch, GCCause cause, long
     @Uninterruptible(reason = "Accesses a JFR buffer.")
     public void emitGCPhasePauseEvent(UnsignedWord gcEpoch, int level, String name, long startTicks) {
         JfrEvent event = getGCPhasePauseEvent(level);
-        if (SubstrateJVM.isRecording() && SubstrateJVM.get().isEnabled(event)) {
+        if (event.shouldEmit()) {
             long end = JfrTicks.elapsedTicks();
             JfrNativeEventWriterData data = StackValue.get(JfrNativeEventWriterData.class);
             JfrNativeEventWriterDataAccess.initializeThreadLocalNativeBuffer(data);

substratevm/src/com.oracle.svm.core.posix/src/com/oracle/svm/core/posix/PosixSubstrateSigprofHandler.java

@@ -25,112 +25,165 @@
 
 package com.oracle.svm.core.posix;
 
+import static com.oracle.svm.core.posix.PosixSubstrateSigprofHandler.Options.SignalHandlerBasedExecutionSampler;
+
+import java.util.List;
+
+import org.graalvm.compiler.options.Option;
+import org.graalvm.nativeimage.ImageSingletons;
 import org.graalvm.nativeimage.IsolateThread;
 import org.graalvm.nativeimage.Platform;
 import org.graalvm.nativeimage.Platforms;
 import org.graalvm.nativeimage.StackValue;
 import org.graalvm.nativeimage.c.function.CEntryPoint;
 import org.graalvm.nativeimage.c.function.CEntryPointLiteral;
+import org.graalvm.nativeimage.c.function.CodePointer;
 import org.graalvm.nativeimage.c.struct.SizeOf;
 import org.graalvm.nativeimage.c.type.VoidPointer;
+import org.graalvm.nativeimage.hosted.Feature;
+import org.graalvm.word.Pointer;
 import org.graalvm.word.UnsignedWord;
 import org.graalvm.word.WordFactory;
 
+import com.oracle.svm.core.IsolateListenerSupport;
+import com.oracle.svm.core.IsolateListenerSupportFeature;
+import com.oracle.svm.core.RegisterDumper;
 import com.oracle.svm.core.Uninterruptible;
 import com.oracle.svm.core.c.function.CEntryPointOptions;
-import com.oracle.svm.core.feature.AutomaticallyRegisteredImageSingleton;
+import com.oracle.svm.core.feature.AutomaticallyRegisteredFeature;
+import com.oracle.svm.core.feature.InternalFeature;
 import com.oracle.svm.core.graal.stackvalue.UnsafeStackValue;
 import com.oracle.svm.core.headers.LibC;
 import com.oracle.svm.core.heap.RestrictHeapAccess;
+import com.oracle.svm.core.jfr.JfrFeature;
+import com.oracle.svm.core.jfr.sampler.JfrExecutionSampler;
+import com.oracle.svm.core.option.HostedOptionKey;
 import com.oracle.svm.core.posix.headers.Pthread;
 import com.oracle.svm.core.posix.headers.Signal;
 import com.oracle.svm.core.posix.headers.Time;
 import com.oracle.svm.core.sampler.SubstrateSigprofHandler;
+import com.oracle.svm.core.thread.ThreadListenerSupport;
+import com.oracle.svm.core.util.TimeUtils;
 
-@AutomaticallyRegisteredImageSingleton(SubstrateSigprofHandler.class)
 public class PosixSubstrateSigprofHandler extends SubstrateSigprofHandler {
-
-    public static final long INTERVAL_S = 0;
-    public static final long INTERVAL_uS = 20_000;
+    private static final CEntryPointLiteral<Signal.AdvancedSignalDispatcher> advancedSignalDispatcher = CEntryPointLiteral.create(PosixSubstrateSigprofHandler.class,
+                    "dispatch", int.class, Signal.siginfo_t.class, Signal.ucontext_t.class);
 
     @Platforms(Platform.HOSTED_ONLY.class)
     public PosixSubstrateSigprofHandler() {
     }
 
-    /** The address of the signal handler for signals handled by Java code, below. */
-    private static final CEntryPointLiteral<Signal.AdvancedSignalDispatcher> advancedSignalDispatcher = CEntryPointLiteral.create(PosixSubstrateSigprofHandler.class,
-                    "dispatch", int.class, Signal.siginfo_t.class, Signal.ucontext_t.class);
-
     @SuppressWarnings("unused")
     @CEntryPoint(include = CEntryPoint.NotIncludedAutomatically.class, publishAs = CEntryPoint.Publish.NotPublished)
     @CEntryPointOptions(prologue = CEntryPointOptions.NoPrologue.class, epilogue = CEntryPointOptions.NoEpilogue.class)
     @RestrictHeapAccess(access = RestrictHeapAccess.Access.NO_ALLOCATION, reason = "Must not allocate in sigprof signal handler.")
     @Uninterruptible(reason = "Signal handler may only execute uninterruptible code.")
     private static void dispatch(@SuppressWarnings("unused") int signalNumber, @SuppressWarnings("unused") Signal.siginfo_t sigInfo, Signal.ucontext_t uContext) {
+        /* We need to keep the code in this method to a minimum to avoid races. */
         if (tryEnterIsolate()) {
-            doUninterruptibleStackWalk(uContext);
+            CodePointer ip = (CodePointer) RegisterDumper.singleton().getIP(uContext);
+            Pointer sp = (Pointer) RegisterDumper.singleton().getSP(uContext);
+            tryUninterruptibleStackWalk(ip, sp);
         }
     }
 
-    private static void registerSigprofSignal() {
+    private static void registerSigprofSignal(Signal.AdvancedSignalDispatcher dispatcher) {
         int structSigActionSize = SizeOf.get(Signal.sigaction.class);
         Signal.sigaction structSigAction = UnsafeStackValue.get(structSigActionSize);
         LibC.memset(structSigAction, WordFactory.signed(0), WordFactory.unsigned(structSigActionSize));
 
         /* Register sa_sigaction signal handler */
         structSigAction.sa_flags(Signal.SA_SIGINFO() | Signal.SA_NODEFER() | Signal.SA_RESTART());
-        structSigAction.sa_sigaction(advancedSignalDispatcher.getFunctionPointer());
+        structSigAction.sa_sigaction(dispatcher);
         Signal.sigaction(Signal.SignalEnum.SIGPROF.getCValue(), structSigAction, WordFactory.nullPointer());
     }
 
-    private static int callSetitimer() {
-        /* Call setitimer to start profiling. */
+    @Override
+    protected void updateInterval() {
+        updateInterval(newIntervalMillis);
+    }
+
+    private static void updateInterval(long ms) {
         Time.itimerval newValue = UnsafeStackValue.get(Time.itimerval.class);
-        Time.itimerval oldValue = UnsafeStackValue.get(Time.itimerval.class);
+        newValue.it_value().set_tv_sec(ms / TimeUtils.millisPerSecond);
+        newValue.it_value().set_tv_usec((ms % TimeUtils.millisPerSecond) * 1000);
+        newValue.it_interval().set_tv_sec(ms / TimeUtils.millisPerSecond);
+        newValue.it_interval().set_tv_usec((ms % TimeUtils.millisPerSecond) * 1000);
 
-        newValue.it_value().set_tv_sec(INTERVAL_S);
-        newValue.it_value().set_tv_usec(INTERVAL_uS);
-        newValue.it_interval().set_tv_sec(INTERVAL_S);
-        newValue.it_interval().set_tv_usec(INTERVAL_uS);
+        int status = Time.NoTransitions.setitimer(Time.TimerTypeEnum.ITIMER_PROF, newValue, WordFactory.nullPointer());
+        PosixUtils.checkStatusIs0(status, "setitimer(which, newValue, oldValue): wrong arguments.");
+    }
 
-        return Time.NoTransitions.setitimer(Time.TimerTypeEnum.ITIMER_PROF, newValue, oldValue);
+    @Override
+    protected void installSignalHandler() {
+        registerSigprofSignal(advancedSignalDispatcher.getFunctionPointer());
+        updateInterval();
     }
 
     @Override
-    protected void install0() {
-        registerSigprofSignal();
-        PosixUtils.checkStatusIs0(callSetitimer(), "setitimer(which, newValue, oldValue): wrong arguments.");
+    protected void uninstallSignalHandler() {
+        /*
+         * Only disable the sampling but do not replace the signal handler with the default one
+         * because a signal might be pending for some thread (the default signal handler would print
+         * "Profiling timer expired" to the output).
+         */
+        updateInterval(0);
     }
 
     @Override
     @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true)
-    protected UnsignedWord createThreadLocalKey() {
+    protected UnsignedWord createNativeThreadLocal() {
         Pthread.pthread_key_tPointer key = StackValue.get(Pthread.pthread_key_tPointer.class);
         PosixUtils.checkStatusIs0(Pthread.pthread_key_create(key, WordFactory.nullPointer()), "pthread_key_create(key, keyDestructor): failed.");
         return key.read();
     }
 
     @Override
     @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true)
-    protected void deleteThreadLocalKey(UnsignedWord key) {
+    protected void deleteNativeThreadLocal(UnsignedWord key) {
         int resultCode = Pthread.pthread_key_delete((Pthread.pthread_key_t) key);
         PosixUtils.checkStatusIs0(resultCode, "pthread_key_delete(key): failed.");
     }
 
     @Override
     @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true)
-    protected void setThreadLocalKeyValue(UnsignedWord key, IsolateThread value) {
+    protected void setNativeThreadLocalValue(UnsignedWord key, IsolateThread value) {
         int resultCode = Pthread.pthread_setspecific((Pthread.pthread_key_t) key, (VoidPointer) value);
         PosixUtils.checkStatusIs0(resultCode, "pthread_setspecific(key, value): wrong arguments.");
     }
 
     @Override
     @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true)
-    protected IsolateThread getThreadLocalKeyValue(UnsignedWord key) {
+    protected IsolateThread getNativeThreadLocalValue(UnsignedWord key) {
         /*
          * Although this method is not async-signal-safe in general we rely on
          * implementation-specific behavior here.
          */
         return (IsolateThread) Pthread.pthread_getspecific((Pthread.pthread_key_t) key);
     }
+
+    public static class Options {
+        @Option(help = "Determines if JFR uses a signal handler for execution sampling.")//
+        public static final HostedOptionKey<Boolean> SignalHandlerBasedExecutionSampler = new HostedOptionKey<>(false);
+    }
+}
+
+@AutomaticallyRegisteredFeature
+class PosixSubstrateSigProfHandlerFeature implements InternalFeature {
+    @Override
+    public List<Class<? extends Feature>> getRequiredFeatures() {
+        return List.of(IsolateListenerSupportFeature.class, JfrFeature.class);
+    }
+
+    @Override
+    public void afterRegistration(AfterRegistrationAccess access) {
+        if (JfrFeature.isExecutionSamplerSupported() && Platform.includedIn(Platform.LINUX.class) && SignalHandlerBasedExecutionSampler.getValue()) {
+            SubstrateSigprofHandler sampler = new PosixSubstrateSigprofHandler();
+            ImageSingletons.add(JfrExecutionSampler.class, sampler);
+            ImageSingletons.add(SubstrateSigprofHandler.class, sampler);
+
+            ThreadListenerSupport.get().register(sampler);
+            IsolateListenerSupport.singleton().register(sampler);
+        }
+    }
 }

substratevm/src/com.oracle.svm.core.posix/src/com/oracle/svm/core/posix/thread/PosixPlatformThreads.java

@@ -265,10 +265,7 @@ public OSThreadHandle startThreadUnmanaged(CFunctionPointer threadRoutine, Point
     @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true)
     public boolean joinThreadUnmanaged(OSThreadHandle threadHandle, WordPointer threadExitStatus) {
         int status = Pthread.pthread_join_no_transition((Pthread.pthread_t) threadHandle, threadExitStatus);
-        if (status != 0) {
-            return false;
-        }
-        return true;
+        return status == 0;
     }
 
     @Override

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/JavaMainWrapper.java

@@ -67,9 +67,9 @@
 import com.oracle.svm.core.jdk.InternalVMMethod;
 import com.oracle.svm.core.jdk.RuntimeSupport;
 import com.oracle.svm.core.log.Log;
-import com.oracle.svm.core.sampler.ProfilingSampler;
 import com.oracle.svm.core.thread.JavaThreads;
 import com.oracle.svm.core.thread.PlatformThreads;
+import com.oracle.svm.core.thread.ThreadListenerSupport;
 import com.oracle.svm.core.thread.VMThreads;
 import com.oracle.svm.core.util.Counter;
 import com.oracle.svm.core.util.VMError;
@@ -161,9 +161,7 @@ private static int runCore0() {
                 VMRuntime.initialize();
             }
 
-            if (ImageSingletons.contains(ProfilingSampler.class)) {
-                ImageSingletons.lookup(ProfilingSampler.class).registerSampler();
-            }
+            ThreadListenerSupport.get().beforeThreadRun();
 
             /*
              * Invoke the application's main method. Invoking the main method via a method handle
@@ -182,8 +180,6 @@ private static int runCore0() {
              * HotSpot VM.
              */
             return 1;
-        } finally {
-            PlatformThreads.exit(Thread.currentThread());
         }
     }