Skip unnecessary JFR registrations for virtual threads.

Author: christianhaeubl

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jfr/JfrNativeEventWriter.java

@@ -24,7 +24,6 @@
  */
 package com.oracle.svm.core.jfr;
 
-import jdk.graal.compiler.word.Word;
 import org.graalvm.word.Pointer;
 import org.graalvm.word.UnsignedWord;
 
@@ -35,6 +34,8 @@
 import com.oracle.svm.core.util.DuplicatedInNativeCode;
 import com.oracle.svm.core.util.VMError;
 
+import jdk.graal.compiler.word.Word;
+
 /**
  * A JFR event writer that does not allocate any objects in the Java heap. Can only be used from
  * {@link Uninterruptible} code to prevent races between threads that try to write a native JFR
@@ -250,14 +251,12 @@ public static void putThread(JfrNativeEventWriterData data, Thread thread) {
         if (thread == null) {
             putThread(data, 0L);
         } else {
-            SubstrateJVM.maybeRegisterVirtualThread(thread);
             putThread(data, SubstrateJVM.getThreadId(thread));
         }
     }
 
     @Uninterruptible(reason = "Accesses a native JFR buffer.", callerMustBe = true)
     public static void putThread(JfrNativeEventWriterData data, long threadId) {
-        SubstrateJVM.maybeRegisterVirtualThread(threadId);
         putLong(data, threadId);
     }
 

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jfr/JfrThreadRepository.java

@@ -24,9 +24,6 @@
  */
 package com.oracle.svm.core.jfr;
 
-import static com.oracle.svm.core.Uninterruptible.CALLED_FROM_UNINTERRUPTIBLE_CODE;
-import jdk.graal.compiler.word.Word;
-
 import org.graalvm.nativeimage.IsolateThread;
 import org.graalvm.nativeimage.Platform;
 import org.graalvm.nativeimage.Platforms;
@@ -42,9 +39,12 @@
 import com.oracle.svm.core.thread.JavaThreads;
 import com.oracle.svm.core.thread.PlatformThreads;
 import com.oracle.svm.core.thread.Target_java_lang_Thread;
+import com.oracle.svm.core.thread.Target_java_lang_VirtualThread;
 import com.oracle.svm.core.thread.VMOperation;
 import com.oracle.svm.core.thread.VMThreads;
 
+import jdk.graal.compiler.word.Word;
+
 /**
  * Repository that collects all metadata about threads and thread groups.
  *
@@ -94,37 +94,32 @@ public void registerRunningThreads() {
             Thread thread = PlatformThreads.fromVMThread(isolateThread);
             if (thread != null) {
                 registerThread(thread);
+                // Re-register vthreads that are already mounted.
+                Thread vthread = PlatformThreads.getMountedVirtualThread(thread);
+                if (vthread != null) {
+                    registerThread(vthread);
+                }
             }
         }
     }
 
-    /**
-     * If this method is called on platform threads, nothing will happen since they are already
-     * registered eagerly upon starting and at epoch changes.
-     */
-    @Uninterruptible(reason = "Locking without transition requires that the whole critical section is uninterruptible.")
-    public void registerThread(long threadId) {
+    @Uninterruptible(reason = "Prevent epoch changes. Prevent races with VM operations that start/stop recording.")
+    public void registerThread(Thread thread) {
         if (!SubstrateJVM.get().isRecording()) {
             return;
         }
 
-        registerThread0(threadId, 0, true, null, null);
-    }
-
-    @Uninterruptible(reason = "Locking without transition requires that the whole critical section is uninterruptible.")
-    public void registerThread(Thread thread) {
-        if (!SubstrateJVM.get().isRecording()) {
+        boolean isVirtual = JavaThreads.isVirtual(thread);
+        if (isVirtual && isVirtualThreadAlreadyRegistered(thread)) {
             return;
         }
 
-        long threadId = JavaThreads.getThreadId(thread);
-        boolean isVirtual = JavaThreads.isVirtual(thread);
-        long osThreadId = isVirtual ? 0 : threadId;
-        registerThread0(threadId, osThreadId, isVirtual, thread, thread.getName());
+        registerThread0(thread, isVirtual);
     }
 
-    @Uninterruptible(reason = CALLED_FROM_UNINTERRUPTIBLE_CODE, mayBeInlined = true)
-    private void registerThread0(long threadId, long osThreadId, boolean isVirtual, Thread thread, String name) {
+    @Uninterruptible(reason = "Locking without transition requires that the whole critical section is uninterruptible.")
+    private void registerThread0(Thread thread, boolean isVirtual) {
+        long threadId = JavaThreads.getThreadId(thread);
         JfrVisited visitedThread = StackValue.get(JfrVisited.class);
         visitedThread.setId(threadId);
         visitedThread.setHash(UninterruptibleUtils.Long.hashCode(threadId));
@@ -145,7 +140,9 @@ private void registerThread0(long threadId, long osThreadId, boolean isVirtual,
             JfrNativeEventWriterDataAccess.initialize(data, epochData.threadBuffer);
 
             /* Similar to JfrThreadConstant::serialize in HotSpot. */
+            long osThreadId = isVirtual ? 0 : threadId;
             long threadGroupId = registerThreadGroup(thread, isVirtual);
+            String name = thread.getName();
 
             JfrNativeEventWriter.putLong(data, threadId);
             JfrNativeEventWriter.putString(data, name); // OS thread name
@@ -158,6 +155,11 @@ private void registerThread0(long threadId, long osThreadId, boolean isVirtual,
                 return;
             }
 
+            if (isVirtual) {
+                Target_java_lang_VirtualThread vthread = JavaThreads.toVirtualTarget(thread);
+                vthread.jfrEpochId = JfrTraceIdEpoch.getInstance().currentEpochId();
+            }
+
             epochData.unflushedThreadCount++;
             /* The buffer may have been replaced with a new one. */
             epochData.threadBuffer = data.getJfrBuffer();
@@ -166,6 +168,16 @@ private void registerThread0(long threadId, long osThreadId, boolean isVirtual,
         }
     }
 
+    @Uninterruptible(reason = "Epoch must not change while in this method.", callerMustBe = true)
+    private static boolean isVirtualThreadAlreadyRegistered(Thread thread) {
+        assert JavaThreads.isVirtual(thread);
+
+        /* Threads only need to be registered once per epoch. */
+        Target_java_lang_VirtualThread vthread = JavaThreads.toVirtualTarget(thread);
+        long epochId = JfrTraceIdEpoch.getInstance().currentEpochId();
+        return vthread.jfrEpochId == epochId;
+    }
+
     @Uninterruptible(reason = "Epoch must not change while in this method.")
     private long registerThreadGroup(Thread thread, boolean isVirtual) {
         if (isVirtual) {

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jfr/SubstrateJVM.java

@@ -26,7 +26,6 @@
 
 import java.util.List;
 
-import com.oracle.svm.core.SubstrateUtil;
 import org.graalvm.nativeimage.ImageSingletons;
 import org.graalvm.nativeimage.IsolateThread;
 import org.graalvm.nativeimage.Platform;
@@ -42,15 +41,13 @@
 import com.oracle.svm.core.jfr.oldobject.JfrOldObjectRepository;
 import com.oracle.svm.core.jfr.sampler.JfrExecutionSampler;
 import com.oracle.svm.core.jfr.throttling.JfrEventThrottling;
-import com.oracle.svm.core.jfr.traceid.JfrTraceIdEpoch;
 import com.oracle.svm.core.log.Log;
 import com.oracle.svm.core.sampler.SamplerBufferPool;
 import com.oracle.svm.core.sampler.SamplerBuffersAccess;
 import com.oracle.svm.core.sampler.SamplerStatistics;
 import com.oracle.svm.core.sampler.SubstrateSigprofHandler;
 import com.oracle.svm.core.thread.JavaThreads;
 import com.oracle.svm.core.thread.JavaVMOperation;
-import com.oracle.svm.core.thread.Target_java_lang_VirtualThread;
 import com.oracle.svm.core.thread.VMThreads;
 import com.oracle.svm.core.util.VMError;
 
@@ -320,32 +317,6 @@ public static long getCurrentThreadId() {
         return 0;
     }
 
-    /**
-     * Register virtual threads if they aren't registered already. Platform threads are registered
-     * eagerly when started and at chunk rotations.
-     */
-    @Uninterruptible(reason = "Epoch should not change while checking generation.")
-    public static void maybeRegisterVirtualThread(Thread thread) {
-        // Do quick preliminary checks to avoid global locking unless necessary.
-        if (JavaThreads.isVirtual(thread)) {
-            Target_java_lang_VirtualThread tjlv = SubstrateUtil.cast(thread, Target_java_lang_VirtualThread.class);
-            int currentEpochGen = JfrTraceIdEpoch.getInstance().currentEpochGeneration();
-            if (tjlv.jfrGeneration != currentEpochGen) {
-                getThreadRepo().registerThread(thread);
-                tjlv.jfrGeneration = currentEpochGen;
-            }
-        }
-    }
-
-    /**
-     * {@link SubstrateJVM#maybeRegisterVirtualThread(Thread)} Is preferred over this method since
-     * it can perform preliminary checks to avoid locking the Thread Repository unnecessarily.
-     */
-    @Uninterruptible(reason = "Epoch should not change while checking generation.")
-    public static void maybeRegisterVirtualThread(long tid) {
-        getThreadRepo().registerThread(tid);
-    }
-
     /**
      * See {@link JVM#storeMetadataDescriptor}.
      */

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jfr/events/JavaMonitorEnterEvent.java

@@ -26,7 +26,6 @@
 
 package com.oracle.svm.core.jfr.events;
 
-import jdk.graal.compiler.word.Word;
 import org.graalvm.nativeimage.StackValue;
 
 import com.oracle.svm.core.Uninterruptible;
@@ -38,15 +37,17 @@
 import com.oracle.svm.core.jfr.JfrTicks;
 import com.oracle.svm.core.jfr.SubstrateJVM;
 
+import jdk.graal.compiler.word.Word;
+
 public class JavaMonitorEnterEvent {
-    public static void emit(Object obj, Thread previousOwner, long startTicks) {
+    public static void emit(Object obj, long previousOwnerTid, long startTicks) {
         if (HasJfrSupport.get()) {
-            emit0(obj, previousOwner, startTicks);
+            emit0(obj, previousOwnerTid, startTicks);
         }
     }
 
     @Uninterruptible(reason = "Accesses a JFR buffer.")
-    public static void emit0(Object obj, Thread previousOwner, long startTicks) {
+    public static void emit0(Object obj, long previousOwnerTid, long startTicks) {
         long duration = JfrTicks.duration(startTicks);
         if (JfrEvent.JavaMonitorEnter.shouldEmit(duration)) {
             JfrNativeEventWriterData data = StackValue.get(JfrNativeEventWriterData.class);
@@ -58,7 +59,7 @@ public static void emit0(Object obj, Thread previousOwner, long startTicks) {
             JfrNativeEventWriter.putEventThread(data);
             JfrNativeEventWriter.putLong(data, SubstrateJVM.get().getStackTraceId(JfrEvent.JavaMonitorEnter, 0));
             JfrNativeEventWriter.putClass(data, obj.getClass());
-            JfrNativeEventWriter.putThread(data, previousOwner);
+            JfrNativeEventWriter.putThread(data, previousOwnerTid);
             JfrNativeEventWriter.putLong(data, Word.objectToUntrackedPointer(obj).rawValue());
             JfrNativeEventWriter.endSmallEvent(data);
         }

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jfr/events/JavaMonitorWaitEvent.java

@@ -40,15 +40,15 @@
 import jdk.graal.compiler.word.Word;
 
 public class JavaMonitorWaitEvent {
-    public static void emit(long startTicks, Object obj, Thread notifier, long timeout, boolean timedOut) {
+    public static void emit(long startTicks, Object obj, long notifierTid, long timeout, boolean timedOut) {
         if (HasJfrSupport.get() && obj != null && !Target_jdk_jfr_internal_JVM_ChunkRotationMonitor.class.equals(obj.getClass()) &&
                         !Target_jdk_jfr_internal_management_HiddenWait.class.equals(obj.getClass())) {
-            emit0(startTicks, obj, notifier, timeout, timedOut);
+            emit0(startTicks, obj, notifierTid, timeout, timedOut);
         }
     }
 
     @Uninterruptible(reason = "Accesses a JFR buffer.")
-    private static void emit0(long startTicks, Object obj, Thread notifier, long timeout, boolean timedOut) {
+    private static void emit0(long startTicks, Object obj, long notifierTid, long timeout, boolean timedOut) {
         long duration = JfrTicks.duration(startTicks);
         if (JfrEvent.JavaMonitorWait.shouldEmit(duration)) {
             JfrNativeEventWriterData data = org.graalvm.nativeimage.StackValue.get(JfrNativeEventWriterData.class);
@@ -60,7 +60,7 @@ private static void emit0(long startTicks, Object obj, Thread notifier, long tim
             JfrNativeEventWriter.putEventThread(data);
             JfrNativeEventWriter.putLong(data, SubstrateJVM.get().getStackTraceId(JfrEvent.JavaMonitorWait, 0));
             JfrNativeEventWriter.putClass(data, obj.getClass());
-            JfrNativeEventWriter.putThread(data, notifier);
+            JfrNativeEventWriter.putThread(data, notifierTid);
             JfrNativeEventWriter.putLong(data, timeout);
             JfrNativeEventWriter.putBoolean(data, timedOut);
             JfrNativeEventWriter.putLong(data, Word.objectToUntrackedPointer(obj).rawValue());

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jfr/traceid/JfrTraceIdEpoch.java

@@ -25,14 +25,15 @@
 
 package com.oracle.svm.core.jfr.traceid;
 
-import jdk.graal.compiler.api.replacements.Fold;
 import org.graalvm.nativeimage.ImageSingletons;
 import org.graalvm.nativeimage.Platform;
 import org.graalvm.nativeimage.Platforms;
 
 import com.oracle.svm.core.Uninterruptible;
 import com.oracle.svm.core.thread.VMOperation;
 
+import jdk.graal.compiler.api.replacements.Fold;
+
 /**
  * Class holding the current JFR epoch. JFR uses an epoch system to safely separate constant pool
  * entries between adjacent chunks. Used to get the current or previous epoch and switch from one
@@ -42,7 +43,7 @@ public class JfrTraceIdEpoch {
     private static final long EPOCH_0_BIT = 0b01;
     private static final long EPOCH_1_BIT = 0b10;
 
-    private int epochGeneration;
+    private long epochId;
 
     @Fold
     public static JfrTraceIdEpoch getInstance() {
@@ -53,15 +54,10 @@ public static JfrTraceIdEpoch getInstance() {
     public JfrTraceIdEpoch() {
     }
 
-    @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true)
-    private boolean getEpoch() {
-        return (epochGeneration & 1) == 0;
-    }
-
     @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true)
     public void changeEpoch() {
         assert VMOperation.isInProgressAtSafepoint();
-        epochGeneration++;
+        epochId++;
     }
 
     @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true)
@@ -79,13 +75,18 @@ public boolean currentEpoch() {
         return getEpoch();
     }
 
-    @Uninterruptible(reason = "Avoid epoch changing while checking generation.", callerMustBe = true)
-    public int currentEpochGeneration() {
-        return epochGeneration;
-    }
-
     @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true)
     public boolean previousEpoch() {
         return !getEpoch();
     }
+
+    @Uninterruptible(reason = "Prevent epoch from changing.", callerMustBe = true)
+    public long currentEpochId() {
+        return epochId;
+    }
+
+    @Uninterruptible(reason = "Prevent epoch from changing.", callerMustBe = true)
+    private boolean getEpoch() {
+        return (epochId & 1) == 0;
+    }
 }

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/monitor/JavaMonitor.java

@@ -36,6 +36,7 @@
 
 import com.oracle.svm.core.Uninterruptible;
 import com.oracle.svm.core.jfr.JfrTicks;
+import com.oracle.svm.core.jfr.SubstrateJVM;
 import com.oracle.svm.core.jfr.events.JavaMonitorEnterEvent;
 import com.oracle.svm.core.thread.JavaThreads;
 import com.oracle.svm.core.util.BasedOnJDKClass;
@@ -60,20 +61,20 @@
 @BasedOnJDKClass(ReentrantLock.class)
 @BasedOnJDKClass(value = ReentrantLock.class, innerClass = "Sync")
 public class JavaMonitor extends JavaMonitorQueuedSynchronizer {
-    protected Thread latest;
+    protected long latestJfrTid;
 
     public JavaMonitor() {
-        latest = null;
+        latestJfrTid = 0;
     }
 
     public void monitorEnter(Object obj) {
         if (!tryLock()) {
             long startTicks = JfrTicks.elapsedTicks();
             acquire(1);
-            JavaMonitorEnterEvent.emit(obj, latest, startTicks);
+            JavaMonitorEnterEvent.emit(obj, latestJfrTid, startTicks);
         }
 
-        latest = Thread.currentThread();
+        latestJfrTid = SubstrateJVM.getCurrentThreadId();
     }
 
     public void monitorExit() {