8233111: Epoch shift synchronization point for Compiler threads

Reviewed-by: egahlin

make/src/classes/build/tools/jfr/GenerateJfrFiles.java

@@ -154,6 +154,7 @@ public class GenerateJfrFiles {
         boolean startTime;
         boolean periodic;
         boolean cutoff;
+        String commitState;
     }
 
     static class FieldElement {
@@ -219,14 +220,15 @@ public class GenerateJfrFiles {
                 currentType.name = attributes.getValue("name");
                 break;
             case "Event":
-                EventElement eventtType = new EventElement();
-                eventtType.name = attributes.getValue("name");
-                eventtType.thread = getBoolean(attributes, "thread", false);
-                eventtType.stackTrace = getBoolean(attributes, "stackTrace", false);
-                eventtType.startTime = getBoolean(attributes, "startTime", true);
-                eventtType.periodic = attributes.getValue("period") != null;
-                eventtType.cutoff = getBoolean(attributes, "cutoff", false);
-                currentType = eventtType;
+                EventElement eventType = new EventElement();
+                eventType.name = attributes.getValue("name");
+                eventType.thread = getBoolean(attributes, "thread", false);
+                eventType.stackTrace = getBoolean(attributes, "stackTrace", false);
+                eventType.startTime = getBoolean(attributes, "startTime", true);
+                eventType.periodic = attributes.getValue("period") != null;
+                eventType.cutoff = getBoolean(attributes, "cutoff", false);
+                eventType.commitState = attributes.getValue("commitState");
+                currentType = eventType;
                 break;
             case "Field":
                 currentField = new FieldElement(metadata);
@@ -459,6 +461,7 @@ public class GenerateJfrFiles {
             out.write("#include \"utilities/ticks.hpp\"");
             out.write("#if INCLUDE_JFR");
             out.write("#include \"jfr/recorder/service/jfrEvent.hpp\"");
+            out.write("#include \"jfr/support/jfrEpochSynchronization.hpp\"");
             out.write("/*");
             out.write(" * Each event class has an assert member function verify() which is invoked");
             out.write(" * just before the engine writes the event and its fields to the data stream.");
@@ -523,7 +526,7 @@ public class GenerateJfrFiles {
         }
         out.write("");
         if (!empty) {
-          printWriteData(out, t.fields);
+          printWriteData(out, t.fields, null);
         }
         out.write("};");
         out.write("");
@@ -566,7 +569,7 @@ public class GenerateJfrFiles {
         }
         out.write("");
         if (!empty) {
-          printWriteData(out, event.fields);
+          printWriteData(out, event.fields, event.commitState);
           out.write("");
         }
         out.write("  using JfrEvent<Event" + event.name + ">::commit; // else commit() is hidden by overloaded versions in this class");
@@ -578,9 +581,13 @@ public class GenerateJfrFiles {
         out.write("};");
     }
 
-    private static void printWriteData(Printer out, List<FieldElement> fields) {
+    private static void printWriteData(Printer out, List<FieldElement> fields, String commitState) {
         out.write("  template <typename Writer>");
         out.write("  void writeData(Writer& w) {");
+        if (("_thread_in_native").equals(commitState)) {
+            out.write("    // explicit epoch synchronization check");
+            out.write("    JfrEpochSynchronization sync;");
+        }
         for (FieldElement field : fields) {
             if (field.struct) {
                 out.write("    _" + field.name + ".writeData(w);");

src/hotspot/share/jfr/metadata/metadata.xml

@@ -464,7 +464,7 @@
     <Field type="ulong" contentType="bytes" name="used" label="Used" />
   </Event>
 
-  <Event name="Compilation" category="Java Virtual Machine, Compiler" label="Compilation" thread="true">
+  <Event name="Compilation" category="Java Virtual Machine, Compiler" label="Compilation" thread="true" commitState="_thread_in_native">
     <Field type="uint" name="compileId" label="Compilation Identifier" relation="CompileId" />
     <Field type="CompilerType" name="compiler" label="Compiler" />
     <Field type="Method" name="method" label="Method" />
@@ -492,7 +492,7 @@
     <Field type="string" name="descriptor" label="Method Descriptor" />
   </Type>
 
-  <Event name="CompilerInlining" category="Java Virtual Machine, Compiler, Optimization" label="Method Inlining" thread="true" startTime="false">
+  <Event name="CompilerInlining" category="Java Virtual Machine, Compiler, Optimization" label="Method Inlining" thread="true" startTime="false" commitState="_thread_in_native">
     <Field type="uint" name="compileId" label="Compilation Identifier" relation="CompileId" />
     <Field type="Method" name="caller" label="Caller Method" />
     <Field type="CalleeMethod" name="callee" struct="true" label="Callee Method" />

src/hotspot/share/jfr/metadata/metadata.xsd

@@ -70,6 +70,7 @@
               <xs:attribute name="stackTrace" type="xs:boolean" use="optional" />
               <xs:attribute name="period" type="periodType" use="optional" />
               <xs:attribute name="cutoff" type="xs:boolean" use="optional" />
+              <xs:attribute name="commitState" type="xs:string" use="optional" />
             </xs:complexType>
           </xs:element>
           <xs:element maxOccurs="unbounded" name="Type">

src/hotspot/share/jfr/recorder/checkpoint/jfrCheckpointManager.cpp

@@ -44,9 +44,9 @@
 #include "logging/log.hpp"
 #include "memory/iterator.hpp"
 #include "memory/resourceArea.hpp"
+#include "runtime/atomic.hpp"
 #include "runtime/handles.inline.hpp"
 #include "runtime/mutex.hpp"
-#include "runtime/orderAccess.hpp"
 #include "runtime/os.inline.hpp"
 #include "runtime/safepoint.hpp"
 
@@ -172,7 +172,7 @@ static BufferPtr lease_free(size_t size, JfrCheckpointMspace* mspace, size_t ret
 }
 
 bool JfrCheckpointManager::use_epoch_transition_mspace(const Thread* thread) const {
-  return _service_thread != thread && _checkpoint_epoch_state != JfrTraceIdEpoch::epoch();
+  return _service_thread != thread && Atomic::load_acquire(&_checkpoint_epoch_state) != JfrTraceIdEpoch::epoch();
 }
 
 static const size_t lease_retry = 10;
@@ -333,7 +333,19 @@ static size_t write_mspace(JfrCheckpointMspace* mspace, JfrChunkWriter& chunkwri
   return wo.processed();
 }
 
-void JfrCheckpointManager::synchronize_epoch() {
+void JfrCheckpointManager::begin_epoch_shift() {
+  assert(SafepointSynchronize::is_at_safepoint(), "invariant");
+  JfrTraceIdEpoch::begin_epoch_shift();
+}
+
+void JfrCheckpointManager::end_epoch_shift() {
+  assert(SafepointSynchronize::is_at_safepoint(), "invariant");
+  debug_only(const u1 current_epoch = JfrTraceIdEpoch::current();)
+  JfrTraceIdEpoch::end_epoch_shift();
+  assert(current_epoch != JfrTraceIdEpoch::current(), "invariant");
+}
+
+void JfrCheckpointManager::synchronize_checkpoint_manager_with_current_epoch() {
   assert(_checkpoint_epoch_state != JfrTraceIdEpoch::epoch(), "invariant");
   OrderAccess::storestore();
   _checkpoint_epoch_state = JfrTraceIdEpoch::epoch();
@@ -341,7 +353,7 @@ void JfrCheckpointManager::synchronize_epoch() {
 
 size_t JfrCheckpointManager::write() {
   const size_t processed = write_mspace<MutexedWriteOp, CompositeOperation>(_free_list_mspace, _chunkwriter);
-  synchronize_epoch();
+  synchronize_checkpoint_manager_with_current_epoch();
   return processed;
 }
 
@@ -365,7 +377,7 @@ size_t JfrCheckpointManager::clear() {
   DiscardOperation discarder(mutexed); // mutexed discard mode
   process_free_list(discarder, _free_list_mspace);
   process_free_list(discarder, _epoch_transition_mspace);
-  synchronize_epoch();
+  synchronize_checkpoint_manager_with_current_epoch();
   return discarder.elements();
 }
 
@@ -410,12 +422,6 @@ size_t JfrCheckpointManager::write_static_type_set_and_threads() {
   return write_epoch_transition_mspace();
 }
 
-void JfrCheckpointManager::shift_epoch() {
-  debug_only(const u1 current_epoch = JfrTraceIdEpoch::current();)
-  JfrTraceIdEpoch::shift_epoch();
-  assert(current_epoch != JfrTraceIdEpoch::current(), "invariant");
-}
-
 void JfrCheckpointManager::on_rotation() {
   assert(SafepointSynchronize::is_at_safepoint(), "invariant");
   JfrTypeManager::on_rotation();

src/hotspot/share/jfr/recorder/checkpoint/jfrCheckpointManager.hpp

@@ -88,8 +88,10 @@ class JfrCheckpointManager : public JfrCHeapObj {
   void write_type_set();
   static void write_type_set_for_unloaded_classes();
 
-  void shift_epoch();
-  void synchronize_epoch();
+  void begin_epoch_shift();
+  void end_epoch_shift();
+  void synchronize_checkpoint_manager_with_current_epoch();
+
   void notify_threads();
 
   JfrCheckpointManager(JfrChunkWriter& cw);

src/hotspot/share/jfr/recorder/checkpoint/types/jfrTypeSet.cpp

@@ -303,7 +303,7 @@ static bool write_klasses() {
     _subsystem_callback = &callback;
     do_klasses();
   } else {
-    LeakKlassWriter lkw(_leakp_writer, _artifacts, _class_unload);
+    LeakKlassWriter lkw(_leakp_writer, _class_unload);
     CompositeKlassWriter ckw(&lkw, &kw);
     CompositeKlassWriterRegistration ckwr(&ckw, &reg);
     CompositeKlassCallback callback(&ckwr);

src/hotspot/share/jfr/recorder/checkpoint/types/traceid/jfrTraceId.inline.hpp

@@ -61,6 +61,7 @@ inline traceid JfrTraceId::get(const Thread* t) {
 }
 
 inline traceid JfrTraceId::use(const Klass* klass) {
+  assert(klass != NULL, "invariant");
   return set_used_and_get(klass);
 }
 
@@ -71,15 +72,12 @@ inline traceid JfrTraceId::use(const Method* method) {
 inline traceid JfrTraceId::use(const Klass* klass, const Method* method) {
   assert(klass != NULL, "invariant");
   assert(method != NULL, "invariant");
-  if (SHOULD_TAG_KLASS_METHOD(klass)) {
-    SET_METHOD_AND_CLASS_USED_THIS_EPOCH(klass);
-  }
-  assert(METHOD_AND_CLASS_USED_THIS_EPOCH(klass), "invariant");
   if (METHOD_FLAG_NOT_USED_THIS_EPOCH(method)) {
-    assert(USED_THIS_EPOCH(klass), "invariant");
+    SET_METHOD_AND_CLASS_USED_THIS_EPOCH(klass);
     SET_METHOD_FLAG_USED_THIS_EPOCH(method);
     JfrTraceIdEpoch::set_changed_tag_state();
   }
+  assert(METHOD_AND_CLASS_USED_THIS_EPOCH(klass), "invariant");
   assert(METHOD_FLAG_USED_THIS_EPOCH(method), "invariant");
   return (METHOD_ID(klass, method));
 }

src/hotspot/share/jfr/recorder/checkpoint/types/traceid/jfrTraceIdEpoch.cpp

@@ -26,12 +26,20 @@
 #include "jfr/recorder/checkpoint/types/traceid/jfrTraceIdEpoch.hpp"
 #include "runtime/safepoint.hpp"
 
-// Alternating epochs on each rotation allow for concurrent tagging.
-// The epoch shift happens only during a safepoint.
 bool JfrTraceIdEpoch::_epoch_state = false;
-bool volatile JfrTraceIdEpoch::_tag_state = false;
+bool JfrTraceIdEpoch::_synchronizing = false;
+volatile bool JfrTraceIdEpoch::_changed_tag_state = false;
 
-void JfrTraceIdEpoch::shift_epoch() {
+void JfrTraceIdEpoch::begin_epoch_shift() {
   assert(SafepointSynchronize::is_at_safepoint(), "invariant");
-  _epoch_state = !_epoch_state;
+  _synchronizing = true;
+  OrderAccess::fence();
+}
+
+void JfrTraceIdEpoch::end_epoch_shift() {
+  assert(SafepointSynchronize::is_at_safepoint(), "invariant");
+  assert(_synchronizing, "invariant");
+  _epoch_state = !_epoch_state;
+  OrderAccess::storestore();
+  _synchronizing = false;
 }

src/hotspot/share/jfr/recorder/checkpoint/types/traceid/jfrTraceIdEpoch.hpp

@@ -41,13 +41,33 @@
 #define METHOD_AND_CLASS_IN_USE_EPOCH_1_BITS (METHOD_AND_CLASS_IN_USE_BITS << EPOCH_1_SHIFT)
 #define METHOD_AND_CLASS_IN_USE_EPOCH_2_BITS (METHOD_AND_CLASS_IN_USE_BITS << EPOCH_2_SHIFT)
 
+ // Epoch alternation on each rotation allow for concurrent tagging.
+ // The epoch shift happens only during a safepoint.
+ //
+ // _synchronizing is a transition state, the purpose of which is to
+ // have JavaThreads that run _thread_in_native (i.e. Compiler threads)
+ // respect the current epoch shift in-progress during a safepoint.
+ //
+ // _changed_tag_state == true signals an incremental modification to artifact tagging
+ // (klasses, methods, CLDs, etc), used to request collection of artifacts.
+ //
 class JfrTraceIdEpoch : AllStatic {
   friend class JfrCheckpointManager;
  private:
   static bool _epoch_state;
-  static bool volatile _tag_state;
+  static bool _synchronizing;
+  static volatile bool _changed_tag_state;
 
-  static void shift_epoch();
+  static void begin_epoch_shift();
+  static void end_epoch_shift();
+
+  static bool changed_tag_state() {
+    return Atomic::load_acquire(&_changed_tag_state);
+  }
+
+  static void set_tag_state(bool value) {
+    Atomic::release_store(&_changed_tag_state, value);
+  }
 
  public:
   static bool epoch() {
@@ -66,6 +86,10 @@ class JfrTraceIdEpoch : AllStatic {
     return _epoch_state ? (u1)0 : (u1)1;
   }
 
+  static bool is_synchronizing() {
+    return Atomic::load_acquire(&_synchronizing);
+  }
+
   static traceid in_use_this_epoch_bit() {
     return _epoch_state ? USED_EPOCH_2_BIT : USED_EPOCH_1_BIT;
   }
@@ -91,16 +115,16 @@ class JfrTraceIdEpoch : AllStatic {
   }
 
   static bool has_changed_tag_state() {
-    if (Atomic::load_acquire(&_tag_state)) {
-      Atomic::release_store(&_tag_state, false);
+    if (changed_tag_state()) {
+      set_tag_state(false);
       return true;
     }
     return false;
   }
 
   static void set_changed_tag_state() {
-    if (!Atomic::load_acquire(&_tag_state)) {
-      Atomic::release_store(&_tag_state, true);
+    if (!changed_tag_state()) {
+      set_tag_state(true);
     }
   }
 };

src/hotspot/share/jfr/recorder/service/jfrRecorderService.cpp

@@ -46,9 +46,9 @@
 #include "jfr/utilities/jfrTypes.hpp"
 #include "logging/log.hpp"
 #include "memory/resourceArea.hpp"
+#include "runtime/atomic.hpp"
 #include "runtime/handles.inline.hpp"
 #include "runtime/mutexLocker.hpp"
-#include "runtime/orderAccess.hpp"
 #include "runtime/os.hpp"
 #include "runtime/safepoint.hpp"
 #include "runtime/thread.inline.hpp"
@@ -390,11 +390,12 @@ void JfrRecorderService::invoke_safepoint_clear() {
 
 void JfrRecorderService::safepoint_clear() {
   assert(SafepointSynchronize::is_at_safepoint(), "invariant");
+  _checkpoint_manager.begin_epoch_shift();
   _string_pool.clear();
-  _storage.clear();
-  _checkpoint_manager.shift_epoch();
-  _chunkwriter.set_time_stamp();
   _stack_trace_repository.clear();
+  _storage.clear();
+  _chunkwriter.set_time_stamp();
+  _checkpoint_manager.end_epoch_shift();
 }
 
 void JfrRecorderService::post_safepoint_clear() {
@@ -559,14 +560,15 @@ void JfrRecorderService::safepoint_write() {
   assert(SafepointSynchronize::is_at_safepoint(), "invariant");
   assert(is_rotation_safepoint_pending(), "invariant");
   set_rotation_safepoint_pending(false);
+  _checkpoint_manager.begin_epoch_shift();
   if (_string_pool.is_modified()) {
     write_stringpool_safepoint(_string_pool, _chunkwriter);
   }
   _checkpoint_manager.on_rotation();
-  _storage.write_at_safepoint();
-  _checkpoint_manager.shift_epoch();
-  _chunkwriter.set_time_stamp();
   write_stacktrace(_stack_trace_repository, _chunkwriter, true);
+  _storage.write_at_safepoint();
+  _chunkwriter.set_time_stamp();
+  _checkpoint_manager.end_epoch_shift();
 }
 
 void JfrRecorderService::post_safepoint_write() {

src/hotspot/share/jfr/support/jfrEpochSynchronization.cpp

@@ -0,0 +1,42 @@
+/*
+* Copyright (c) 2019, 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.
+*
+* 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.
+*
+*/
+
+#include "precompiled.hpp"
+#include "jfr/recorder/checkpoint/types/traceid/jfrTraceIdEpoch.hpp"
+#include "jfr/support/jfrEpochSynchronization.hpp"
+#include "runtime/interfaceSupport.inline.hpp"
+#include "runtime/thread.inline.hpp"
+
+JfrEpochSynchronization::JfrEpochSynchronization() {
+  if (JfrTraceIdEpoch::is_synchronizing()) {
+    // only at a safepoint
+    Thread* thread = Thread::current();
+    assert(thread != NULL, "invariant");
+    assert(thread->is_Java_thread(), "invariant");
+    JavaThread* const jt = (JavaThread*)thread;
+    assert(jt->thread_state() == _thread_in_native, "invariant");
+    // use ordinary transition to have the thread block and await the new epoch
+    ThreadInVMfromNative transition(jt);
+  }
+}

src/hotspot/share/jfr/support/jfrEpochSynchronization.hpp

@@ -0,0 +1,37 @@
+/*
+* Copyright (c) 2019, 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.
+*
+* 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.
+*
+*/
+
+#ifndef SHARE_JFR_SUPPORT_JFREPOCHSYNCHRONIZATION_HPP
+#define SHARE_JFR_SUPPORT_JFREPOCHSYNCHRONIZATION_HPP
+
+/*
+ * JavaThreads running _thread_in_native (Compiler threads) must synchronize
+ * with the upcoming epoch in case there is an epoch shift in-progress.
+ */
+class JfrEpochSynchronization {
+ public:
+  JfrEpochSynchronization();
+};
+
+#endif // SHARE_JFR_SUPPORT_JFREPOCHSYNCHRONIZATION_HPP