Review feedback and minor test case improvements.

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/feature/AutomaticallyRegisteredFeature.java

@@ -34,7 +34,9 @@
 
 /**
  * {@link InternalFeature} classes with this annotation are automatically registered using an
- * annotation processor.
+ * annotation processor. If multiple implementations for a class are annotated with
+ * {@link AutomaticallyRegisteredFeature}, only the most specific one will be registered as a
+ * feature.
  *
  * Note that this requires the `SVM_PROCESSOR` to be defined as an annotation processor in the
  * suite.py file.

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

@@ -714,7 +714,7 @@ protected void operate() {
             SubstrateJVM.get().recording = false;
             JfrExecutionSampler.singleton().update();
 
-            /* No further JFR events are emitted, so free all JFR-related buffers. */
+            /* No further JFR events are emitted, so free some JFR-related buffers. */
             for (IsolateThread isolateThread = VMThreads.firstThread(); isolateThread.isNonNull(); isolateThread = VMThreads.nextThread(isolateThread)) {
                 JfrThreadLocal.stopRecording(isolateThread, false);
             }

substratevm/src/com.oracle.svm.test/src/com/oracle/svm/test/jfr/utils/JfrFileParser.java

@@ -27,18 +27,23 @@
 package com.oracle.svm.test.jfr.utils;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
 
 import java.io.IOException;
 import java.nio.file.Path;
+import java.util.ArrayDeque;
+import java.util.ArrayList;
+import java.util.Collection;
 import java.util.HashMap;
 
-import org.junit.Assert;
-
+import com.oracle.svm.core.jfr.JfrCheckpointType;
 import com.oracle.svm.core.jfr.JfrChunkWriter;
 import com.oracle.svm.core.jfr.JfrReservedEvent;
 import com.oracle.svm.core.jfr.JfrTicks;
 import com.oracle.svm.core.jfr.JfrType;
+import com.oracle.svm.test.jfr.utils.poolparsers.AbstractSerializerParser;
 import com.oracle.svm.test.jfr.utils.poolparsers.ClassConstantPoolParser;
 import com.oracle.svm.test.jfr.utils.poolparsers.ClassLoaderConstantPoolParser;
 import com.oracle.svm.test.jfr.utils.poolparsers.ConstantPoolParser;
@@ -59,105 +64,136 @@
 public class JfrFileParser {
 
     private static final HashMap<Long, ConstantPoolParser> supportedConstantPools;
+    private static final ArrayList<AbstractSerializerParser> serializerParsers;
 
     static {
         supportedConstantPools = new HashMap<>();
+        serializerParsers = new ArrayList<>();
+
+        addParser(JfrType.Class, new ClassConstantPoolParser());
+        addParser(JfrType.ClassLoader, new ClassLoaderConstantPoolParser());
+        addParser(JfrType.Package, new PackageConstantPoolParser());
+        addParser(JfrType.Module, new ModuleConstantPoolParser());
+
+        addParser(JfrType.Symbol, new SymbolConstantPoolParser());
+        addParser(JfrType.Method, new MethodConstantPoolParser());
+        addParser(JfrType.StackTrace, new StacktraceConstantPoolParser());
+
+        addParser(JfrType.Thread, new ThreadConstantPoolParser());
+        addParser(JfrType.ThreadGroup, new ThreadGroupConstantPoolParser());
+
+        addParser(JfrType.FrameType, new FrameTypeConstantPoolParser());
+        addParser(JfrType.ThreadState, new ThreadStateConstantPoolParser());
+        addParser(JfrType.GCName, new GCNameConstantPoolParser());
+        addParser(JfrType.GCCause, new GCCauseConstantPoolParser());
+        addParser(JfrType.VMOperation, new VMOperationConstantPoolParser());
+        addParser(JfrType.MonitorInflationCause, new MonitorInflationCauseConstantPoolParser());
+    }
 
-        supportedConstantPools.put(JfrType.Class.getId(), new ClassConstantPoolParser());
-        supportedConstantPools.put(JfrType.ClassLoader.getId(), new ClassLoaderConstantPoolParser());
-        supportedConstantPools.put(JfrType.Package.getId(), new PackageConstantPoolParser());
-        supportedConstantPools.put(JfrType.Module.getId(), new ModuleConstantPoolParser());
+    private static void addParser(JfrType type, ConstantPoolParser parser) {
+        supportedConstantPools.put(type.getId(), parser);
+        if (parser instanceof AbstractSerializerParser p) {
+            serializerParsers.add(p);
+        }
+    }
 
-        supportedConstantPools.put(JfrType.Symbol.getId(), new SymbolConstantPoolParser());
-        supportedConstantPools.put(JfrType.Method.getId(), new MethodConstantPoolParser());
-        supportedConstantPools.put(JfrType.StackTrace.getId(), new StacktraceConstantPoolParser());
-        supportedConstantPools.put(JfrType.FrameType.getId(), new FrameTypeConstantPoolParser());
+    public static HashMap<Long, ConstantPoolParser> getSupportedConstantPools() {
+        return supportedConstantPools;
+    }
 
-        supportedConstantPools.put(JfrType.Thread.getId(), new ThreadConstantPoolParser());
-        supportedConstantPools.put(JfrType.ThreadGroup.getId(), new ThreadGroupConstantPoolParser());
-        supportedConstantPools.put(JfrType.ThreadState.getId(), new ThreadStateConstantPoolParser());
+    public static void parse(Path path) throws IOException {
+        RecordingInput input = new RecordingInput(path.toFile());
+        FileHeaderInfo header = parseFileHeader(input);
+        parseMetadata(input, header.metadataPosition);
 
-        supportedConstantPools.put(JfrType.GCName.getId(), new GCNameConstantPoolParser());
-        supportedConstantPools.put(JfrType.GCCause.getId(), new GCCauseConstantPoolParser());
-        supportedConstantPools.put(JfrType.VMOperation.getId(), new VMOperationConstantPoolParser());
-        supportedConstantPools.put(JfrType.MonitorInflationCause.getId(), new MonitorInflationCauseConstantPoolParser());
+        Collection<Long> constantPoolOffsets = getConstantPoolOffsets(input, header.checkpointPosition);
+        verifyConstantPools(input, constantPoolOffsets);
     }
 
-    public static HashMap<Long, ConstantPoolParser> getSupportedConstantPools() {
-        return supportedConstantPools;
+    private static void parseMetadata(RecordingInput input, long metadataPosition) throws IOException {
+        parseMetadataHeader(input, metadataPosition);
+        MetadataDescriptor.read(input);
+    }
+
+    private static void parseMetadataHeader(RecordingInput input, long metadataPosition) throws IOException {
+        input.position(metadataPosition);
+        assertTrue("Metadata size is invalid!", input.readInt() > 0);
+        assertEquals(JfrReservedEvent.METADATA.getId(), input.readLong());
+        assertTrue("Metadata timestamp is invalid!", input.readLong() > 0);
+        input.readLong(); // Duration.
+        assertTrue("Metadata ID is invalid!", input.readLong() > 0);
     }
 
-    private static Positions parserFileHeader(RecordingInput input) throws IOException {
+    private static FileHeaderInfo parseFileHeader(RecordingInput input) throws IOException {
         byte[] fileMagic = new byte[JfrChunkWriter.FILE_MAGIC.length];
         input.readFully(fileMagic); // File magic.
         assertEquals("File magic is not correct!", new String(JfrChunkWriter.FILE_MAGIC), new String(fileMagic));
         assertEquals("JFR version major is not correct!", JfrChunkWriter.JFR_VERSION_MAJOR, input.readRawShort());
         assertEquals("JFR version minor is not correct!", JfrChunkWriter.JFR_VERSION_MINOR, input.readRawShort());
-        assertTrue("Chunk size is invalid!", input.readRawLong() > 0); // Chunk size.
+        assertTrue("Chunk size is invalid!", input.readRawLong() > 0);
 
-        long constantPoolPosition = input.readRawLong();
-        assertTrue("Constant pool positions is invalid!", constantPoolPosition > 0);
+        long checkpointPosition = input.readRawLong();
+        assertTrue("Checkpoint positions is invalid!", checkpointPosition > 0);
         long metadataPosition = input.readRawLong();
-        assertTrue("Metadata positions is null!", metadataPosition != 0);
+        assertTrue("Metadata position is invalid", metadataPosition > 0);
 
         long startingTime = input.readRawLong();
-        assertTrue("Starting time is invalid!", startingTime > 0); // Starting time.
-        Assert.assertTrue("Starting time is bigger than current time!", startingTime < JfrTicks.currentTimeNanos());
+        assertTrue("Starting time is invalid!", startingTime > 0);
+        assertTrue("Starting time is bigger than current time!", startingTime < JfrTicks.currentTimeNanos());
         input.readRawLong(); // Duration.
-        assertTrue("Chunk start tick is invalid!", input.readRawLong() > 0); // ChunkStartTick.
-        assertTrue("Tick frequency is invalid!", input.readRawLong() > 0); // Tick frequency.
+        assertTrue("Chunk start tick is invalid!", input.readRawLong() > 0);
+        assertTrue("Tick frequency is invalid!", input.readRawLong() > 0);
         int shouldUseCompressedInt = input.readRawInt();
-        assertTrue("Compressed int must be either 0 or 1!", shouldUseCompressedInt == 0 || shouldUseCompressedInt == 1); // ChunkWriteTick.
+        assertTrue("Compressed int must be either 0 or 1!", shouldUseCompressedInt == 0 || shouldUseCompressedInt == 1);
 
-        return new Positions(constantPoolPosition, metadataPosition);
+        return new FileHeaderInfo(checkpointPosition, metadataPosition);
     }
 
-    private static void parseMetadataHeader(RecordingInput input, long metadataPosition) throws IOException {
-        input.position(metadataPosition); // Seek to starting position of metadata region.
-        assertTrue("Metadata size is invalid!", input.readInt() > 0); // Size of metadata.
-        assertEquals(JfrReservedEvent.METADATA.getId(), input.readLong());
-        assertTrue("Metadata timestamp is invalid!", input.readLong() > 0); // Timestamp.
-        input.readLong(); // Duration.
-        input.readLong(); // Metadata ID.
-    }
-
-    private static void parseMetadata(RecordingInput input, long metadataPosition) throws IOException {
-        parseMetadataHeader(input, metadataPosition);
-        MetadataDescriptor.read(input);
-    }
+    private static ArrayDeque<Long> getConstantPoolOffsets(RecordingInput input, long initialCheckpointPosition) throws IOException {
+        ArrayDeque<Long> constantPoolOffsets = new ArrayDeque<>();
+        long deltaNext;
+        long currentCheckpointPosition = initialCheckpointPosition;
+        do {
+            input.position(currentCheckpointPosition);
+            assertTrue("Constant pool size is invalid!", input.readInt() > 0);
+            assertEquals(JfrReservedEvent.CHECKPOINT.getId(), input.readLong());
+            assertTrue("Constant pool timestamp is invalid!", input.readLong() > 0);
+            input.readLong(); // Duration.
+            deltaNext = input.readLong();
+            assertTrue("Delta to next checkpoint is invalid!", deltaNext <= 0);
+            byte checkpointType = input.readByte();
+            assertTrue("Checkpoint type is invalid!", checkpointType == JfrCheckpointType.Flush.getId() || checkpointType == JfrCheckpointType.Threads.getId());
+
+            constantPoolOffsets.addFirst(input.position());
+
+            currentCheckpointPosition += deltaNext;
+        } while (deltaNext != 0);
 
-    private static long parseConstantPoolHeader(RecordingInput input, long constantPoolPosition) throws IOException {
-        input.position(constantPoolPosition); // Seek to starting position of constant pools.
-        // Size of constant pools.
-        assertTrue("Constant pool size is invalid!", input.readInt() > 0);
-        // Constant pools region ID.
-        assertEquals(JfrReservedEvent.CHECKPOINT.getId(), input.readLong());
-        assertTrue("Constant pool timestamp is invalid!", input.readLong() > 0); // Timestamp.
-        input.readLong(); // Duration.
-        long deltaNext = input.readLong(); // Offset to a next constant pools region.
-        assertTrue(input.readBoolean()); // Flush.
-        return deltaNext;
+        return constantPoolOffsets;
     }
 
-    private static void verifyConstantPools(RecordingInput input, long constantPoolPosition) throws IOException {
-        long deltaNext;
-        long currentConstantPoolPosition = constantPoolPosition;
-        do {
-            deltaNext = parseConstantPoolHeader(input, currentConstantPoolPosition);
-            long numberOfCPs = input.readInt();
+    private static void verifyConstantPools(RecordingInput input, Collection<Long> constantPoolOffsets) throws IOException {
+        for (Long offset : constantPoolOffsets) {
+            input.position(offset);
+            int numberOfCPs = input.readInt();
             for (int i = 0; i < numberOfCPs; i++) {
                 ConstantPoolParser constantPoolParser = supportedConstantPools.get(input.readLong());
-                Assert.assertNotNull("Unknown constant pool!", constantPoolParser);
+                assertNotNull("Unknown constant pool!", constantPoolParser);
                 constantPoolParser.parse(input);
             }
-            currentConstantPoolPosition += deltaNext;
-        } while (deltaNext != 0);
+            compareFoundAndExpectedIds();
+        }
 
-        /* Now that we collected all data, verify and clear it. */
-        compareFoundAndExpectedIds();
+        verifySerializers();
         resetConstantPoolParsers();
     }
 
+    private static void verifySerializers() {
+        for (ConstantPoolParser parser : serializerParsers) {
+            assertFalse("Serializer data must always be present in the chunk.", parser.isEmpty());
+        }
+    }
+
     private static void compareFoundAndExpectedIds() {
         for (ConstantPoolParser parser : supportedConstantPools.values()) {
             parser.compareFoundAndExpectedIds();
@@ -170,29 +206,6 @@ public static void resetConstantPoolParsers() {
         }
     }
 
-    public static void parse(Path path) throws IOException {
-        RecordingInput input = new RecordingInput(path.toFile());
-        Positions positions = parserFileHeader(input);
-        verifyConstantPools(input, positions.getConstantPoolPosition());
-        parseMetadata(input, positions.getMetadataPosition());
-    }
-
-    private static class Positions {
-
-        private final long constantPoolPosition;
-        private final long metadataPosition;
-
-        Positions(long constantPoolPosition, long metadataPositions) {
-            this.constantPoolPosition = constantPoolPosition;
-            this.metadataPosition = metadataPositions;
-        }
-
-        public long getConstantPoolPosition() {
-            return constantPoolPosition;
-        }
-
-        public long getMetadataPosition() {
-            return metadataPosition;
-        }
+    private record FileHeaderInfo(long checkpointPosition, long metadataPosition) {
     }
 }

substratevm/src/com.oracle.svm.test/src/com/oracle/svm/test/jfr/utils/poolparsers/AbstractRepositoryParser.java

@@ -0,0 +1,29 @@
+/*
+ * Copyright (c) 2023, 2023, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.  Oracle designates this
+ * particular file as subject to the "Classpath" exception as provided
+ * by Oracle in the LICENSE file that accompanied this code.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+package com.oracle.svm.test.jfr.utils.poolparsers;
+
+public abstract class AbstractRepositoryParser extends ConstantPoolParser {
+}

substratevm/src/com.oracle.svm.test/src/com/oracle/svm/test/jfr/utils/poolparsers/AbstractSerializerParser.java

@@ -0,0 +1,29 @@
+/*
+ * Copyright (c) 2023, 2023, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.  Oracle designates this
+ * particular file as subject to the "Classpath" exception as provided
+ * by Oracle in the LICENSE file that accompanied this code.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+package com.oracle.svm.test.jfr.utils.poolparsers;
+
+public abstract class AbstractSerializerParser extends ConstantPoolParser {
+}

substratevm/src/com.oracle.svm.test/src/com/oracle/svm/test/jfr/utils/poolparsers/ClassConstantPoolParser.java

@@ -34,7 +34,7 @@
 import com.oracle.svm.core.jfr.JfrType;
 import com.oracle.svm.test.jfr.utils.RecordingInput;
 
-public class ClassConstantPoolParser extends ConstantPoolParser {
+public class ClassConstantPoolParser extends AbstractRepositoryParser {
 
     @Override
     public void parse(RecordingInput input) throws IOException {

substratevm/src/com.oracle.svm.test/src/com/oracle/svm/test/jfr/utils/poolparsers/ClassLoaderConstantPoolParser.java

@@ -31,7 +31,7 @@
 import com.oracle.svm.core.jfr.JfrType;
 import com.oracle.svm.test.jfr.utils.RecordingInput;
 
-public class ClassLoaderConstantPoolParser extends ConstantPoolParser {
+public class ClassLoaderConstantPoolParser extends AbstractRepositoryParser {
 
     @Override
     public void reset() {

substratevm/src/com.oracle.svm.test/src/com/oracle/svm/test/jfr/utils/poolparsers/ConstantPoolParser.java

@@ -52,6 +52,10 @@ protected ConstantPoolParser() {
         foundIds.add(0L);
     }
 
+    public boolean isEmpty() {
+        return foundIds.isEmpty();
+    }
+
     protected final void addFoundId(long id) {
         foundIds.add(id);
     }

substratevm/src/com.oracle.svm.test/src/com/oracle/svm/test/jfr/utils/poolparsers/FrameTypeConstantPoolParser.java

@@ -26,17 +26,19 @@
 
 package com.oracle.svm.test.jfr.utils.poolparsers;
 
-import com.oracle.svm.test.jfr.utils.RecordingInput;
+import java.io.IOException;
+
 import org.junit.Assert;
 
-import java.io.IOException;
+import com.oracle.svm.test.jfr.utils.RecordingInput;
 
-public class FrameTypeConstantPoolParser extends ConstantPoolParser {
+public class FrameTypeConstantPoolParser extends AbstractSerializerParser {
 
     @Override
     public void parse(RecordingInput input) throws IOException {
-        int numberOfFrameTypes = input.readInt();
-        for (int i = 0; i < numberOfFrameTypes; i++) {
+        int count = input.readInt();
+        Assert.assertTrue(count > 0);
+        for (int i = 0; i < count; i++) {
             addFoundId(input.readInt()); // FrameTypeId.
             Assert.assertFalse("FrameTypeName is empty!", input.readUTF().isEmpty()); // FrameTypeName.
         }