8252898: remove bulk registration of JFR CompilerPhaseType names

Reviewed-by: kvn, jcm

Author: Doug Simon

src/hotspot/share/compiler/compileBroker.cpp

@@ -601,19 +601,16 @@ void register_jfr_phasetype_serializer(CompilerType compiler_type) {
   ResourceMark rm;
   static bool first_registration = true;
   if (compiler_type == compiler_jvmci) {
-    // register serializer, phases will be added later lazily.
-    GrowableArray<const char*>* jvmci_phase_names = new GrowableArray<const char*>(1);
-    jvmci_phase_names->append("NOT_A_PHASE_NAME");
-    CompilerEvent::PhaseEvent::register_phases(jvmci_phase_names);
+    CompilerEvent::PhaseEvent::get_phase_id("NOT_A_PHASE_NAME", false, false, false);
     first_registration = false;
 #ifdef COMPILER2
   } else if (compiler_type == compiler_c2) {
     assert(first_registration, "invariant"); // c2 must be registered first.
     GrowableArray<const char*>* c2_phase_names = new GrowableArray<const char*>(PHASE_NUM_TYPES);
     for (int i = 0; i < PHASE_NUM_TYPES; i++) {
-      c2_phase_names->append(CompilerPhaseTypeHelper::to_string((CompilerPhaseType)i));
+      const char* phase_name = CompilerPhaseTypeHelper::to_string((CompilerPhaseType) i);
+      CompilerEvent::PhaseEvent::get_phase_id(phase_name, false, false, false);
     }
-    CompilerEvent::PhaseEvent::register_phases(c2_phase_names);
     first_registration = false;
 #endif // COMPILER2
   }

src/hotspot/share/compiler/compilerEvent.cpp

@@ -34,67 +34,84 @@
 class PhaseTypeGuard : public StackObj {
  private:
   static Semaphore _mutex_semaphore;
+  bool _enabled;
  public:
-  PhaseTypeGuard() {
-    _mutex_semaphore.wait();
+  PhaseTypeGuard(bool enabled=true) {
+    if (enabled) {
+      _mutex_semaphore.wait();
+      _enabled = true;
+    } else {
+      _enabled = false;
+    }
   }
   ~PhaseTypeGuard() {
-    _mutex_semaphore.signal();
+    if (_enabled) {
+      _mutex_semaphore.signal();
+    }
   }
 };
 
 Semaphore PhaseTypeGuard::_mutex_semaphore(1);
 
-static void write_phases(JfrCheckpointWriter& writer, u4 base_idx, GrowableArray<const char*>* phases) {
-  assert(phases != NULL, "invariant");
-  assert(phases->is_nonempty(), "invariant");
-  const u4 nof_entries = phases->length();
-  writer.write_count(nof_entries);
-  for (u4 i = 0; i < nof_entries; i++) {
-    writer.write_key(base_idx + i);
-    writer.write(phases->at(i));
-  }
-}
-
+// Table for mapping compiler phases names to int identifiers.
 static GrowableArray<const char*>* phase_names = NULL;
 
 class CompilerPhaseTypeConstant : public JfrSerializer {
  public:
   void serialize(JfrCheckpointWriter& writer) {
     PhaseTypeGuard guard;
-    write_phases(writer, 0, phase_names);
+    assert(phase_names != NULL, "invariant");
+    assert(phase_names->is_nonempty(), "invariant");
+    const u4 nof_entries = phase_names->length();
+    writer.write_count(nof_entries);
+    for (u4 i = 0; i < nof_entries; i++) {
+      writer.write_key(i);
+      writer.write(phase_names->at(i));
+    }
   }
 };
 
-// This function provides support for adding dynamic entries to JFR type CompilerPhaseType.
-// The mapping for CompilerPhaseType is maintained as growable array phase_names.
-// The serializer CompilerPhaseTypeConstant must be registered with JFR at vm init.
-// Registration of new phase names creates mapping, serialize it for current chunk and registers its serializer with JFR if it is not already done.
-int CompilerEvent::PhaseEvent::register_phases(GrowableArray<const char*>* new_phases) {
-  int idx = -1;
-  if (new_phases == NULL || new_phases->is_empty()) {
-    return idx;
+static int lookup_phase(const char* phase_name) {
+  for (int i = 0; i < phase_names->length(); i++) {
+    const char* name = phase_names->at(i);
+    if (strcmp(name, phase_name) == 0) {
+      return i;
+    }
   }
+  return -1;
+}
+
+int CompilerEvent::PhaseEvent::get_phase_id(const char* phase_name, bool may_exist, bool use_strdup, bool sync) {
+  int index;
   bool register_jfr_serializer = false;
   {
-    PhaseTypeGuard guard;
+    PhaseTypeGuard guard(sync);
     if (phase_names == NULL) {
-      phase_names = new (ResourceObj::C_HEAP, mtCompiler) GrowableArray<const char*>(100, mtCompiler);
+      phase_names = new (ResourceObj::C_HEAP, mtInternal) GrowableArray<const char*>(100, mtCompiler);
       register_jfr_serializer = true;
+    } else if (may_exist) {
+      index = lookup_phase(phase_name);
+      if (index != -1) {
+        return index;
+      }
+    } else {
+      assert((index = lookup_phase(phase_name)) == -1, "phase name \"%s\" already registered: %d", phase_name, index);
     }
-    idx = phase_names->length();
-    phase_names->appendAll(new_phases);
-    guarantee(phase_names->length() < 256, "exceeds maximum supported phases");
+
+    index = phase_names->length();
+    phase_names->append(use_strdup ? strdup(phase_name) : phase_name);
   }
   if (register_jfr_serializer) {
     JfrSerializer::register_serializer(TYPE_COMPILERPHASETYPE, false, new CompilerPhaseTypeConstant());
   } else if (Jfr::is_recording()) {
-    // serialize new_phases.
+    // serialize new phase.
     JfrCheckpointWriter writer;
     writer.write_type(TYPE_COMPILERPHASETYPE);
-    write_phases(writer, idx, new_phases);
+    writer.write_count(1);
+    writer.write_key(index);
+    writer.write(phase_name);
   }
-  return idx;
+  return index;
 }
 
 void CompilerEvent::CompilationEvent::post(EventCompilation& event, int compile_id, CompilerType compiler_type, Method* method, int compile_level, bool success, bool is_osr, int code_size, int inlined_bytecodes) {

src/hotspot/share/compiler/compilerEvent.hpp

@@ -64,15 +64,13 @@ class CompilerEvent : AllStatic {
   class PhaseEvent : AllStatic {
     friend class CompilerPhaseTypeConstant;
    public:
-    /**
-     * Register compiler phases for JFR type CompilerPhaseType serialization purposes.
-     * This method is called during compiler creation or during compilation.
-     * Registration will serialize the passed in phase constants, supporting bulk and/or incremental registrations.
-     * This method returns start index of new list that just got appended to phase_names.
-     * Param new_phases may contain duplicates.
-     * Return value could be used for mapping purpose at caller site, or caller can assume explicit order of registration.
-     */
-    static int register_phases(GrowableArray<const char*>* new_phases) NOT_JFR_RETURN_(-1);
+
+    // Gets a unique identifier for `phase_name`, computing and registering it first if necessary.
+    // If `may_exist` is true, then current registrations are searched first. If false, then
+    // there must not be an existing registration for `phase_name`.
+    // If `use_strdup` is true, then `phase_name` is strdup'ed before registration.
+    // If `sync` is true, then access to the registration table is synchronized.
+    static int get_phase_id(const char* phase_name, bool may_exist, bool use_strdup, bool sync) NOT_JFR_RETURN_(-1);
 
     static void post(EventCompilerPhase& event, const Ticks& start_time, int phase, int compile_id, int level) NOT_JFR_RETURN();
     static void post(EventCompilerPhase& event, jlong start_time, int phase, int compile_id, int level) {

src/hotspot/share/jvmci/jvmciCompilerToVM.cpp

@@ -2639,19 +2639,11 @@ C2V_VMENTRY_0(jlong, ticksNow, (JNIEnv* env, jobject))
   return CompilerEvent::ticksNow();
 }
 
-C2V_VMENTRY_0(jint, registerCompilerPhases, (JNIEnv* env, jobject, jobjectArray jphases))
+C2V_VMENTRY_0(jint, registerCompilerPhase, (JNIEnv* env, jobject, jstring jphase_name))
 #if INCLUDE_JFR
-  if (jphases == NULL) {
-    return -1;
-  }
-  JVMCIObjectArray phases = JVMCIENV->wrap(jphases);
-  int len = JVMCIENV->get_length(phases);
-  GrowableArray<const char*>* jvmci_phase_names = new GrowableArray<const char*>(len);
-  for (int i = 0; i < len; i++) {
-    JVMCIObject phase = JVMCIENV->get_object_at(phases, i);
-    jvmci_phase_names->append(strdup(JVMCIENV->as_utf8_string(phase)));
-  }
-  return CompilerEvent::PhaseEvent::register_phases(jvmci_phase_names);
+  JVMCIObject phase_name = JVMCIENV->wrap(jphase_name);
+  const char *name = JVMCIENV->as_utf8_string(phase_name);
+  return CompilerEvent::PhaseEvent::get_phase_id(name, true, true, true);
 #else
   return -1;
 #endif // !INCLUDE_JFR
@@ -2823,7 +2815,7 @@ JNINativeMethod CompilerToVM::methods[] = {
   {CC "addFailedSpeculation",                         CC "(J[B)Z",                                                                          FN_PTR(addFailedSpeculation)},
   {CC "callSystemExit",                               CC "(I)V",                                                                            FN_PTR(callSystemExit)},
   {CC "ticksNow",                                     CC "()J",                                                                             FN_PTR(ticksNow)},
-  {CC "registerCompilerPhases",                       CC "([" STRING ")I",                                                                  FN_PTR(registerCompilerPhases)},
+  {CC "registerCompilerPhase",                        CC "(" STRING ")I",                                                                   FN_PTR(registerCompilerPhase)},
   {CC "notifyCompilerPhaseEvent",                     CC "(JIII)V",                                                                         FN_PTR(notifyCompilerPhaseEvent)},
   {CC "notifyCompilerInliningEvent",                  CC "(I" HS_RESOLVED_METHOD HS_RESOLVED_METHOD "ZLjava/lang/String;I)V",               FN_PTR(notifyCompilerInliningEvent)},
 };

src/jdk.internal.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/CompilerToVM.java

@@ -999,9 +999,9 @@ HotSpotResolvedObjectTypeImpl getResolvedJavaType(long displacement, boolean com
     /**
      * Adds phases in HotSpot JFR.
      *
-     * @see JFR.CompilerPhaseEvent#registerPhases and JFR.CompilerPhaseEvent#write
+     * @see JFR.CompilerPhaseEvent#write
      */
-    native int registerCompilerPhases(String[] phases);
+    native int registerCompilerPhase(String phaseName);
 
     /**
      * @see JFR.CompilerPhaseEvent#write

src/jdk.internal.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/JFR.java

@@ -21,7 +21,6 @@
  * questions.
  */
 
-
 package jdk.vm.ci.hotspot;
 
 import static jdk.vm.ci.hotspot.CompilerToVM.compilerToVM;
@@ -33,8 +32,9 @@
 import jdk.vm.ci.meta.ResolvedJavaMethod;
 
 /**
- * Helper methods for interacting with the Java Flight Recorder (JFR) to register events and notify it when events occur.
- * The JFR events are defined in {see @code src/share/jfr/metadata/metadata.xml}.
+ * Helper methods for interacting with the Java Flight Recorder (JFR) to register events and notify
+ * it when events occur. The JFR events are defined in {see @code
+ * src/share/jfr/metadata/metadata.xml}.
  */
 public final class JFR {
 
@@ -60,22 +60,7 @@ public static final class CompilerPhaseEvent {
         private static final ConcurrentHashMap<String, Integer> phaseToId = new ConcurrentHashMap<>();
 
         private static int getPhaseToId(String phaseName) {
-            String[] phaseNames = { phaseName };
-            return phaseToId.computeIfAbsent(phaseName, k -> compilerToVM().registerCompilerPhases(phaseNames));
-        }
-
-        /**
-         * Registers new compiler phases with JFR. This should be called during compiler initialization.
-         *
-         * @param phaseNames compiler phase names
-         */
-        public static synchronized void registerPhases(String[] phaseNames) {
-            ArrayList<String> toProcess = new ArrayList<>(Arrays.asList(phaseNames));
-            toProcess.removeAll(phaseToId.keySet());
-            int pid = compilerToVM().registerCompilerPhases(toProcess.toArray(new String[toProcess.size()]));
-            for (String phase : toProcess) {
-                phaseToId.put(phase, pid++);
-            }
+            return phaseToId.computeIfAbsent(phaseName, k -> compilerToVM().registerCompilerPhase(phaseName));
         }
 
         /**
@@ -92,8 +77,8 @@ public static void write(long startTime, String phaseName, int compileId, int ph
     }
 
     /**
-     * Helper methods for managing JFR CompilerInlining events.
-     * The events are defined in {see @code src/share/jfr/metadata/metadata.xml}.
+     * Helper methods for managing JFR CompilerInlining events. The events are defined in {see @code
+     * src/share/jfr/metadata/metadata.xml}.
      */
     public static final class CompilerInliningEvent {
 
@@ -112,4 +97,3 @@ public static void write(int compileId, ResolvedJavaMethod caller, ResolvedJavaM
         }
     }
 }
-