Extract MutableJvmSymbol

Naming the byte array as an explicit type gives us: - one Javadoc to explain the contents - memoized `new String(bytes)` Also clean up dead code and add missing changelog.
atlassian · Jul 1, 2024 · ec12c6f · ec12c6f
1 parent 7e89ac3
commit ec12c6f
Show file tree

Hide file tree

Showing 6 changed files with 66 additions and 94 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -26,6 +26,11 @@ Dropping a requirement of a major version of a dependency is a new contract.
 ## [Unreleased]
 [Unreleased]: https://github.com/atlassian/report/compare/release-4.4.0...master
 
+### Added
+- Add `MutableJvmSymbol`, `JfrFilter.Builder.symbolModifier` and `MultiJfrFilter.Builder.symbolModifier`.
+  They allow for mutating JVM symbols in JFR files, e.g. to normalize dynamic proxy names or lambda names.
+  This way, the same JVM code can be profiled multiple times and resulting JFRs can be merged or diffed.
+
 ## [4.4.0] - 2024-01-18
 [4.4.0]: https://github.com/atlassian/report/compare/release-4.3.0...release-4.4.0
 

diff --git a/src/main/java/tools/profiler/jfr/converter/CheckpointEvent.java b/src/main/java/tools/profiler/jfr/converter/CheckpointEvent.java
@@ -5,6 +5,7 @@
 
 package tools.profiler.jfr.converter;
 
+import com.atlassian.performance.tools.report.api.jfr.MutableJvmSymbol;
 import org.openjdk.jmc.flightrecorder.testutils.parser.MetadataEvent;
 import org.openjdk.jmc.flightrecorder.testutils.parser.RecordingStream;
 import org.openjdk.jmc.flightrecorder.testutils.parser.StreamingChunkParser;
@@ -27,8 +28,6 @@ public final class CheckpointEvent {
 
     private final Listener listener;
 
-    private static final int CHUNK_HEADER_SIZE = 68;
-
     private final RecordingStream stream;
     private final MetadataEvent metadata;
     private final ByteBuffer payload;
@@ -71,24 +70,12 @@ public byte[] payload() {
 
     private void readConstants(JfrClass type) throws IOException {
         switch (type.name) {
-//            case "jdk.types.ChunkHeader":
-//                stream.skip(CHUNK_HEADER_SIZE + 3);
-//                break;
-//            case "java.lang.Thread":
-//                readThreads(type.fields.size());
-//                break;
-//            case "java.lang.Class":
-//                readClasses(type.fields.size());
-//                break;
             case "java.lang.String":
                 readStrings();
                 break;
             case "jdk.types.Symbol":
                 readSymbols();
                 break;
-//            case "jdk.types.Method":
-//                readMethods();
-//                break;
             case "jdk.types.StackTrace":
                 readStackTraces();
                 break;
@@ -101,68 +88,23 @@ private void readConstants(JfrClass type) throws IOException {
         }
     }
 
-
-    //    private void readThreads(int fieldCount) {
-//        int count = threads.preallocate(getVarint());
-//        for (int i = 0; i < count; i++) {
-//            long id = getVarlong();
-//            String osName = getString();
-//            int osThreadId = getVarint();
-//            String javaName = getString();
-//            long javaThreadId = getVarlong();
-//            readFields(fieldCount - 4);
-//            threads.put(id, javaName != null ? javaName : osName);
-//        }
-//    }
-//
-//    private void readClasses(int fieldCount) {
-//        int count = classes.preallocate(getVarint());
-//        for (int i = 0; i < count; i++) {
-//            long id = getVarlong();
-//            long loader = getVarlong();
-//            long name = getVarlong();
-//            long pkg = getVarlong();
-//            int modifiers = getVarint();
-//            readFields(fieldCount - 4);
-//            classes.put(id, new ClassRef(name));
-//        }
-//    }
-//
-//    private void readMethods() {
-//        int count = methods.preallocate(getVarint());
-//        for (int i = 0; i < count; i++) {
-//            long id = getVarlong();
-//            long cls = getVarlong();
-//            long name = getVarlong();
-//            long sig = getVarlong();
-//            int modifiers = getVarint();
-//            int hidden = getVarint();
-//            methods.put(id, new MethodRef(cls, name, sig));
-//        }
-//    }
-//
     private void readStackTraces() throws IOException {
         int count = stream.readVarint();
         for (int i = 0; i < count; i++) {
-            long id = stream.readVarlong();
-            int truncated = stream.readVarint();
-            Object stackTrace = readStackTrace();
+            stream.readVarlong();
+            stream.readVarint();
+            readStackTrace();
         }
     }
 
-    private Object readStackTrace() throws IOException {
+    private void readStackTrace() throws IOException {
         int depth = stream.readVarint();
-        long[] methods = new long[depth];
-        byte[] types = new byte[depth];
-        int[] locations = new int[depth];
         for (int i = 0; i < depth; i++) {
-            methods[i] = stream.readVarlong();
-            int line = stream.readVarint();
-            int bci = stream.readVarint();
-            locations[i] = line << 16 | (bci & 0xffff);
-            types[i] = stream.read();
+            stream.readVarlong();
+            stream.readVarint();
+            stream.readVarint();
+            stream.read();
         }
-        return new Object(); // methods, types, locations
     }
 
     private void readStrings() throws IOException {
@@ -202,22 +144,22 @@ private void readSymbols() throws IOException {
         for (int i = 0; i < count; i++) {
             long id = stream.readVarlong();
             byte encoding = stream.read();
-            if (encoding != 3) { // TODO maybe just reuse [readVarstring]
+            if (encoding != 3) {
                 throw new IllegalArgumentException("Invalid symbol encoding " + encoding);
             }
-            byte[] symbolPayload = stream.readVarbytes();
+            MutableJvmSymbol symbolPayload = new MutableJvmSymbol(stream.readVarbytes());
             updateSymbol(symbolPayload);
-            symbols.put(id, new String(symbolPayload));
+            symbols.put(id, symbolPayload.toString());
         }
     }
 
-    private void updateSymbol(byte[] symbolPayload) {
-        listener.onSymbol(symbolPayload);
+    private void updateSymbol(MutableJvmSymbol symbol) {
+        listener.onSymbol(symbol);
         int symbolEndPosition = (int) stream.position();
         // rewind before the payload, but after the varint in [readVarbytes]
-        int symbolPosition = symbolEndPosition - symbolPayload.length;
+        int symbolPosition = symbolEndPosition - symbol.getPayload().length;
         payload.position(symbolPosition);
-        payload.put(symbolPayload);
+        payload.put(symbol.getPayload());
     }
 
     private void readEnumValues(String typeName) throws IOException {
@@ -260,16 +202,10 @@ private void readFields(boolean[] numeric) throws IOException {
         }
     }
 
-    private void readFields(int count) throws IOException {
-        while (count-- > 0) {
-            stream.readVarlong();
-        }
-    }
-
     @FunctionalInterface
     public interface Listener {
 
-        void onSymbol(byte[] symbolPayload);
+        void onSymbol(MutableJvmSymbol symbolPayload);
     }
 
 }
diff --git a/src/main/kotlin/com/atlassian/performance/tools/report/api/jfr/JfrFilter.kt b/src/main/kotlin/com/atlassian/performance/tools/report/api/jfr/JfrFilter.kt
@@ -13,7 +13,7 @@ import java.util.function.Predicate
 class JfrFilter private constructor(
     private val eventFilter: Predicate<RecordedEvent>,
     private val filteredRecording: Function<Path, Path>,
-    private val symbolModifier: Consumer<ByteArray>
+    private val symbolModifier: Consumer<MutableJvmSymbol>
 ) {
     private val logger = LogManager.getLogger(this::class.java)
 
@@ -32,14 +32,22 @@ class JfrFilter private constructor(
         private var eventFilter: Predicate<RecordedEvent> = Predicate { true }
         private var filteredRecording: Function<Path, Path> =
             Function { it.resolveSibling("filtered-" + it.fileName.toString()) }
-        private var symbolModifier: Consumer<ByteArray> = Consumer { }
+        private var symbolModifier: Consumer<MutableJvmSymbol> = Consumer { }
 
         fun eventFilter(eventFilter: Predicate<RecordedEvent>): Builder {
             this.eventFilter = eventFilter
             return this
         }
 
-        fun symbolModifier(symbolModifier: Consumer<ByteArray>) = apply {
+        /**
+         * Symbol examples:
+         * - package names, e.g. `org/apache/tomcat/util/modeler`
+         * - class names, e.g. `webwork/action/factory/JspActionFactoryProxy`
+         * - method names, e.g. `compileSoy`
+         * - method signatures, e.g. `(Lcom/google/template/soy/exprtree/ExprNode$ParentExprNode;)Ljava/util/List;`
+         * - native function names, e.g. `AddNode::Ideal`
+         */
+        fun symbolModifier(symbolModifier: Consumer<MutableJvmSymbol>) = apply {
             this.symbolModifier = symbolModifier
         }
 

diff --git a/src/main/kotlin/com/atlassian/performance/tools/report/api/jfr/MultiJfrFilter.kt b/src/main/kotlin/com/atlassian/performance/tools/report/api/jfr/MultiJfrFilter.kt
@@ -16,7 +16,7 @@ import java.util.function.Predicate
 class MultiJfrFilter private constructor(
     private val input: Path,
     private val outputs: List<FilteredOutput>,
-    private val symbolModifier: Consumer<ByteArray>
+    private val symbolModifier: Consumer<MutableJvmSymbol>
 ) {
 
     private class CompositeJfrEventListener(
@@ -82,14 +82,14 @@ class MultiJfrFilter private constructor(
         private val input: Path
     ) {
         private val outputs = mutableListOf<FilteredOutput>()
-        private var symbolModifier: Consumer<ByteArray> = Consumer { }
+        private var symbolModifier: Consumer<MutableJvmSymbol> = Consumer { }
 
         fun output(output: Path, filter: Predicate<RecordedEvent>): Builder {
             outputs.add(FilteredOutput(output, filter))
             return this
         }
 
-        fun symbolModifier(symbolModifier: Consumer<ByteArray>) = apply { this.symbolModifier = symbolModifier }
+        fun symbolModifier(symbolModifier: Consumer<MutableJvmSymbol>) = apply { this.symbolModifier = symbolModifier }
 
         fun build(): MultiJfrFilter {
             check(outputs.isNotEmpty()) { "At least one output must be specified" }

diff --git a/src/main/kotlin/com/atlassian/performance/tools/report/api/jfr/MutableJvmSymbol.kt b/src/main/kotlin/com/atlassian/performance/tools/report/api/jfr/MutableJvmSymbol.kt
@@ -0,0 +1,23 @@
+package com.atlassian.performance.tools.report.api.jfr
+
+/**
+ * JVM symbol examples:
+ * - package names, e.g. `org/apache/tomcat/util/modeler`
+ * - class names, e.g. `webwork/action/factory/JspActionFactoryProxy`
+ * - method names, e.g. `compileSoy`
+ * - method signatures, e.g. `(Lcom/google/template/soy/exprtree/ExprNode$ParentExprNode;)Ljava/util/List;`
+ * - native function names, e.g. `AddNode::Ideal`
+ *
+ * To optimize JFR filtering, the byte array is not defensively copied.
+ */
+class MutableJvmSymbol(
+    /**
+     * The binary representation of the JVM symbol. The bytes can be overwritten.
+     */
+    val payload: ByteArray
+) {
+
+    private val string = String(payload)
+
+    override fun toString() = string
+}
diff --git a/src/test/kotlin/com/atlassian/performance/tools/report/jfr/JfrFilterTest.kt b/src/test/kotlin/com/atlassian/performance/tools/report/jfr/JfrFilterTest.kt
@@ -1,6 +1,7 @@
 package com.atlassian.performance.tools.report.jfr
 
 import com.atlassian.performance.tools.report.api.jfr.JfrFilter
+import com.atlassian.performance.tools.report.api.jfr.MutableJvmSymbol
 import com.atlassian.performance.tools.report.api.result.CompressedResult
 import jdk.jfr.consumer.RecordedEvent
 import org.apache.logging.log4j.LogManager
@@ -97,9 +98,9 @@ class JfrFilterTest {
             }
 
         }
-        val checkpointListener = CheckpointEvent.Listener { symbolPayload ->
+        val checkpointListener = CheckpointEvent.Listener { symbol ->
             symbolCount.incrementAndGet()
-            uniqueSymbols += String(symbolPayload)
+            uniqueSymbols += symbol.toString()
         }
         StreamingChunkParser(chunkListener, checkpointListener).parse(this)
         return result
@@ -180,11 +181,10 @@ class JfrFilterTest {
      * - `org/springframework/core/$Proxy724`
      * - `jdk/proxy176/$Proxy724`
      */
-    private fun normalizeDynamicProxy(symbolPayload: ByteArray) {
-        val payload = String(symbolPayload)
-        if (payload.contains(Regex("\\\$Proxy[0-9]")) || payload.contains(Regex("proxy[0-9]"))) {
-            val newSymbol = "PROXY".padEnd(symbolPayload.size, '_')
-            ByteBuffer.wrap(symbolPayload).put(newSymbol.toByteArray())
+    private fun normalizeDynamicProxy(symbol: MutableJvmSymbol) {
+        if (symbol.toString().contains(Regex("\\\$Proxy[0-9]")) || symbol.toString().contains(Regex("proxy[0-9]"))) {
+            val newSymbol = "PROXY".padEnd(symbol.payload.size, '_')
+            ByteBuffer.wrap(symbol.payload).put(newSymbol.toByteArray())
         }
     }