From 8d36b43925a5ba016827f73893a8b4d44138ad98 Mon Sep 17 00:00:00 2001 From: mgronlun Date: Thu, 15 Nov 2018 11:10:04 +0100 Subject: [PATCH] 8210024: JFR calls virtual is_Java_thread from ~Thread() Reviewed-by: kbarrett, dholmes, dcubed, egahlin --- src/share/vm/jfr/jfr.cpp | 14 ++-- src/share/vm/jfr/jfr.hpp | 5 +- src/share/vm/jfr/support/jfrThreadLocal.cpp | 84 +++++++++++++-------- src/share/vm/jfr/support/jfrThreadLocal.hpp | 20 ++--- src/share/vm/runtime/thread.cpp | 23 +----- 5 files changed, 76 insertions(+), 70 deletions(-) diff --git a/src/share/vm/jfr/jfr.cpp b/src/share/vm/jfr/jfr.cpp index 1d706c267..fd61352ee 100644 --- a/src/share/vm/jfr/jfr.cpp +++ b/src/share/vm/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); } } diff --git a/src/share/vm/jfr/jfr.hpp b/src/share/vm/jfr/jfr.hpp index 13a82baad..4c4b78e60 100644 --- a/src/share/vm/jfr/jfr.hpp +++ b/src/share/vm/jfr/jfr.hpp @@ -46,8 +46,9 @@ 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); diff --git a/src/share/vm/jfr/support/jfrThreadLocal.cpp b/src/share/vm/jfr/support/jfrThreadLocal.cpp index 9ed12625a..c25c0915e 100644 --- a/src/share/vm/jfr/support/jfrThreadLocal.cpp +++ b/src/share/vm/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); -} - -JfrBuffer* JfrThreadLocal::acquire(Thread* thread, size_t size) { - return JfrStorage::acquire_thread_local(thread, size); + if (tl->_stackframes != NULL) { + FREE_C_HEAP_ARRAY(JfrStackFrame, tl->_stackframes, mtTracing); + } + tl->_dead = true; } -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, mtTracing); - 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)); } diff --git a/src/share/vm/jfr/support/jfrThreadLocal.hpp b/src/share/vm/jfr/support/jfrThreadLocal.hpp index 5c0d21a8f..0d981ad33 100644 --- a/src/share/vm/jfr/support/jfrThreadLocal.hpp +++ b/src/share/vm/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 diff --git a/src/share/vm/runtime/thread.cpp b/src/share/vm/runtime/thread.cpp index 42fc46841..e3a0c85ac 100644 --- a/src/share/vm/runtime/thread.cpp +++ b/src/share/vm/runtime/thread.cpp @@ -33,7 +33,6 @@ #include "interpreter/linkResolver.hpp" #include "interpreter/oopMapCache.hpp" #include "jfr/jfrEvents.hpp" -#include "jfr/support/jfrThreadId.hpp" #include "jvmtifiles/jvmtiEnv.hpp" #include "memory/gcLocker.inline.hpp" #include "memory/metaspaceShared.hpp" @@ -345,8 +344,6 @@ Thread::~Thread() { // Reclaim the objectmonitors from the omFreeList of the moribund thread. ObjectSynchronizer::omFlush (this) ; - JFR_ONLY(Jfr::on_thread_destruct(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 @@ -1215,6 +1212,7 @@ NamedThread::NamedThread() : Thread() { } NamedThread::~NamedThread() { + JFR_ONLY(Jfr::on_thread_exit(this);) if (_name != NULL) { FREE_C_HEAP_ARRAY(char, _name, mtThread); _name = NULL; @@ -1672,11 +1670,7 @@ void JavaThread::run() { JvmtiExport::post_thread_start(this); } - EventThreadStart event; - if (event.should_commit()) { - event.set_thread(JFR_THREAD_ID(this)); - event.commit(); - } + JFR_ONLY(Jfr::on_thread_start(this);) // We call another function to do the rest so we are sure that the stack addresses used // from there will be lower than the stack base just computed @@ -1803,17 +1797,7 @@ void JavaThread::exit(bool destroy_vm, ExitType exit_type) { } } } - - // 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 @@ -1890,6 +1874,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 assert(_privileged_stack_top == NULL, "must be NULL when we get here"); + JFR_ONLY(Jfr::on_thread_exit(this);) if (active_handles() != NULL) { JNIHandleBlock* block = active_handles(); -- GitLab