all: rename and normalize reflective class names (perfmark#214)

carl-mastrangelo · Mar 13, 2023 · 3d79f41 · 3d79f41
1 parent 4cd65bd
commit 3d79f41
Show file tree

Hide file tree

Showing 27 changed files with 174 additions and 223 deletions.
diff --git a/api/testing/src/test/java/io/perfmark/apitesting/ArtifactTest.java b/api/testing/src/test/java/io/perfmark/apitesting/ArtifactTest.java
@@ -34,9 +34,9 @@ public void hasPackage() throws Exception {
     List<String> reflectiveClasses =
         List.of(
             "io.perfmark.impl.SecretPerfMarkImpl$PerfMarkImpl",
-            "io.perfmark.java9.Reflect9$VarHandleGlobalMarkRecorder",
-            "io.perfmark.java6.SecretSynchronizedGlobalMarkRecorder$SynchronizedGlobalMarkRecorder",
-            "io.perfmark.java7.SecretMethodHandleGenerator$MethodHandleGenerator"
+            "io.perfmark.java9.SecretMarkRecorder$VarHandleMarkRecorder",
+            "io.perfmark.java6.SecretMarkRecorder$SynchronizedMarkRecorder",
+            "io.perfmark.java7.SecretGenerator$MethodHandleGenerator"
             );
     for (String reflectiveClass : reflectiveClasses) {
       Class<?> clz = Class.forName(reflectiveClass, false, getClass().getClassLoader());

diff --git a/impl/src/main/java/io/perfmark/impl/Generator.java b/impl/src/main/java/io/perfmark/impl/Generator.java
@@ -26,7 +26,7 @@
  *
  * <p>Normal users are not expected to use this class.
  */
-public abstract class Generator {
+public class Generator {
   /**
    * This field is here as a hack. This class is a shared dependency of both {@link
    * SecretPerfMarkImpl} and {@link Storage}. The impl needs to record the first time an event
@@ -64,15 +64,17 @@ protected Generator() {}
    *
    * @param generation the generation id, shifted left by {@link #GEN_OFFSET}.
    */
-  public abstract void setGeneration(long generation);
+  public void setGeneration(long generation) {}
 
   /**
    * Gets the current generation, shifted left by {@link #GEN_OFFSET}. An odd number means the
    * library is enabled, while an even number means the library is disabled.
    *
    * @return the current generation or {@link #FAILURE}.
    */
-  public abstract long getGeneration();
+  public long getGeneration() {
+    return FAILURE;
+  }
 
   /**
    * Returns the approximate cost to change the generation.

diff --git a/impl/src/main/java/io/perfmark/impl/GlobalMarkRecorder.java b/impl/src/main/java/io/perfmark/impl/GlobalMarkRecorder.java
diff --git a/impl/src/main/java/io/perfmark/impl/MarkRecorder.java b/impl/src/main/java/io/perfmark/impl/MarkRecorder.java
@@ -16,37 +16,117 @@
 
 package io.perfmark.impl;
 
-public abstract class MarkRecorder {
+/**
+ * A MarkRecorder records tracing events from {@link io.perfmark.PerfMark} calls.  Instances
+ * of this class can be called concurrently by multiple threads.
+ */
+public class MarkRecorder {
 
   protected MarkRecorder() {}
 
-  public abstract void start(long gen, String taskName, String tagName, long tagId, long nanoTime);
+  public void start(long gen, String taskName, String tagName, long tagId) {
+    unimplemented();
+  }
+
+  public void start(long gen, String taskName) {
+    unimplemented();
+  }
+
+  public void start(long gen, String taskName, String subTaskName) {
+    unimplemented();
+  }
+
+  public void startAt(long gen, String taskName, String tagName, long tagId, long nanoTime) {
+    unimplemented();
+  }
+
+  public void startAt(long gen, String taskName, long nanoTime) {
+    unimplemented();
+  }
+
+  public void startAt(long gen, String taskName, String subTaskName, long nanoTime) {
+    unimplemented();
+  }
+
+  public void link(long gen, long linkId) {
+    unimplemented();
+  }
+
+  public void stop(long gen) {
+    unimplemented();
+  }
+
+  public void stop(long gen, String taskName, String tagName, long tagId) {
+    unimplemented();
+  }
+
+  public void stop(long gen, String taskName) {
+    unimplemented();
+  }
+
+  public void stop(long gen, String taskName, String subTaskName) {
+    unimplemented();
+  }
+
+  public void stopAt(long gen, long nanoTime) {
+    unimplemented();
+  }
 
-  public abstract void start(long gen, String taskName, long nanoTime);
+  public void stopAt(long gen, String taskName, String tagName, long tagId, long nanoTime) {
+    unimplemented();
+  }
 
-  public abstract void start(long gen, String taskName, String subTaskName, long nanoTime);
+  public void stopAt(long gen, String taskName, long nanoTime) {
+    unimplemented();
+  }
 
-  public abstract void link(long gen, long linkId);
+  public void stopAt(long gen, String taskName, String subTaskName, long nanoTime) {
+    unimplemented();
+  }
 
-  public abstract void stop(long gen, long nanoTime);
+  public void event(long gen, String eventName, String tagName, long tagId) {
+    unimplemented();
+  }
 
-  public abstract void stop(long gen, String taskName, String tagName, long tagId, long nanoTime);
+  public void event(long gen, String eventName) {
+    unimplemented();
+  }
 
-  public abstract void stop(long gen, String taskName, long nanoTime);
+  public void event(long gen, String eventName, String subEventName) {
+    unimplemented();
+  }
 
-  public abstract void stop(long gen, String taskName, String subTaskName, long nanoTime);
+  public void eventAt(long gen, String eventName, String tagName, long tagId, long nanoTime) {
+    unimplemented();
+  }
 
-  public abstract void event(long gen, String eventName, String tagName, long tagId, long nanoTime);
+  public void eventAt(long gen, String eventName, long nanoTime) {
+    unimplemented();
+  }
 
-  public abstract void event(long gen, String eventName, long nanoTime);
+  public void eventAt(long gen, String eventName, String subEventName, long nanoTime) {
+    unimplemented();
+  }
 
-  public abstract void event(long gen, String eventName, String subEventName, long nanoTime);
+  public void attachTag(long gen, String tagName, long tagId) {
+    unimplemented();
+  }
 
-  public abstract void attachTag(long gen, String tagName, long tagId);
+  public void attachKeyedTag(long gen, String name, String value) {
+    unimplemented();
+  }
 
-  public abstract void attachKeyedTag(long gen, String name, String value);
+  public void attachKeyedTag(long gen, String name, long value0) {
+    unimplemented();
+  }
 
-  public abstract void attachKeyedTag(long gen, String name, long value0);
+  public void attachKeyedTag(long gen, String name, long value0, long value1) {
+    unimplemented();
+  }
 
-  public abstract void attachKeyedTag(long gen, String name, long value0, long value1);
+  /**
+   * This method exists for subclasses to add custom behavior to unimplemented method calls.
+   * All the other methods in this class invoke this method.
+   */
+  public void unimplemented() {}
 }
diff --git a/impl/src/main/java/io/perfmark/impl/NoopGenerator.java b/impl/src/main/java/io/perfmark/impl/NoopGenerator.java
diff --git a/impl/src/main/java/io/perfmark/impl/SecretPerfMarkImpl.java b/impl/src/main/java/io/perfmark/impl/SecretPerfMarkImpl.java
@@ -38,7 +38,7 @@ public static final class PerfMarkImpl extends Impl {
 
     private static final AtomicLong linkIdAlloc = new AtomicLong(1);
     private static final Generator generator;
-    private static final GlobalMarkRecorder markRecorder;
+    private static final MarkRecorder markRecorder;
 
     // May be null if debugging is disabled.
     private static final Object logger;
@@ -59,22 +59,22 @@ public static final class PerfMarkImpl extends Impl {
       Throwable[] problems = new Throwable[4];
       // Avoid using a for-loop for this code, as it makes it easier for tools like Proguard to rewrite.
       try {
-        Class<?> clz = Class.forName("io.perfmark.java7.SecretMethodHandleGenerator$MethodHandleGenerator");
+        Class<?> clz = Class.forName("io.perfmark.java7.SecretGenerator$MethodHandleGenerator");
         gen = clz.asSubclass(Generator.class).getConstructor().newInstance();
       } catch (Throwable t) {
         problems[0] = t;
       }
       if (gen == null) {
         try {
-          Class<?> clz = Class.forName("io.perfmark.java9.SecretVarHandleGenerator$VarHandleGenerator");
+          Class<?> clz = Class.forName("io.perfmark.java9.SecretGenerator$VarHandleGenerator");
           gen = clz.asSubclass(Generator.class).getConstructor().newInstance();
         } catch (Throwable t) {
           problems[1] = t;
         }
       }
       if (gen == null) {
         try {
-          Class<?> clz = Class.forName("io.perfmark.java6.SecretVolatileGenerator$VolatileGenerator");
+          Class<?> clz = Class.forName("io.perfmark.java6.SecretGenerator$VolatileGenerator");
           gen = clz.asSubclass(Generator.class).getConstructor().newInstance();
         } catch (Throwable t) {
           problems[2] = t;
@@ -90,7 +90,7 @@ public static final class PerfMarkImpl extends Impl {
         // Generator.  By doing a cast here, Java pushes the verification to when this branch is
         // actually taken, which is uncommon.  Avoid reflectively loading the class, which may
         // make binary shrinkers drop the NoopGenerator class.
-        generator = (Generator) (Object) new NoopGenerator();
+        generator = new Generator();
         isNoop = true;
       }
 
@@ -127,27 +127,27 @@ public static final class PerfMarkImpl extends Impl {
       problems[2] = null;
       problems[3] = null;
 
-      GlobalMarkRecorder markRecorder0 = null;
+      MarkRecorder markRecorder0 = null;
       if (!isNoop) {
         try {
           Class<?> clz =
-              Class.forName("io.perfmark.java9.Reflect9$VarHandleGlobalMarkRecorder");
-          markRecorder0 = clz.asSubclass(GlobalMarkRecorder.class).getConstructor().newInstance();
+              Class.forName("io.perfmark.java9.SecretMarkRecorder$VarHandleMarkRecorder");
+          markRecorder0 = clz.asSubclass(MarkRecorder.class).getConstructor().newInstance();
         } catch (Throwable t) {
           problems[0] = t;
         }
         if (markRecorder0 == null) {
           try {
             Class<?> clz =
-                Class.forName("io.perfmark.java6.SecretSynchronizedGlobalMarkRecorder$SynchronizedGlobalMarkRecorder");
-            markRecorder0 = clz.asSubclass(GlobalMarkRecorder.class).getConstructor().newInstance();
+                Class.forName("io.perfmark.java6.SecretMarkRecorder$SynchronizedMarkRecorder");
+            markRecorder0 = clz.asSubclass(MarkRecorder.class).getConstructor().newInstance();
           } catch (Throwable t) {
             problems[1] = t;
           }
         }
       }
       if (markRecorder0 == null) {
-        markRecorder0 = new GlobalMarkRecorder();
+        markRecorder0 = new MarkRecorder();
       }
       markRecorder = markRecorder0;
     }

diff --git a/java15/src/jmh/java/io/perfmark/java15/HiddenClassVarHandleMarkerBenchmarkTest.java b/java15/src/jmh/java/io/perfmark/java15/HiddenClassVarHandleMarkerBenchmarkTest.java
@@ -16,9 +16,7 @@
 
 package io.perfmark.java15;
 
-import io.perfmark.impl.GlobalMarkRecorder;
 import io.perfmark.impl.MarkRecorder;
-import io.perfmark.impl.MarkRecorderRef;
 import io.perfmark.testing.MarkHolderRecorder;
 import java.util.concurrent.TimeUnit;
 import org.junit.Test;
@@ -63,8 +61,8 @@ public void markHolderBenchmark() throws Exception {
   @State(Scope.Thread)
   public static class SecretHiddenClassMarkRecorderBenchmark extends MarkHolderRecorder {
     @Override
-    public GlobalMarkRecorder getMarkRecorder() {
-      return new Reflect15.HiddenClassVarHandleGlobalMarkRecorder();
+    public MarkRecorder getMarkRecorder() {
+      return new SecretMarkRecorder.HiddenClassMarkRecorder();
     }
 
     @Benchmark