8210024: JFR calls virtual is_Java_thread from ~Thread()

Reviewed-by: kbarrett, dholmes, dcubed, egahlin

src/hotspot/share/jfr/jfr.cpp

@@ -63,13 +63,17 @@ void Jfr::on_unloading_classes() {
   }
 }
 
-void Jfr::on_thread_exit(JavaThread* thread) {
-  JfrThreadLocal::on_exit(thread);
+void Jfr::on_thread_start(Thread* t) {
+  JfrThreadLocal::on_start(t);
 }
 
-void Jfr::on_thread_destruct(Thread* thread) {
-  if (JfrRecorder::is_created()) {
-    JfrThreadLocal::on_destruct(thread);
+void Jfr::on_thread_exit(Thread* t) {
+  JfrThreadLocal::on_exit(t);
+}
+
+void Jfr::on_java_thread_dismantle(JavaThread* jt) {
+  if (JfrRecorder::is_recording()) {
+    JfrCheckpointManager::write_thread_checkpoint(jt);
   }
 }
 
@@ -90,7 +94,3 @@ bool Jfr::on_flight_recorder_option(const JavaVMOption** option, char* delimiter
 bool Jfr::on_start_flight_recording_option(const JavaVMOption** option, char* delimiter) {
   return JfrOptionSet::parse_start_flight_recording_option(option, delimiter);
 }
-
-Thread* Jfr::sampler_thread() {
-  return JfrThreadSampling::sampler_thread();
-}

src/hotspot/share/jfr/jfr.hpp

@@ -46,13 +46,13 @@ class Jfr : AllStatic {
   static void on_vm_init();
   static void on_vm_start();
   static void on_unloading_classes();
-  static void on_thread_exit(JavaThread* thread);
-  static void on_thread_destruct(Thread* thread);
+  static void on_thread_start(Thread* thread);
+  static void on_thread_exit(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);
   static bool on_start_flight_recording_option(const JavaVMOption** option, char* delimiter);
   static void weak_oops_do(BoolObjectClosure* is_alive, OopClosure* f);
-  static Thread* sampler_thread();
 };
 
 #endif // SHARE_VM_JFR_JFR_HPP

src/hotspot/share/jfr/periodic/sampling/jfrThreadSampler.cpp

@@ -618,10 +618,3 @@ void JfrThreadSampling::set_native_sample_interval(size_t period) {
 void JfrThreadSampling::on_javathread_suspend(JavaThread* thread) {
   JfrThreadSampler::on_javathread_suspend(thread);
 }
-
-Thread* JfrThreadSampling::sampler_thread() {
-  if (_instance == NULL) {
-    return NULL;
-  }
-  return _instance->_sampler != NULL ? _instance->_sampler->_sampler_thread : NULL;
-}

src/hotspot/share/jfr/periodic/sampling/jfrThreadSampler.hpp

@@ -50,7 +50,6 @@ class JfrThreadSampling : public JfrCHeapObj {
   static void set_java_sample_interval(size_t period);
   static void set_native_sample_interval(size_t period);
   static void on_javathread_suspend(JavaThread* thread);
-  static Thread* sampler_thread();
 };
 
 #endif // SHARE_VM_JFR_PERIODIC_SAMPLING_JFRTHREADSAMPLER_HPP

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

@@ -23,6 +23,7 @@
  */
 
 #include "precompiled.hpp"
+#include "jfr/jfrEvents.hpp"
 #include "jfr/jni/jfrJavaSupport.hpp"
 #include "jfr/periodic/jfrThreadCPULoadEvent.hpp"
 #include "jfr/recorder/jfrRecorder.hpp"
@@ -35,6 +36,7 @@
 #include "memory/allocation.inline.hpp"
 #include "runtime/os.hpp"
 #include "runtime/thread.inline.hpp"
+#include "utilities/sizes.hpp"
 
 /* This data structure is per thread and only accessed by the thread itself, no locking required */
 JfrThreadLocal::JfrThreadLocal() :
@@ -73,53 +75,76 @@ const JfrCheckpointBlobHandle& JfrThreadLocal::thread_checkpoint() const {
   return _thread_cp;
 }
 
-void JfrThreadLocal::set_dead() {
-  assert(!is_dead(), "invariant");
-  _dead = true;
+static void send_java_thread_start_event(JavaThread* jt) {
+  EventThreadStart event;
+  event.set_thread(jt->jfr_thread_local()->thread_id());
+  event.commit();
 }
 
-void JfrThreadLocal::on_exit(JavaThread* thread) {
+void JfrThreadLocal::on_start(Thread* t) {
+  assert(t != NULL, "invariant");
+  assert(Thread::current() == t, "invariant");
   if (JfrRecorder::is_recording()) {
-    JfrCheckpointManager::write_thread_checkpoint(thread);
-    JfrThreadCPULoadEvent::send_event_for_thread(thread);
+    if (t->is_Java_thread()) {
+      send_java_thread_start_event((JavaThread*)t);
+    }
   }
-  thread->jfr_thread_local()->set_dead();
 }
 
-void JfrThreadLocal::on_destruct(Thread* thread) {
-  JfrThreadLocal* const tl = thread->jfr_thread_local();
+static void send_java_thread_end_events(traceid id, JavaThread* jt) {
+  assert(jt != NULL, "invariant");
+  assert(Thread::current() == jt, "invariant");
+  assert(jt->jfr_thread_local()->trace_id() == id, "invariant");
+  EventThreadEnd event;
+  event.set_thread(id);
+  event.commit();
+  JfrThreadCPULoadEvent::send_event_for_thread(jt);
+}
+
+void JfrThreadLocal::release(JfrThreadLocal* tl, Thread* t) {
+  assert(tl != NULL, "invariant");
+  assert(t != NULL, "invariant");
+  assert(Thread::current() == t, "invariant");
+  assert(!tl->is_dead(), "invariant");
+  assert(tl->shelved_buffer() == NULL, "invariant");
   if (tl->has_native_buffer()) {
-    release(tl->native_buffer(), thread);
+    JfrStorage::release_thread_local(tl->native_buffer(), t);
   }
   if (tl->has_java_buffer()) {
-    release(tl->java_buffer(), thread);
+    JfrStorage::release_thread_local(tl->java_buffer(), t);
   }
-  assert(tl->shelved_buffer() == NULL, "invariant");
-  if (thread->jfr_thread_local()->has_java_event_writer()) {
+  if (tl->has_java_event_writer()) {
+    assert(t->is_Java_thread(), "invariant");
     JfrJavaSupport::destroy_global_jni_handle(tl->java_event_writer());
   }
-  destroy_stackframes(thread);
+  if (tl->_stackframes != NULL) {
+    FREE_C_HEAP_ARRAY(JfrStackFrame, tl->_stackframes);
+  }
+  tl->_dead = true;
 }
 
-JfrBuffer* JfrThreadLocal::acquire(Thread* thread, size_t size) {
-  return JfrStorage::acquire_thread_local(thread, size);
-}
-
-void JfrThreadLocal::release(JfrBuffer* buffer, Thread* thread) {
-  assert(buffer != NULL, "invariant");
-  JfrStorage::release_thread_local(buffer, thread);
+void JfrThreadLocal::on_exit(Thread* t) {
+  assert(t != NULL, "invariant");
+  JfrThreadLocal * const tl = t->jfr_thread_local();
+  assert(!tl->is_dead(), "invariant");
+  if (JfrRecorder::is_recording()) {
+    if (t->is_Java_thread()) {
+      send_java_thread_end_events(tl->thread_id(), (JavaThread*)t);
+    }
+  }
+  release(tl, Thread::current()); // because it could be that Thread::current() != t
 }
 
 JfrBuffer* JfrThreadLocal::install_native_buffer() const {
   assert(!has_native_buffer(), "invariant");
-  _native_buffer = acquire(Thread::current());
+  _native_buffer = JfrStorage::acquire_thread_local(Thread::current());
   return _native_buffer;
 }
 
 JfrBuffer* JfrThreadLocal::install_java_buffer() const {
   assert(!has_java_buffer(), "invariant");
   assert(!has_java_event_writer(), "invariant");
-  _java_buffer = acquire(Thread::current());
+  _java_buffer = JfrStorage::acquire_thread_local(Thread::current());
   return _java_buffer;
 }
 
@@ -131,11 +156,10 @@ JfrStackFrame* JfrThreadLocal::install_stackframes() const {
   return _stackframes;
 }
 
-void JfrThreadLocal::destroy_stackframes(Thread* thread) {
-  assert(thread != NULL, "invariant");
-  JfrStackFrame* frames = thread->jfr_thread_local()->stackframes();
-  if (frames != NULL) {
-    FREE_C_HEAP_ARRAY(JfrStackFrame, frames);
-    thread->jfr_thread_local()->set_stackframes(NULL);
-  }
+ByteSize JfrThreadLocal::trace_id_offset() {
+  return in_ByteSize(offset_of(JfrThreadLocal, _trace_id));
+}
+
+ByteSize JfrThreadLocal::java_event_writer_offset() {
+  return in_ByteSize(offset_of(JfrThreadLocal, _java_event_writer));
 }

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

@@ -27,11 +27,11 @@
 
 #include "jfr/recorder/checkpoint/jfrCheckpointBlob.hpp"
 #include "jfr/utilities/jfrTypes.hpp"
-#include "utilities/sizes.hpp"
 
 class JavaThread;
 class JfrBuffer;
 class JfrStackFrame;
+class Thread;
 
 class JfrThreadLocal {
  private:
@@ -56,7 +56,7 @@ class JfrThreadLocal {
   JfrBuffer* install_java_buffer() const;
   JfrStackFrame* install_stackframes() const;
 
-  void set_dead();
+  static void release(JfrThreadLocal* tl, Thread* t);
 
  public:
   JfrThreadLocal();
@@ -213,20 +213,12 @@ class JfrThreadLocal {
   void set_thread_checkpoint(const JfrCheckpointBlobHandle& handle);
   const JfrCheckpointBlobHandle& thread_checkpoint() const;
 
-  static JfrBuffer* acquire(Thread* t, size_t size = 0);
-  static void release(JfrBuffer* buffer, Thread* t);
-  static void destroy_stackframes(Thread* t);
-  static void on_exit(JavaThread* t);
-  static void on_destruct(Thread* t);
+  static void on_start(Thread* t);
+  static void on_exit(Thread* t);
 
   // Code generation
-  static ByteSize trace_id_offset() {
-    return in_ByteSize(offset_of(JfrThreadLocal, _trace_id));
-  }
-
-  static ByteSize java_event_writer_offset() {
-    return in_ByteSize(offset_of(JfrThreadLocal, _java_event_writer));
-  }
+  static ByteSize trace_id_offset();
+  static ByteSize java_event_writer_offset();
 };
 
 #endif // SHARE_VM_JFR_SUPPORT_JFRTHREADLOCAL_HPP

src/hotspot/share/runtime/thread.cpp

@@ -41,7 +41,6 @@
 #include "interpreter/linkResolver.hpp"
 #include "interpreter/oopMapCache.hpp"
 #include "jfr/jfrEvents.hpp"
-#include "jfr/support/jfrThreadId.hpp"
 #include "jvmtifiles/jvmtiEnv.hpp"
 #include "logging/log.hpp"
 #include "logging/logConfiguration.hpp"
@@ -369,6 +368,8 @@ void Thread::call_run() {
 
   register_thread_stack_with_NMT();
 
+  JFR_ONLY(Jfr::on_thread_start(this);)
+
   log_debug(os, thread)("Thread " UINTX_FORMAT " stack dimensions: "
     PTR_FORMAT "-" PTR_FORMAT " (" SIZE_FORMAT "k).",
     os::current_thread_id(), p2i(stack_base() - stack_size()),
@@ -393,8 +394,6 @@ void Thread::call_run() {
 }
 
 Thread::~Thread() {
-  JFR_ONLY(Jfr::on_thread_destruct(this);)
-
   // Notify the barrier set that a thread is being destroyed. Note that a barrier
   // set might not be available if we encountered errors during bootstrapping.
   BarrierSet* const barrier_set = BarrierSet::barrier_set();
@@ -402,7 +401,6 @@ Thread::~Thread() {
     barrier_set->on_thread_destroy(this);
   }
 
-
   // stack_base can be NULL if the thread is never started or exited before
   // record_stack_base_and_size called. Although, we would like to ensure
   // that all started threads do call record_stack_base_and_size(), there is
@@ -1258,6 +1256,7 @@ NonJavaThread::NonJavaThread() : Thread(), _next(NULL) {
 }
 
 NonJavaThread::~NonJavaThread() {
+  JFR_ONLY(Jfr::on_thread_exit(this);)
   // Remove this thread from _the_list.
   MutexLockerEx lock(NonJavaThreadsList_lock, Mutex::_no_safepoint_check_flag);
   NonJavaThread* volatile* p = &_the_list._head;
@@ -1780,12 +1779,7 @@ void JavaThread::run() {
 
   if (JvmtiExport::should_post_thread_life()) {
     JvmtiExport::post_thread_start(this);
-  }
 
-  EventThreadStart event;
-  if (event.should_commit()) {
-    event.set_thread(JFR_THREAD_ID(this));
-    event.commit();
   }
 
   // We call another function to do the rest so we are sure that the stack addresses used
@@ -1889,17 +1883,7 @@ void JavaThread::exit(bool destroy_vm, ExitType exit_type) {
         CLEAR_PENDING_EXCEPTION;
       }
     }
-
-    // Called before the java thread exit since we want to read info
-    // from java_lang_Thread object
-    EventThreadEnd event;
-    if (event.should_commit()) {
-      event.set_thread(JFR_THREAD_ID(this));
-      event.commit();
-    }
-
-    // Call after last event on thread
-    JFR_ONLY(Jfr::on_thread_exit(this);)
+    JFR_ONLY(Jfr::on_java_thread_dismantle(this);)
 
     // Call Thread.exit(). We try 3 times in case we got another Thread.stop during
     // the execution of the method. If that is not enough, then we don't really care. Thread.stop
@@ -1988,6 +1972,7 @@ void JavaThread::exit(bool destroy_vm, ExitType exit_type) {
 
   // These things needs to be done while we are still a Java Thread. Make sure that thread
   // is in a consistent state, in case GC happens
+  JFR_ONLY(Jfr::on_thread_exit(this);)
 
   if (active_handles() != NULL) {
     JNIHandleBlock* block = active_handles();