Metadata races (TestStart) and thread exclusion inclusion (TestRecursive) JEP-349-branch
authormgronlun
Fri, 11 Oct 2019 19:46:05 +0200
branchJEP-349-branch
changeset 58567 e77a97d0edbb
parent 58540 43229acd75ea
child 58569 5469bde803fe
Metadata races (TestStart) and thread exclusion inclusion (TestRecursive)
src/hotspot/share/jfr/jfr.cpp
src/hotspot/share/jfr/jfr.hpp
src/hotspot/share/jfr/jni/jfrJniMethod.cpp
src/hotspot/share/jfr/jni/jfrJniMethod.hpp
src/hotspot/share/jfr/jni/jfrJniMethodRegistration.cpp
src/hotspot/share/jfr/recorder/checkpoint/jfrMetadataEvent.cpp
src/hotspot/share/jfr/recorder/repository/jfrRepository.cpp
src/hotspot/share/jfr/recorder/repository/jfrRepository.hpp
src/hotspot/share/jfr/recorder/service/jfrPostBox.cpp
src/hotspot/share/jfr/recorder/service/jfrRecorderService.cpp
src/hotspot/share/jfr/recorder/service/jfrRecorderService.hpp
src/hotspot/share/jfr/recorder/service/jfrRecorderThread.cpp
src/hotspot/share/jfr/recorder/service/jfrRecorderThreadLoop.cpp
src/hotspot/share/jfr/support/jfrThreadLocal.cpp
src/hotspot/share/jfr/support/jfrThreadLocal.hpp
src/jdk.jfr/share/classes/jdk/jfr/internal/JVM.java
src/jdk.jfr/share/classes/jdk/jfr/internal/MetadataRepository.java
--- a/src/hotspot/share/jfr/jfr.cpp	Thu Oct 10 17:36:57 2019 +0200
+++ b/src/hotspot/share/jfr/jfr.cpp	Fri Oct 11 19:46:05 2019 +0200
@@ -71,15 +71,15 @@
   JfrThreadLocal::on_exit(t);
 }
 
-void Jfr::exclude_thread(const Thread* t) {
+void Jfr::exclude_thread(Thread* t) {
   JfrThreadLocal::exclude(t);
 }
 
-void Jfr::include_thread(const Thread* t) {
+void Jfr::include_thread(Thread* t) {
   JfrThreadLocal::include(t);
 }
 
-bool Jfr::is_excluded(const Thread* t) {
+bool Jfr::is_excluded(Thread* t) {
   return t != NULL && t->jfr_thread_local()->is_excluded();
 }
 
--- a/src/hotspot/share/jfr/jfr.hpp	Thu Oct 10 17:36:57 2019 +0200
+++ b/src/hotspot/share/jfr/jfr.hpp	Fri Oct 11 19:46:05 2019 +0200
@@ -48,9 +48,9 @@
   static void on_unloading_classes();
   static void on_thread_start(Thread* thread);
   static void on_thread_exit(Thread* thread);
-  static void exclude_thread(const Thread* thread);
-  static bool is_excluded(const Thread* thread);
-  static void include_thread(const Thread* thread);
+  static void exclude_thread(Thread* thread);
+  static bool is_excluded(Thread* thread);
+  static void include_thread(Thread* thread);
   static void on_java_thread_dismantle(JavaThread* jt);
   static void on_vm_shutdown(bool exception_handler = false);
   static bool on_flight_recorder_option(const JavaVMOption** option, char* delimiter);
--- a/src/hotspot/share/jfr/jni/jfrJniMethod.cpp	Thu Oct 10 17:36:57 2019 +0200
+++ b/src/hotspot/share/jfr/jni/jfrJniMethod.cpp	Fri Oct 11 19:46:05 2019 +0200
@@ -284,8 +284,8 @@
   return JfrJavaEventWriter::flush(writer, used_size, requested_size, thread);
 JVM_END
 
-JVM_ENTRY_NO_ENV(void, jfr_flush(JNIEnv* env, jobject jvm, jboolean include_metadata))
-  JfrRepository::flush(include_metadata == JNI_TRUE, thread);
+JVM_ENTRY_NO_ENV(void, jfr_flush(JNIEnv* env, jobject jvm))
+  JfrRepository::flush(thread);
 JVM_END
 
 JVM_ENTRY_NO_ENV(void, jfr_set_repository_location(JNIEnv* env, jobject repo, jstring location))
--- a/src/hotspot/share/jfr/jni/jfrJniMethod.hpp	Thu Oct 10 17:36:57 2019 +0200
+++ b/src/hotspot/share/jfr/jni/jfrJniMethod.hpp	Fri Oct 11 19:46:05 2019 +0200
@@ -113,7 +113,7 @@
 
 jboolean JNICALL jfr_event_writer_flush(JNIEnv* env, jclass cls, jobject writer, jint used_size, jint requested_size);
 
-void JNICALL jfr_flush(JNIEnv* env, jobject jvm, jboolean include_metadata);
+void JNICALL jfr_flush(JNIEnv* env, jobject jvm);
 void JNICALL jfr_abort(JNIEnv* env, jobject jvm, jstring errorMsg);
 
 jlong JNICALL jfr_get_epoch_address(JNIEnv* env, jobject jvm);
--- a/src/hotspot/share/jfr/jni/jfrJniMethodRegistration.cpp	Thu Oct 10 17:36:57 2019 +0200
+++ b/src/hotspot/share/jfr/jni/jfrJniMethodRegistration.cpp	Fri Oct 11 19:46:05 2019 +0200
@@ -70,7 +70,7 @@
       (char*)"getEventWriter", (char*)"()Ljava/lang/Object;", (void*)jfr_get_event_writer,
       (char*)"newEventWriter", (char*)"()Ljdk/jfr/internal/EventWriter;", (void*)jfr_new_event_writer,
       (char*)"flush", (char*)"(Ljdk/jfr/internal/EventWriter;II)Z", (void*)jfr_event_writer_flush,
-      (char*)"flush", (char*)"(Z)V", (void*)jfr_flush,
+      (char*)"flush", (char*)"()V", (void*)jfr_flush,
       (char*)"setRepositoryLocation", (char*)"(Ljava/lang/String;)V", (void*)jfr_set_repository_location,
       (char*)"abort", (char*)"(Ljava/lang/String;)V", (void*)jfr_abort,
       (char*)"getEpochAddress", (char*)"()J",(void*)jfr_get_epoch_address,
--- a/src/hotspot/share/jfr/recorder/checkpoint/jfrMetadataEvent.cpp	Thu Oct 10 17:36:57 2019 +0200
+++ b/src/hotspot/share/jfr/recorder/checkpoint/jfrMetadataEvent.cpp	Fri Oct 11 19:46:05 2019 +0200
@@ -33,7 +33,7 @@
 
 static jbyteArray metadata_blob = NULL;
 static u8 metadata_id = 0;
-static u8 last_written_metadata_id = 0;
+static u8 last_metadata_id = 0;
 
 static void write_metadata_blob(JfrChunkWriter& chunkwriter) {
   assert(metadata_blob != NULL, "invariant");
@@ -49,7 +49,7 @@
 
 void JfrMetadataEvent::write(JfrChunkWriter& chunkwriter) {
   assert(chunkwriter.is_valid(), "invariant");
-  if (last_written_metadata_id == metadata_id && chunkwriter.has_metadata()) {
+  if (last_metadata_id == metadata_id && chunkwriter.has_metadata()) {
     return;
   }
   // header
@@ -60,11 +60,11 @@
   chunkwriter.write((u8)0); // duration
   chunkwriter.write(metadata_id); // metadata id
   write_metadata_blob(chunkwriter); // payload
-  last_written_metadata_id = metadata_id;
   // fill in size of metadata descriptor event
   const int64_t size_written = chunkwriter.current_offset() - metadata_offset;
   chunkwriter.write_padded_at_offset((u4)size_written, metadata_offset);
   chunkwriter.set_last_metadata_offset(metadata_offset);
+  last_metadata_id = metadata_id;
 }
 
 void JfrMetadataEvent::update(jbyteArray metadata) {
--- a/src/hotspot/share/jfr/recorder/repository/jfrRepository.cpp	Thu Oct 10 17:36:57 2019 +0200
+++ b/src/hotspot/share/jfr/recorder/repository/jfrRepository.cpp	Fri Oct 11 19:46:05 2019 +0200
@@ -165,7 +165,7 @@
   return _chunkwriter->close();
 }
 
-void JfrRepository::flush(bool metadata, JavaThread* jt) {
+void JfrRepository::flush(JavaThread* jt) {
   DEBUG_ONLY(JfrJavaSupport::check_java_thread_in_vm(jt));
   if (!Jfr::is_recording()) {
     return;
@@ -173,7 +173,7 @@
   if (!_chunkwriter->is_valid()) {
     return;
   }
-  instance()._post_box.post((metadata || !_chunkwriter->has_metadata()) ? MSG_FLUSHPOINT_METADATA : MSG_FLUSHPOINT);
+  instance()._post_box.post(MSG_FLUSHPOINT);
 }
 
 size_t JfrRepository::flush_chunk() {
--- a/src/hotspot/share/jfr/recorder/repository/jfrRepository.hpp	Thu Oct 10 17:36:57 2019 +0200
+++ b/src/hotspot/share/jfr/recorder/repository/jfrRepository.hpp	Fri Oct 11 19:46:05 2019 +0200
@@ -70,7 +70,7 @@
  public:
   static void set_path(jstring location, JavaThread* jt);
   static void set_chunk_path(jstring path, JavaThread* jt);
-  static void flush(bool metadata, JavaThread* jt);
+  static void flush(JavaThread* jt);
   static jlong current_chunk_start_nanos();
 };
 
--- a/src/hotspot/share/jfr/recorder/service/jfrPostBox.cpp	Thu Oct 10 17:36:57 2019 +0200
+++ b/src/hotspot/share/jfr/recorder/service/jfrPostBox.cpp	Fri Oct 11 19:46:05 2019 +0200
@@ -33,8 +33,7 @@
                              (MSGBIT(MSG_START))  |          \
                              (MSGBIT(MSG_CLONE_IN_MEMORY)) | \
                              (MSGBIT(MSG_VM_ERROR))        | \
-                             (MSGBIT(MSG_FLUSHPOINT))      | \
-                             (MSGBIT(MSG_FLUSHPOINT_METADATA)) \
+                             (MSGBIT(MSG_FLUSHPOINT))        \
                            )
 
 static JfrPostBox* _instance = NULL;
--- a/src/hotspot/share/jfr/recorder/service/jfrRecorderService.cpp	Thu Oct 10 17:36:57 2019 +0200
+++ b/src/hotspot/share/jfr/recorder/service/jfrRecorderService.cpp	Fri Oct 11 19:46:05 2019 +0200
@@ -647,10 +647,7 @@
 static bool write_metadata_in_flushpoint = false;
 
 size_t JfrRecorderService::flush() {
-  size_t total_elements = 0;
-  if (write_metadata_in_flushpoint) {
-    total_elements = flush_metadata(_chunkwriter);
-  }
+  size_t total_elements = flush_metadata(_chunkwriter);
   const size_t storage_elements = flush_storage(_storage, _chunkwriter);
   if (0 == storage_elements) {
     return total_elements;
@@ -674,11 +671,10 @@
 typedef Content<EventFlush, JfrRecorderService, &JfrRecorderService::flush> FlushFunctor;
 typedef WriteContent<FlushFunctor> Flush;
 
-void JfrRecorderService::flush(int msgs) {
+void JfrRecorderService::flushpoint() {
   assert(_chunkwriter.is_valid(), "invariant");
   ResourceMark rm;
   HandleMark hm;
-  write_metadata_in_flushpoint = (msgs & MSGBIT(MSG_FLUSHPOINT_METADATA));
   ++flushpoint_id;
   reset_thread_local_buffer();
   FlushFunctor flushpoint(*this);
--- a/src/hotspot/share/jfr/recorder/service/jfrRecorderService.hpp	Thu Oct 10 17:36:57 2019 +0200
+++ b/src/hotspot/share/jfr/recorder/service/jfrRecorderService.hpp	Fri Oct 11 19:46:05 2019 +0200
@@ -66,7 +66,7 @@
   JfrRecorderService();
   void start();
   void rotate(int msgs);
-  void flush(int msgs);
+  void flushpoint();
   void process_full_buffers();
   void scavenge();
   void evaluate_chunk_size_for_rotation();
--- a/src/hotspot/share/jfr/recorder/service/jfrRecorderThread.cpp	Thu Oct 10 17:36:57 2019 +0200
+++ b/src/hotspot/share/jfr/recorder/service/jfrRecorderThread.cpp	Fri Oct 11 19:46:05 2019 +0200
@@ -98,7 +98,7 @@
   instanceHandle h_thread_oop(THREAD, (instanceOop)result.get_jobject());
   assert(h_thread_oop.not_null(), "invariant");
   // attempt thread start
-  const Thread* const t = start_thread(h_thread_oop, recorderthread_entry,THREAD);
+  Thread* const t = start_thread(h_thread_oop, recorderthread_entry,THREAD);
   if (!HAS_PENDING_EXCEPTION) {
     Jfr::exclude_thread(t);
     cp_manager->register_service_thread(t);
--- a/src/hotspot/share/jfr/recorder/service/jfrRecorderThreadLoop.cpp	Thu Oct 10 17:36:57 2019 +0200
+++ b/src/hotspot/share/jfr/recorder/service/jfrRecorderThreadLoop.cpp	Fri Oct 11 19:46:05 2019 +0200
@@ -40,7 +40,7 @@
   #define START (msgs & (MSGBIT(MSG_START)))
   #define SHUTDOWN (msgs & MSGBIT(MSG_SHUTDOWN))
   #define ROTATE (msgs & (MSGBIT(MSG_ROTATE)|MSGBIT(MSG_STOP)))
-  #define FLUSHPOINT (msgs & (MSGBIT(MSG_FLUSHPOINT)|MSGBIT(MSG_FLUSHPOINT_METADATA)))
+  #define FLUSHPOINT (msgs & (MSGBIT(MSG_FLUSHPOINT)))
   #define PROCESS_FULL_BUFFERS (msgs & (MSGBIT(MSG_ROTATE)|MSGBIT(MSG_STOP)|MSGBIT(MSG_FULLBUFFER)))
   #define SCAVENGE (msgs & (MSGBIT(MSG_DEADBUFFER)))
 
@@ -74,7 +74,7 @@
       } else if (ROTATE) {
         service.rotate(msgs);
       } else if (FLUSHPOINT) {
-        service.flush(msgs);
+        service.flushpoint();
       }
       JfrMsg_lock->lock();
       post_box.notify_waiters();
--- a/src/hotspot/share/jfr/support/jfrThreadLocal.cpp	Thu Oct 10 17:36:57 2019 +0200
+++ b/src/hotspot/share/jfr/support/jfrThreadLocal.cpp	Fri Oct 11 19:46:05 2019 +0200
@@ -108,6 +108,26 @@
   }
 }
 
+void JfrThreadLocal::release(Thread* t) {
+  if (has_java_event_writer()) {
+    assert(t->is_Java_thread(), "invariant");
+    JfrJavaSupport::destroy_global_jni_handle(java_event_writer());
+    _java_event_writer = NULL;
+  }
+  if (has_native_buffer()) {
+    JfrStorage::release_thread_local(native_buffer(), t);
+    _native_buffer = NULL;
+  }
+  if (has_java_buffer()) {
+    JfrStorage::release_thread_local(java_buffer(), t);
+    _java_buffer = NULL;
+  }
+  if (_stackframes != NULL) {
+    FREE_C_HEAP_ARRAY(JfrStackFrame, _stackframes);
+    _stackframes = NULL;
+  }
+}
+
 void JfrThreadLocal::release(JfrThreadLocal* tl, Thread* t) {
   assert(tl != NULL, "invariant");
   assert(t != NULL, "invariant");
@@ -115,19 +135,7 @@
   assert(!tl->is_dead(), "invariant");
   assert(tl->shelved_buffer() == NULL, "invariant");
   tl->_dead = true;
-  if (tl->has_java_event_writer()) {
-    assert(t->is_Java_thread(), "invariant");
-    const jobject event_writer = tl->java_event_writer();
-    tl->set_java_event_writer(NULL);
-    JfrJavaSupport::destroy_global_jni_handle(event_writer);
-  }
-  if (tl->has_native_buffer()) {
-    JfrStorage::release_thread_local(tl->native_buffer(), t);
-  }
-  if (tl->has_java_buffer()) {
-    JfrStorage::release_thread_local(tl->java_buffer(), t);
-  }
-  FREE_C_HEAP_ARRAY(JfrStackFrame, tl->_stackframes);
+  tl->release(t);
 }
 
 void JfrThreadLocal::on_exit(Thread* t) {
@@ -179,14 +187,16 @@
   return in_ByteSize(offset_of(JfrThreadLocal, _java_event_writer));
 }
 
-void JfrThreadLocal::exclude(const Thread* t) {
+void JfrThreadLocal::exclude(Thread* t) {
   assert(t != NULL, "invariant");
   t->jfr_thread_local()->_excluded = true;
+  t->jfr_thread_local()->release(t);
 }
 
-void JfrThreadLocal::include(const Thread* t) {
+void JfrThreadLocal::include(Thread* t) {
   assert(t != NULL, "invariant");
   t->jfr_thread_local()->_excluded = false;
+  t->jfr_thread_local()->release(t);
 }
 
 u4 JfrThreadLocal::stackdepth() const {
--- a/src/hotspot/share/jfr/support/jfrThreadLocal.hpp	Thu Oct 10 17:36:57 2019 +0200
+++ b/src/hotspot/share/jfr/support/jfrThreadLocal.hpp	Fri Oct 11 19:46:05 2019 +0200
@@ -56,7 +56,7 @@
   JfrBuffer* install_native_buffer() const;
   JfrBuffer* install_java_buffer() const;
   JfrStackFrame* install_stackframes() const;
-
+  void release(Thread* t);
   static void release(JfrThreadLocal* tl, Thread* t);
 
  public:
@@ -216,8 +216,8 @@
   void set_thread_blob(const JfrBlobHandle& handle);
   const JfrBlobHandle& thread_blob() const;
 
-  static void exclude(const Thread* t);
-  static void include(const Thread* t);
+  static void exclude(Thread* t);
+  static void include(Thread* t);
 
   static void on_start(Thread* t);
   static void on_exit(Thread* t);
--- a/src/jdk.jfr/share/classes/jdk/jfr/internal/JVM.java	Thu Oct 10 17:36:57 2019 +0200
+++ b/src/jdk.jfr/share/classes/jdk/jfr/internal/JVM.java	Fri Oct 11 19:46:05 2019 +0200
@@ -466,10 +466,8 @@
      * pointers to the metadata event, last check point event, correct file size and
      * the generation id.
      *
-     * @param includeMetadata {@code true} if metadata event should be written, {@code false}
-     *        otherwise
      */
-    public native void flush(boolean includeMetadata);
+    public native void flush();
     /**
      * Sets the location of the disk repository, to be used at an emergency
      * dump.
--- a/src/jdk.jfr/share/classes/jdk/jfr/internal/MetadataRepository.java	Thu Oct 10 17:36:57 2019 +0200
+++ b/src/jdk.jfr/share/classes/jdk/jfr/internal/MetadataRepository.java	Fri Oct 11 19:46:05 2019 +0200
@@ -62,7 +62,6 @@
     private boolean staleMetadata = true;
     private boolean unregistered;
     private long lastUnloaded = -1;
-    private boolean flushMetadata;
 
     public MetadataRepository() {
         initializeJVMEventTypes();
@@ -144,7 +143,6 @@
         typeLibrary.addType(handler.getPlatformEventType());
         if (jvm.isRecording()) {
             storeDescriptorInJVM(); // needed for emergency dump
-            flushMetadata = true;
             settingsManager.setEventControl(handler.getEventControl());
             settingsManager.updateRetransform(Collections.singletonList((eventClass)));
         } else {
@@ -265,20 +263,21 @@
     // Lock around setOutput ensures that other threads don't
     // emit events after setOutput and unregister the event class, before a call
     // to storeDescriptorInJVM
-    synchronized void setOutput(String filename) {
+    synchronized void setOutput(String filename) {
+        if (staleMetadata) {
+            storeDescriptorInJVM();
+        }
         jvm.setOutput(filename);
         if (filename != null) {
             RepositoryFiles.notifyNewFile();
         }
-        flushMetadata = false;
         unregisterUnloaded();
         if (unregistered) {
-            staleMetadata = typeLibrary.clearUnregistered();
+            if (typeLibrary.clearUnregistered()) {
+                storeDescriptorInJVM();
+            }
             unregistered = false;
         }
-        if (staleMetadata) {
-            storeDescriptorInJVM();
-        }
     }
 
     private void unregisterUnloaded() {
@@ -318,11 +317,10 @@
     }
 
     public synchronized void flush() {
-        jvm.flush(flushMetadata);
-        this.flushMetadata = false;
+        if (staleMetadata) {
+            storeDescriptorInJVM();
+        }
+        jvm.flush();
     }
 
-
-
-
 }