fix skipCount to apply to virtual frames.

Author: roberttoyonaga

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

@@ -39,7 +39,8 @@
  * IDs depend on the JDK version (see metadata.xml file) and are computed at image build time.
  */
 public final class JfrEvent {
-    public static final JfrEvent ThreadStart = create("jdk.ThreadStart");
+    private static final int defaultInternalSkipCount = VMInspectionOptions.JfrTrimInternalStackTraces.getValue() ? 5 : 0;
+    public static final JfrEvent ThreadStart = create("jdk.ThreadStart", defaultInternalSkipCount - 1);
     public static final JfrEvent ThreadEnd = create("jdk.ThreadEnd");
     public static final JfrEvent ThreadCPULoad = create("jdk.ThreadCPULoad");
     public static final JfrEvent DataLoss = create("jdk.DataLoss");
@@ -70,25 +71,32 @@ public final class JfrEvent {
     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 OldObjectSample = create("jdk.OldObjectSample");
+    public static final JfrEvent OldObjectSample = create("jdk.OldObjectSample", 7);
     public static final JfrEvent ObjectAllocationSample = create("jdk.ObjectAllocationSample", JfrEventFlags.SupportsThrottling);
     public static final JfrEvent NativeMemoryUsage = create("jdk.NativeMemoryUsage");
     public static final JfrEvent NativeMemoryUsageTotal = create("jdk.NativeMemoryUsageTotal");
 
     private final long id;
     private final String name;
     private final int flags;
+    private final int skipCount;
 
     @Platforms(Platform.HOSTED_ONLY.class)
     public static JfrEvent create(String name, JfrEventFlags... flags) {
-        return new JfrEvent(name, flags);
+        return new JfrEvent(name, defaultInternalSkipCount, flags);
     }
 
     @Platforms(Platform.HOSTED_ONLY.class)
-    private JfrEvent(String name, JfrEventFlags... flags) {
+    public static JfrEvent create(String name, int skipCount, JfrEventFlags... flags) {
+        return new JfrEvent(name, skipCount, flags);
+    }
+
+    @Platforms(Platform.HOSTED_ONLY.class)
+    private JfrEvent(String name, int skipCount, JfrEventFlags... flags) {
         this.id = JfrMetadataTypeLibrary.lookupPlatformEvent(name);
         this.name = name;
         this.flags = EnumBitmask.computeBitmask(flags);
+        this.skipCount = skipCount;
     }
 
     @Uninterruptible(reason = CALLED_FROM_UNINTERRUPTIBLE_CODE, mayBeInlined = true)
@@ -98,7 +106,7 @@ public long getId() {
 
     @Uninterruptible(reason = CALLED_FROM_UNINTERRUPTIBLE_CODE, mayBeInlined = true)
     public int getSkipCount() {
-        return VMInspectionOptions.JfrTrimInternalStackTraces.getValue() ? 3 : 0;
+        return skipCount;
     }
 
     @Uninterruptible(reason = CALLED_FROM_UNINTERRUPTIBLE_CODE, mayBeInlined = true)

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

@@ -272,9 +272,7 @@ private static int recordIp(SamplerSampleWriterData data, CodePointer ip) {
         /* Increment the number of seen frames. */
         data.setSeenFrames(data.getSeenFrames() + 1);
 
-        if (shouldSkipFrame(data)) {
-            return NO_ERROR;
-        } else if (shouldTruncate(data)) {
+        if (shouldTruncate(data)) {
             return TRUNCATED;
         }
 
@@ -288,15 +286,10 @@ private static int recordIp(SamplerSampleWriterData data, CodePointer ip) {
         return BUFFER_SIZE_EXCEEDED;
     }
 
-    @Uninterruptible(reason = CALLED_FROM_UNINTERRUPTIBLE_CODE, mayBeInlined = true)
-    private static boolean shouldSkipFrame(SamplerSampleWriterData data) {
-        return data.getSeenFrames() <= data.getSkipCount();
-    }
-
     @Uninterruptible(reason = CALLED_FROM_UNINTERRUPTIBLE_CODE, mayBeInlined = true)
     private static boolean shouldTruncate(SamplerSampleWriterData data) {
-        int numFrames = data.getSeenFrames() - data.getSkipCount();
-        if (numFrames > data.getMaxDepth()) {
+        int maxFrames = data.getMaxDepth() + data.getSkipCount();
+        if (data.getSeenFrames() > maxFrames) {
             /* The stack size exceeds given depth. Stop walk! */
             data.setTruncated(true);
             return true;

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

@@ -115,7 +115,8 @@ private static void serializeStackTraces(SamplerBuffer rawStackTraceBuffer) {
             int sampleSize = current.readInt(0);
             current = current.add(Integer.BYTES);
 
-            /* Padding. */
+            /* Sample size, excluding the header and the end marker. */
+            int skipCount = current.readInt(0);
             current = current.add(Integer.BYTES);
 
             /* Tick. */
@@ -132,16 +133,16 @@ private static void serializeStackTraces(SamplerBuffer rawStackTraceBuffer) {
 
             assert current.subtract(entryStart).equal(SamplerSampleWriter.getHeaderSize());
 
-            current = serializeStackTrace(current, end, sampleSize, sampleHash, isTruncated, sampleTick, threadId, threadState);
+            current = serializeStackTrace(current, end, sampleSize, sampleHash, isTruncated, sampleTick, threadId, threadState, skipCount);
         }
 
         SamplerBufferAccess.reinitialize(rawStackTraceBuffer);
     }
 
     @Uninterruptible(reason = "Wraps the call to the possibly interruptible serializer.", calleeMustBe = false)
     private static Pointer serializeStackTrace(Pointer rawStackTrace, Pointer bufferEnd, int sampleSize, int sampleHash,
-                    boolean isTruncated, long sampleTick, long threadId, long threadState) {
+                    boolean isTruncated, long sampleTick, long threadId, long threadState, int skipCount) {
         return SamplerStackTraceSerializer.singleton().serializeStackTrace(rawStackTrace, bufferEnd, sampleSize,
-                        sampleHash, isTruncated, sampleTick, threadId, threadState);
+                        sampleHash, isTruncated, sampleTick, threadId, threadState, skipCount);
     }
 }

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

@@ -57,10 +57,16 @@ public final class SamplerJfrStackTraceSerializer implements SamplerStackTraceSe
     /** 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();
+
     @Override
     @Uninterruptible(reason = "Prevent JFR recording and epoch change.")
     public Pointer serializeStackTrace(Pointer rawStackTrace, Pointer bufferEnd, int sampleSize, int sampleHash,
-                    boolean isTruncated, long sampleTick, long threadId, long threadState) {
+                    boolean isTruncated, long sampleTick, long threadId, long threadState, int skipCount) {
         Pointer current = rawStackTrace;
         CIntPointer statusPtr = StackValue.get(CIntPointer.class);
         JfrStackTraceRepository.JfrStackTraceTableEntry entry = SubstrateJVM.getStackTraceRepo().getOrPutStackTrace(current, Word.unsigned(sampleSize), sampleHash, statusPtr);
@@ -70,7 +76,7 @@ public Pointer serializeStackTrace(Pointer rawStackTrace, Pointer bufferEnd, int
         if (status == JfrStackTraceRepository.JfrStackTraceTableEntryStatus.INSERTED || status == JfrStackTraceRepository.JfrStackTraceTableEntryStatus.EXISTING_RAW) {
             /* Walk the IPs and serialize the stacktrace. */
             assert current.add(sampleSize).belowThan(bufferEnd);
-            boolean serialized = serializeStackTrace(current, sampleSize, isTruncated, stackTraceId);
+            boolean serialized = serializeStackTrace(current, sampleSize, isTruncated, stackTraceId, skipCount);
             if (serialized) {
                 SubstrateJVM.getStackTraceRepo().commitSerializedStackTrace(entry);
             }
@@ -100,7 +106,7 @@ public Pointer serializeStackTrace(Pointer rawStackTrace, Pointer bufferEnd, int
     }
 
     @Uninterruptible(reason = "Prevent JFR recording and epoch change.")
-    private static boolean serializeStackTrace(Pointer rawStackTrace, int sampleSize, boolean isTruncated, long stackTraceId) {
+    private static boolean serializeStackTrace(Pointer rawStackTrace, int sampleSize, boolean isTruncated, long stackTraceId, int skipCount) {
         assert sampleSize % Long.BYTES == 0;
 
         JfrBuffer targetBuffer = SubstrateJVM.getStackTraceRepo().getCurrentBuffer();
@@ -112,18 +118,20 @@ 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().
          */
-        int numStackTraceElements = visitRawStackTrace(rawStackTrace, sampleSize, Word.nullPointer());
+        int numStackTraceElements = visitRawStackTrace(rawStackTrace, sampleSize, Word.nullPointer(), skipCount);
         if (numStackTraceElements == 0) {
             return false;
         }
 
         JfrNativeEventWriterData data = StackValue.get(JfrNativeEventWriterData.class);
         JfrNativeEventWriterDataAccess.initialize(data, targetBuffer);
         JfrNativeEventWriter.putLong(data, stackTraceId);
-        JfrNativeEventWriter.putBoolean(data, isTruncated);
+        JfrNativeEventWriter.putBoolean(data, isTruncated || FRAME_COUNT_DATA.isTruncated());
         JfrNativeEventWriter.putInt(data, numStackTraceElements);
-        visitRawStackTrace(rawStackTrace, sampleSize, data);
+        visitRawStackTrace(rawStackTrace, sampleSize, data, skipCount);
         boolean success = JfrNativeEventWriter.commit(data);
 
         /* Buffer can get replaced with a larger one. */
@@ -132,10 +140,14 @@ 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) {
+    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);
+
         while (ipPtr.belowThan(rawStackTraceEnd)) {
             long ip = ipPtr.readLong(0);
             numStackTraceElements += visitFrame(data, ip);
@@ -167,10 +179,18 @@ private static int visitFrame(JfrNativeEventWriterData data, CodeInfo codeInfo,
         int numStackTraceElements = 0;
         FRAME_INFO_CURSOR.initialize(codeInfo, ip, false);
         while (FRAME_INFO_CURSOR.advance()) {
+            if (FRAME_COUNT_DATA.shouldSkip()) {
+                FRAME_COUNT_DATA.incrementSkipped();
+                continue;
+            } else if (FRAME_COUNT_DATA.shouldTruncate()) {
+                FRAME_COUNT_DATA.setTruncated();
+                break;
+            }
             if (data.isNonNull()) {
                 FrameInfoQueryResult frame = FRAME_INFO_CURSOR.get();
                 serializeStackTraceElement(data, frame);
             }
+            FRAME_COUNT_DATA.incrementTotal();
             numStackTraceElements++;
         }
         return numStackTraceElements;
@@ -185,4 +205,49 @@ private static void serializeStackTraceElement(JfrNativeEventWriterData data, Fr
         JfrNativeEventWriter.putInt(data, stackTraceElement.getBci());
         JfrNativeEventWriter.putLong(data, JfrFrameType.FRAME_AOT_COMPILED.getId());
     }
+
+    private static final class FrameCountData {
+        private int skipcount;
+        private int totalCount;
+        private int skippedCount;
+        private boolean truncated;
+
+        @Uninterruptible(reason = Uninterruptible.CALLED_FROM_UNINTERRUPTIBLE_CODE, mayBeInlined = true)
+        public void reset(int skipCount) {
+            this.skipcount = skipCount;
+            totalCount = 0;
+            skippedCount = 0;
+            truncated = false;
+        }
+
+        @Uninterruptible(reason = Uninterruptible.CALLED_FROM_UNINTERRUPTIBLE_CODE, mayBeInlined = true) //
+        public boolean shouldSkip() {
+            return skippedCount < skipcount;
+        }
+
+        @Uninterruptible(reason = Uninterruptible.CALLED_FROM_UNINTERRUPTIBLE_CODE, mayBeInlined = true) //
+        public boolean shouldTruncate() {
+            return totalCount > SubstrateJVM.getStackTraceRepo().getStackTraceDepth();
+        }
+
+        @Uninterruptible(reason = Uninterruptible.CALLED_FROM_UNINTERRUPTIBLE_CODE, mayBeInlined = true)
+        public void setTruncated() {
+            truncated = true;
+        }
+
+        @Uninterruptible(reason = Uninterruptible.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 = Uninterruptible.CALLED_FROM_UNINTERRUPTIBLE_CODE, mayBeInlined = true)
+        public void incrementTotal() {
+            totalCount++;
+        }
+    }
 }

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

@@ -62,8 +62,8 @@ public static void begin(SamplerSampleWriterData data) {
         SamplerSampleWriter.putInt(data, 0);
         /* Sample size. (will be patched later) */
         SamplerSampleWriter.putInt(data, 0);
-        /* Padding so that the long values below are aligned. */
-        SamplerSampleWriter.putInt(data, 0);
+        /* Skipcount will be used later during serialization. */
+        SamplerSampleWriter.putInt(data, data.getSkipCount());
 
         SamplerSampleWriter.putLong(data, JfrTicks.elapsedTicks());
         SamplerSampleWriter.putLong(data, SubstrateJVM.getCurrentThreadId());

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

@@ -52,8 +52,9 @@ static SamplerStackTraceSerializer singleton() {
      * @param sampleTick The timestamp of the sample.
      * @param threadId A unique identifier for the sampled thread.
      * @param threadState The state of the sampled thread.
+     * @param skipCount The number of top frames to omit.
      * @return A pointer to the next stack trace entry or the end of the buffer.
      */
     Pointer serializeStackTrace(Pointer rawStackTrace, Pointer bufferEnd, int sampleSize, int sampleHash,
-                    boolean isTruncated, long sampleTick, long threadId, long threadState);
+                    boolean isTruncated, long sampleTick, long threadId, long threadState, int skipCount);
 }

substratevm/src/com.oracle.svm.test/src/com/oracle/svm/test/jfr/TestAllocationRequiringGCEvent.java

@@ -55,7 +55,7 @@ public void test() throws Throwable {
     private static void validateEvents(List<RecordedEvent> events) {
         assertTrue(events.size() > 0);
         for (RecordedEvent event : events) {
-            checkStackTraceTrimming(event, "emit");
+            checkStackTraceTrimming(event, "maybeCollectOnAllocation");
         }
     }
 

substratevm/src/com.oracle.svm.test/src/com/oracle/svm/test/jfr/TestJavaMonitorEnterEvent.java

@@ -91,7 +91,7 @@ private void validateEvents(List<RecordedEvent> events) {
                 break;
             }
 
-            checkStackTraceTrimming(event, "emit");
+            checkStackTraceTrimming(event, "monitorEnter");
         }
         assertTrue("Expected monitor blocked event not found", found);
     }

substratevm/src/com.oracle.svm.test/src/com/oracle/svm/test/jfr/TestJavaMonitorInflateEvent.java

@@ -91,7 +91,7 @@ private void validateEvents(List<RecordedEvent> events) {
                 foundCauseEnter = true;
             }
 
-            checkStackTraceTrimming(event, "emit");
+            checkStackTraceTrimming(event, "monitorEnter");
         }
         assertTrue("Expected monitor inflate event not found.", foundCauseEnter);
     }

substratevm/src/com.oracle.svm.test/src/com/oracle/svm/test/jfr/TestJavaMonitorWaitEvent.java

@@ -114,7 +114,7 @@ private void validateEvents(List<RecordedEvent> events) {
             }
             lastEventThreadName = eventThread;
 
-            checkStackTraceTrimming(event, "emit");
+            checkStackTraceTrimming(event, "await");
         }
         assertFalse("Wrong number of events: " + prodCount + " " + consCount,
                         abs(prodCount - consCount) > 1 || abs(consCount - COUNT) > 1);