Remove internal frames from the top of JFR event stacktraces.

Author: christianhaeubl

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

@@ -85,9 +85,6 @@ public final class VMInspectionOptions {
                     AccumulatingLocatableMultiOptionValue.Strings.buildWithCommaDelimiter(),
                     VMInspectionOptions::validateEnableMonitoringFeatures);
 
-    @Option(help = "Determine whether to trim internal frames from JFR stacktraces (defaults to true).")//
-    public static final HostedOptionKey<Boolean> JfrTrimInternalStackTraces = new HostedOptionKey<>(true, VMInspectionOptions::notSupportedOnWindows);
-
     @Option(help = "Dumps all runtime compiled methods on SIGUSR2.", type = OptionType.User) //
     public static final HostedOptionKey<Boolean> DumpRuntimeCompilationOnSignal = new HostedOptionKey<>(false, VMInspectionOptions::notSupportedOnWindows);
 

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

@@ -32,15 +32,13 @@
 import com.oracle.svm.core.Uninterruptible;
 import com.oracle.svm.core.collections.EnumBitmask;
 import com.oracle.svm.core.thread.JavaThreads;
-import com.oracle.svm.core.VMInspectionOptions;
 
 /**
  * This file contains the VM-level events that Native Image supports on all JDK versions. The event
  * IDs depend on the JDK version (see metadata.xml file) and are computed at image build time.
  */
 public final class JfrEvent {
-    private static final int defaultInternalSkipCount = VMInspectionOptions.JfrTrimInternalStackTraces.getValue() ? 5 : 0;
-    public static final JfrEvent ThreadStart = create("jdk.ThreadStart", defaultInternalSkipCount - 1);
+    public static final JfrEvent ThreadStart = create("jdk.ThreadStart", 4);
     public static final JfrEvent ThreadEnd = create("jdk.ThreadEnd");
     public static final JfrEvent ThreadCPULoad = create("jdk.ThreadCPULoad");
     public static final JfrEvent DataLoss = create("jdk.DataLoss");
@@ -62,17 +60,17 @@ public final class JfrEvent {
     public static final JfrEvent SafepointBegin = create("jdk.SafepointBegin", JfrEventFlags.HasDuration);
     public static final JfrEvent SafepointEnd = create("jdk.SafepointEnd", JfrEventFlags.HasDuration);
     public static final JfrEvent ExecuteVMOperation = create("jdk.ExecuteVMOperation", JfrEventFlags.HasDuration);
-    public static final JfrEvent JavaMonitorEnter = create("jdk.JavaMonitorEnter", JfrEventFlags.HasDuration);
-    public static final JfrEvent ThreadPark = create("jdk.ThreadPark", JfrEventFlags.HasDuration);
-    public static final JfrEvent JavaMonitorWait = create("jdk.JavaMonitorWait", JfrEventFlags.HasDuration);
-    public static final JfrEvent JavaMonitorInflate = create("jdk.JavaMonitorInflate", JfrEventFlags.HasDuration);
-    public static final JfrEvent ObjectAllocationInNewTLAB = create("jdk.ObjectAllocationInNewTLAB");
+    public static final JfrEvent JavaMonitorEnter = create("jdk.JavaMonitorEnter", 5, JfrEventFlags.HasDuration);
+    public static final JfrEvent ThreadPark = create("jdk.ThreadPark", 5, JfrEventFlags.HasDuration);
+    public static final JfrEvent JavaMonitorWait = create("jdk.JavaMonitorWait", 5, JfrEventFlags.HasDuration);
+    public static final JfrEvent JavaMonitorInflate = create("jdk.JavaMonitorInflate", 5, JfrEventFlags.HasDuration);
+    public static final JfrEvent ObjectAllocationInNewTLAB = create("jdk.ObjectAllocationInNewTLAB", 5);
     public static final JfrEvent GCHeapSummary = create("jdk.GCHeapSummary");
     public static final JfrEvent ThreadAllocationStatistics = create("jdk.ThreadAllocationStatistics");
-    public static final JfrEvent SystemGC = create("jdk.SystemGC", JfrEventFlags.HasDuration);
-    public static final JfrEvent AllocationRequiringGC = create("jdk.AllocationRequiringGC");
+    public static final JfrEvent SystemGC = create("jdk.SystemGC", 5, JfrEventFlags.HasDuration);
+    public static final JfrEvent AllocationRequiringGC = create("jdk.AllocationRequiringGC", 5);
     public static final JfrEvent OldObjectSample = create("jdk.OldObjectSample", 7);
-    public static final JfrEvent ObjectAllocationSample = create("jdk.ObjectAllocationSample", JfrEventFlags.SupportsThrottling);
+    public static final JfrEvent ObjectAllocationSample = create("jdk.ObjectAllocationSample", 5, JfrEventFlags.SupportsThrottling);
     public static final JfrEvent NativeMemoryUsage = create("jdk.NativeMemoryUsage");
     public static final JfrEvent NativeMemoryUsageTotal = create("jdk.NativeMemoryUsageTotal");
 
@@ -82,12 +80,12 @@ public final class JfrEvent {
     private final int skipCount;
 
     @Platforms(Platform.HOSTED_ONLY.class)
-    public static JfrEvent create(String name, JfrEventFlags... flags) {
-        return new JfrEvent(name, defaultInternalSkipCount, flags);
+    private static JfrEvent create(String name, JfrEventFlags... flags) {
+        return new JfrEvent(name, -1, flags);
     }
 
     @Platforms(Platform.HOSTED_ONLY.class)
-    public static JfrEvent create(String name, int skipCount, JfrEventFlags... flags) {
+    private static JfrEvent create(String name, int skipCount, JfrEventFlags... flags) {
         return new JfrEvent(name, skipCount, flags);
     }
 
@@ -106,6 +104,7 @@ public long getId() {
 
     @Uninterruptible(reason = CALLED_FROM_UNINTERRUPTIBLE_CODE, mayBeInlined = true)
     public int getSkipCount() {
+        assert skipCount >= 0 : "method may only be called for events that need a stack trace";
         return skipCount;
     }
 

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jfr/events/JavaMonitorInflateEvent.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;
@@ -39,6 +38,8 @@
 import com.oracle.svm.core.jfr.SubstrateJVM;
 import com.oracle.svm.core.monitor.MonitorInflationCause;
 
+import jdk.graal.compiler.word.Word;
+
 public class JavaMonitorInflateEvent {
     public static void emit(Object obj, long startTicks, MonitorInflationCause cause) {
         if (HasJfrSupport.get()) {
@@ -47,7 +48,7 @@ public static void emit(Object obj, long startTicks, MonitorInflationCause cause
     }
 
     @Uninterruptible(reason = "Accesses a JFR buffer.")
-    public static void emit0(Object obj, long startTicks, MonitorInflationCause cause) {
+    private static void emit0(Object obj, long startTicks, MonitorInflationCause cause) {
         long duration = JfrTicks.duration(startTicks);
         if (JfrEvent.JavaMonitorInflate.shouldEmit(duration)) {
             JfrNativeEventWriterData data = StackValue.get(JfrNativeEventWriterData.class);

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/sampler/SamplerBuffersAccess.java

@@ -115,7 +115,7 @@ private static void serializeStackTraces(SamplerBuffer rawStackTraceBuffer) {
             int sampleSize = current.readInt(0);
             current = current.add(Integer.BYTES);
 
-            /* Sample size, excluding the header and the end marker. */
+            /* Number of vframes that should be skipped at the top of the stack trace. */
             int skipCount = current.readInt(0);
             current = current.add(Integer.BYTES);
 

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/sampler/SamplerJfrStackTraceSerializer.java

@@ -25,7 +25,8 @@
 
 package com.oracle.svm.core.sampler;
 
-import jdk.graal.compiler.word.Word;
+import static com.oracle.svm.core.Uninterruptible.CALLED_FROM_UNINTERRUPTIBLE_CODE;
+
 import org.graalvm.nativeimage.StackValue;
 import org.graalvm.nativeimage.c.function.CodePointer;
 import org.graalvm.nativeimage.c.type.CIntPointer;
@@ -49,19 +50,18 @@
 import com.oracle.svm.core.jfr.events.ExecutionSampleEvent;
 import com.oracle.svm.core.util.VMError;
 
+import jdk.graal.compiler.word.Word;
+
 /**
  * A concrete implementation of {@link SamplerStackTraceSerializer} designed for JFR stack trace
  * serialization.
+ *
+ * All static mutable state in this class is preallocated and reused by multiple threads. This is
+ * safe because only one thread at a time may serialize stack traces.
  */
 public final class SamplerJfrStackTraceSerializer implements SamplerStackTraceSerializer {
-    /** This value is used by multiple threads but only by a single thread at a time. */
     private static final CodeInfoDecoder.FrameInfoCursor FRAME_INFO_CURSOR = new CodeInfoDecoder.FrameInfoCursor();
-
-    /*
-     * This is static so that a single instance can be preallocated and reused. Only one thread ever
-     * serializes at a given time.
-     */
-    private static final FrameCountData FRAME_COUNT_DATA = new FrameCountData();
+    private static final StackTraceVisitorData VISITOR_DATA = new StackTraceVisitorData();
 
     @Override
     @Uninterruptible(reason = "Prevent JFR recording and epoch change.")
@@ -117,20 +117,19 @@ private static boolean serializeStackTrace(Pointer rawStackTrace, int sampleSize
         /*
          * One IP may correspond to multiple Java-level stack frames. We need to precompute the
          * number of stack trace elements because the count can't be patched later on
-         * (JfrNativeEventWriter.putInt() would not necessarily reserve enough bytes).
-         *
-         * The first pass-through also sets FRAME_COUNT_DATA.isTruncated().
+         * (JfrNativeEventWriter.putInt() would not necessarily reserve enough bytes). We also
+         * precompute if the stack trace was truncated.
          */
-        int numStackTraceElements = visitRawStackTrace(rawStackTrace, sampleSize, Word.nullPointer(), skipCount);
-        if (numStackTraceElements == 0) {
+        visitRawStackTrace(rawStackTrace, sampleSize, Word.nullPointer(), skipCount);
+        if (VISITOR_DATA.getUsedFrames() == 0) {
             return false;
         }
 
         JfrNativeEventWriterData data = StackValue.get(JfrNativeEventWriterData.class);
         JfrNativeEventWriterDataAccess.initialize(data, targetBuffer);
         JfrNativeEventWriter.putLong(data, stackTraceId);
-        JfrNativeEventWriter.putBoolean(data, isTruncated || FRAME_COUNT_DATA.isTruncated());
-        JfrNativeEventWriter.putInt(data, numStackTraceElements);
+        JfrNativeEventWriter.putBoolean(data, isTruncated || VISITOR_DATA.isTruncated());
+        JfrNativeEventWriter.putInt(data, VISITOR_DATA.getUsedFrames());
         visitRawStackTrace(rawStackTrace, sampleSize, data, skipCount);
         boolean success = JfrNativeEventWriter.commit(data);
 
@@ -140,24 +139,20 @@ private static boolean serializeStackTrace(Pointer rawStackTrace, int sampleSize
     }
 
     @Uninterruptible(reason = "Prevent JFR recording and epoch change.")
-    private static int visitRawStackTrace(Pointer rawStackTrace, int sampleSize, JfrNativeEventWriterData data, int skipCount) {
-        int numStackTraceElements = 0;
-        Pointer rawStackTraceEnd = rawStackTrace.add(sampleSize);
-        Pointer ipPtr = rawStackTrace;
-
-        // Reset FrameCountData before every serialization of a new stacktrace.
-        FRAME_COUNT_DATA.reset(skipCount);
+    private static void visitRawStackTrace(Pointer rawStackTrace, int sampleSize, JfrNativeEventWriterData data, int skipCount) {
+        VISITOR_DATA.reset(skipCount);
 
+        Pointer ipPtr = rawStackTrace;
+        Pointer rawStackTraceEnd = rawStackTrace.add(sampleSize);
         while (ipPtr.belowThan(rawStackTraceEnd)) {
             long ip = ipPtr.readLong(0);
-            numStackTraceElements += visitFrame(data, ip);
+            visitFrame(data, ip);
             ipPtr = ipPtr.add(Long.BYTES);
         }
-        return numStackTraceElements;
     }
 
     @Uninterruptible(reason = "Prevent JFR recording, epoch change, and that the GC frees the CodeInfo.")
-    private static int visitFrame(JfrNativeEventWriterData data, long address) {
+    private static void visitFrame(JfrNativeEventWriterData data, long address) {
         CodePointer ip = Word.pointer(address);
         UntetheredCodeInfo untetheredInfo = CodeInfoTable.lookupCodeInfo(ip);
         if (untetheredInfo.isNull()) {
@@ -168,32 +163,30 @@ private static int visitFrame(JfrNativeEventWriterData data, long address) {
         Object tether = CodeInfoAccess.acquireTether(untetheredInfo);
         try {
             CodeInfo tetheredCodeInfo = CodeInfoAccess.convert(untetheredInfo, tether);
-            return visitFrame(data, tetheredCodeInfo, ip);
+            visitFrame(data, tetheredCodeInfo, ip);
         } finally {
             CodeInfoAccess.releaseTether(untetheredInfo, tether);
         }
     }
 
     @Uninterruptible(reason = "Prevent JFR recording and epoch change.")
-    private static int visitFrame(JfrNativeEventWriterData data, CodeInfo codeInfo, CodePointer ip) {
-        int numStackTraceElements = 0;
+    private static void visitFrame(JfrNativeEventWriterData data, CodeInfo codeInfo, CodePointer ip) {
         FRAME_INFO_CURSOR.initialize(codeInfo, ip, false);
         while (FRAME_INFO_CURSOR.advance()) {
-            if (FRAME_COUNT_DATA.shouldSkip()) {
-                FRAME_COUNT_DATA.incrementSkipped();
+            if (VISITOR_DATA.shouldSkipFrame()) {
+                VISITOR_DATA.incrementSkippedFrames();
                 continue;
-            } else if (FRAME_COUNT_DATA.shouldTruncate()) {
-                FRAME_COUNT_DATA.setTruncated();
+            } else if (VISITOR_DATA.shouldTruncate()) {
+                VISITOR_DATA.setTruncated();
                 break;
             }
+
+            VISITOR_DATA.incrementUsedFrames();
             if (data.isNonNull()) {
                 FrameInfoQueryResult frame = FRAME_INFO_CURSOR.get();
                 serializeStackTraceElement(data, frame);
             }
-            FRAME_COUNT_DATA.incrementTotal();
-            numStackTraceElements++;
         }
-        return numStackTraceElements;
     }
 
     @Uninterruptible(reason = "Prevent JFR recording and epoch change.")
@@ -206,48 +199,53 @@ private static void serializeStackTraceElement(JfrNativeEventWriterData data, Fr
         JfrNativeEventWriter.putLong(data, JfrFrameType.FRAME_AOT_COMPILED.getId());
     }
 
-    private static final class FrameCountData {
-        private int skipcount;
-        private int totalCount;
-        private int skippedCount;
+    private static final class StackTraceVisitorData {
+        private int framesToSkip;
+        private int skippedFrames;
+        private int usedFrames;
         private boolean truncated;
 
-        @Uninterruptible(reason = Uninterruptible.CALLED_FROM_UNINTERRUPTIBLE_CODE, mayBeInlined = true)
+        @Uninterruptible(reason = CALLED_FROM_UNINTERRUPTIBLE_CODE, mayBeInlined = true)
         public void reset(int skipCount) {
-            this.skipcount = skipCount;
-            totalCount = 0;
-            skippedCount = 0;
+            framesToSkip = skipCount;
+            skippedFrames = 0;
+            usedFrames = 0;
             truncated = false;
         }
 
-        @Uninterruptible(reason = Uninterruptible.CALLED_FROM_UNINTERRUPTIBLE_CODE, mayBeInlined = true) //
-        public boolean shouldSkip() {
-            return skippedCount < skipcount;
+        @Uninterruptible(reason = CALLED_FROM_UNINTERRUPTIBLE_CODE, mayBeInlined = true) //
+        public boolean shouldSkipFrame() {
+            return skippedFrames < framesToSkip;
+        }
+
+        @Uninterruptible(reason = CALLED_FROM_UNINTERRUPTIBLE_CODE, mayBeInlined = true)
+        public void incrementSkippedFrames() {
+            skippedFrames++;
         }
 
-        @Uninterruptible(reason = Uninterruptible.CALLED_FROM_UNINTERRUPTIBLE_CODE, mayBeInlined = true) //
+        @Uninterruptible(reason = CALLED_FROM_UNINTERRUPTIBLE_CODE, mayBeInlined = true) //
         public boolean shouldTruncate() {
-            return totalCount > SubstrateJVM.getStackTraceRepo().getStackTraceDepth();
+            return usedFrames >= SubstrateJVM.getStackTraceRepo().getStackTraceDepth();
         }
 
-        @Uninterruptible(reason = Uninterruptible.CALLED_FROM_UNINTERRUPTIBLE_CODE, mayBeInlined = true)
+        @Uninterruptible(reason = CALLED_FROM_UNINTERRUPTIBLE_CODE, mayBeInlined = true)
         public void setTruncated() {
             truncated = true;
         }
 
-        @Uninterruptible(reason = Uninterruptible.CALLED_FROM_UNINTERRUPTIBLE_CODE, mayBeInlined = true)
+        @Uninterruptible(reason = CALLED_FROM_UNINTERRUPTIBLE_CODE, mayBeInlined = true)
         public boolean isTruncated() {
             return truncated;
         }
 
-        @Uninterruptible(reason = Uninterruptible.CALLED_FROM_UNINTERRUPTIBLE_CODE, mayBeInlined = true)
-        public void incrementSkipped() {
-            skippedCount++;
+        @Uninterruptible(reason = CALLED_FROM_UNINTERRUPTIBLE_CODE, mayBeInlined = true)
+        public void incrementUsedFrames() {
+            usedFrames++;
         }
 
-        @Uninterruptible(reason = Uninterruptible.CALLED_FROM_UNINTERRUPTIBLE_CODE, mayBeInlined = true)
-        public void incrementTotal() {
-            totalCount++;
+        @Uninterruptible(reason = CALLED_FROM_UNINTERRUPTIBLE_CODE, mayBeInlined = true)
+        public int getUsedFrames() {
+            return usedFrames;
         }
     }
 }

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/sampler/SamplerSampleWriter.java

@@ -25,8 +25,6 @@
 
 package com.oracle.svm.core.sampler;
 
-import jdk.graal.compiler.api.replacements.Fold;
-import jdk.graal.compiler.word.Word;
 import org.graalvm.word.Pointer;
 import org.graalvm.word.UnsignedWord;
 
@@ -37,6 +35,9 @@
 import com.oracle.svm.core.jfr.JfrTicks;
 import com.oracle.svm.core.jfr.SubstrateJVM;
 
+import jdk.graal.compiler.api.replacements.Fold;
+import jdk.graal.compiler.word.Word;
+
 public final class SamplerSampleWriter {
     public static final long JFR_STACK_TRACE_END = -1;
     public static final long EXECUTION_SAMPLE_END = -2;
@@ -62,9 +63,8 @@ public static void begin(SamplerSampleWriterData data) {
         SamplerSampleWriter.putInt(data, 0);
         /* Sample size. (will be patched later) */
         SamplerSampleWriter.putInt(data, 0);
-        /* Skipcount will be used later during serialization. */
-        SamplerSampleWriter.putInt(data, data.getSkipCount());
 
+        SamplerSampleWriter.putInt(data, data.getSkipCount());
         SamplerSampleWriter.putLong(data, JfrTicks.elapsedTicks());
         SamplerSampleWriter.putLong(data, SubstrateJVM.getCurrentThreadId());
         SamplerSampleWriter.putLong(data, JfrThreadState.getId(Thread.State.RUNNABLE));