From b6d60280e789436c7f9e3cd1447c8f77b77e77b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20Gr=C3=B6nlund?= Date: Wed, 4 Jun 2025 08:20:48 +0000 Subject: [PATCH] 8358429: JFR: minimize the time the Threads_lock is held for sampling Reviewed-by: egahlin --- .../periodic/sampling/jfrSampleMonitor.hpp | 74 +++++++++++++++ .../periodic/sampling/jfrSampleRequest.hpp | 9 +- .../periodic/sampling/jfrThreadSampler.cpp | 95 +++++++++++-------- .../periodic/sampling/jfrThreadSampling.cpp | 29 ++---- 4 files changed, 145 insertions(+), 62 deletions(-) create mode 100644 src/hotspot/share/jfr/periodic/sampling/jfrSampleMonitor.hpp diff --git a/src/hotspot/share/jfr/periodic/sampling/jfrSampleMonitor.hpp b/src/hotspot/share/jfr/periodic/sampling/jfrSampleMonitor.hpp new file mode 100644 index 00000000000..a9e0b1728a3 --- /dev/null +++ b/src/hotspot/share/jfr/periodic/sampling/jfrSampleMonitor.hpp @@ -0,0 +1,74 @@ +/* + * Copyright (c) 2025, 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_PERIODIC_SAMPLING_JFRSAMPLEMONITOR_HPP +#define SHARE_JFR_PERIODIC_SAMPLING_JFRSAMPLEMONITOR_HPP + +#include "jfr/periodic/sampling/jfrSampleRequest.hpp" +#include "jfr/utilities/jfrTime.hpp" +#include "memory/allocation.hpp" +#include "runtime/javaThread.hpp" +#include "runtime/mutex.hpp" + +class JfrSampleMonitor : public StackObj { + private: + JfrThreadLocal* const _tl; + Monitor* const _sample_monitor; + mutable bool _waiting; + public: + JfrSampleMonitor(JfrThreadLocal* tl) : + _tl(tl), _sample_monitor(tl->sample_monitor()), _waiting(false) { + assert(tl != nullptr, "invariant"); + assert(_sample_monitor != nullptr, "invariant"); + _sample_monitor->lock_without_safepoint_check(); + } + + bool is_waiting() const { + assert_lock_strong(_sample_monitor); + _waiting = _tl->sample_state() == WAITING_FOR_NATIVE_SAMPLE; + return _waiting; + } + + void install_java_sample_request() { + assert_lock_strong(_sample_monitor); + assert(_waiting, "invariant"); + assert(_tl->sample_state() == WAITING_FOR_NATIVE_SAMPLE, "invariant"); + JfrSampleRequest request; + request._sample_ticks = JfrTicks::now(); + _tl->set_sample_request(request); + _tl->set_sample_state(JAVA_SAMPLE); + _sample_monitor->notify_all(); + } + + ~JfrSampleMonitor() { + assert_lock_strong(_sample_monitor); + if (!_waiting) { + _tl->set_sample_state(NO_SAMPLE); + _sample_monitor->notify_all(); + } + _sample_monitor->unlock(); + } +}; + +#endif // SHARE_JFR_PERIODIC_SAMPLING_JFRSAMPLEMONITOR_HPP diff --git a/src/hotspot/share/jfr/periodic/sampling/jfrSampleRequest.hpp b/src/hotspot/share/jfr/periodic/sampling/jfrSampleRequest.hpp index 8cc2b66aa9e..6567e7f8bff 100644 --- a/src/hotspot/share/jfr/periodic/sampling/jfrSampleRequest.hpp +++ b/src/hotspot/share/jfr/periodic/sampling/jfrSampleRequest.hpp @@ -48,10 +48,11 @@ enum JfrSampleResult { }; enum JfrSampleRequestType { - NO_SAMPLE = 0, - NATIVE_SAMPLE = 1, - JAVA_SAMPLE = 2, - NOF_SAMPLE_TYPES + NO_SAMPLE, + JAVA_SAMPLE, + NATIVE_SAMPLE, + WAITING_FOR_NATIVE_SAMPLE, + NOF_SAMPLE_STATES }; struct JfrSampleRequest { diff --git a/src/hotspot/share/jfr/periodic/sampling/jfrThreadSampler.cpp b/src/hotspot/share/jfr/periodic/sampling/jfrThreadSampler.cpp index 3efa0b0d581..4c44c43772d 100644 --- a/src/hotspot/share/jfr/periodic/sampling/jfrThreadSampler.cpp +++ b/src/hotspot/share/jfr/periodic/sampling/jfrThreadSampler.cpp @@ -24,6 +24,7 @@ #include "jfr/metadata/jfrSerializer.hpp" #include "jfr/recorder/service/jfrOptionSet.hpp" +#include "jfr/periodic/sampling/jfrSampleMonitor.hpp" #include "jfr/periodic/sampling/jfrSampleRequest.hpp" #include "jfr/periodic/sampling/jfrThreadSampling.hpp" #include "jfr/periodic/sampling/jfrThreadSampler.hpp" @@ -230,38 +231,41 @@ void JfrSamplerThread::task_stacktrace(JfrSampleRequestType type, JavaThread** l JavaThread* start = nullptr; elapsedTimer sample_time; sample_time.start(); - { - MutexLocker tlock(Threads_lock); - ThreadsListHandle tlh; - // Resolve a sample session relative start position index into the thread list array. - // In cases where the last sampled thread is null or not-null but stale, find_index() returns -1. - _cur_index = tlh.list()->find_index_of_JavaThread(*last_thread); - JavaThread* current = _cur_index != -1 ? *last_thread : nullptr; + ThreadsListHandle tlh; + // Resolve a sample session relative start position index into the thread list array. + // In cases where the last sampled thread is null or not-null but stale, find_index() returns -1. + _cur_index = tlh.list()->find_index_of_JavaThread(*last_thread); + JavaThread* current = _cur_index != -1 ? *last_thread : nullptr; - while (num_samples < sample_limit) { - current = next_thread(tlh.list(), start, current); - if (current == nullptr) { - break; - } - if (is_excluded(current)) { - continue; - } - if (start == nullptr) { - start = current; // remember the thread where we started to attempt sampling - } - bool success; - if (JAVA_SAMPLE == type) { - success = sample_java_thread(current); - } else { - assert(type == NATIVE_SAMPLE, "invariant"); - success = sample_native_thread(current); - } - if (success) { - num_samples++; - } + while (num_samples < sample_limit) { + current = next_thread(tlh.list(), start, current); + if (current == nullptr) { + break; + } + if (is_excluded(current)) { + continue; + } + if (start == nullptr) { + start = current; // remember the thread where we started to attempt sampling + } + bool success; + if (JAVA_SAMPLE == type) { + success = sample_java_thread(current); + } else { + assert(type == NATIVE_SAMPLE, "invariant"); + success = sample_native_thread(current); + } + if (success) { + num_samples++; + } + if (SafepointSynchronize::is_at_safepoint()) { + // For _thread_in_native, we cannot get the Threads_lock. + // For _thread_in_Java, well, there are none. + break; } - *last_thread = current; // remember the thread we last attempted to sample } + + *last_thread = current; // remember the thread we last attempted to sample sample_time.stop(); log_trace(jfr)("JFR thread sampling done in %3.7f secs with %d java %d native samples", sample_time.seconds(), type == JAVA_SAMPLE ? num_samples : 0, type == NATIVE_SAMPLE ? num_samples : 0); @@ -338,17 +342,32 @@ bool JfrSamplerThread::sample_native_thread(JavaThread* jt) { SafepointMechanism::arm_local_poll_release(jt); - // Barriers needed to keep the next read of thread state from floating up. - if (UseSystemMemoryBarrier) { - SystemMemoryBarrier::emit(); - } else { - OrderAccess::storeload(); + // Take the Threads_lock for two purposes: + // 1) Avoid sampling through a safepoint which could result + // in touching oops in case of virtual threads. + // 2) Prevent JFR from issuing an epoch rotation while the sampler thread + // is actively processing a thread in native, as both threads are now + // outside the safepoint protocol. + + // OrderAccess::fence() as part of acquiring the lock prevents loads from floating up. + JfrMutexTryLock threads_lock(Threads_lock); + + if (!threads_lock.acquired() || !jt->has_last_Java_frame()) { + // Remove the native sample request and release the potentially waiting thread. + JfrSampleMonitor jsm(tl); + return false; } - if (jt->thread_state() != _thread_in_native || !jt->has_last_Java_frame()) { - MonitorLocker lock(tl->sample_monitor(), Monitor::_no_safepoint_check_flag); - tl->set_sample_state(NO_SAMPLE); - lock.notify_all(); + if (jt->thread_state() != _thread_in_native) { + assert_lock_strong(Threads_lock); + JfrSampleMonitor jsm(tl); + if (jsm.is_waiting()) { + // The thread has already returned from native, + // now in _thread_in_vm and is waiting to be sampled. + // Convert the native sample request into a java sample request + // and let the thread process the ljf on its own. + jsm.install_java_sample_request(); + } return false; } diff --git a/src/hotspot/share/jfr/periodic/sampling/jfrThreadSampling.cpp b/src/hotspot/share/jfr/periodic/sampling/jfrThreadSampling.cpp index 9bae25bcb3c..aa72c29cf50 100644 --- a/src/hotspot/share/jfr/periodic/sampling/jfrThreadSampling.cpp +++ b/src/hotspot/share/jfr/periodic/sampling/jfrThreadSampling.cpp @@ -28,6 +28,7 @@ #include "code/nmethod.hpp" #include "interpreter/interpreter.hpp" #include "jfr/jfrEvents.hpp" +#include "jfr/periodic/sampling/jfrSampleMonitor.hpp" #include "jfr/periodic/sampling/jfrSampleRequest.hpp" #include "jfr/periodic/sampling/jfrThreadSampling.hpp" #include "jfr/recorder/stacktrace/jfrStackTrace.hpp" @@ -307,24 +308,6 @@ static void drain_enqueued_requests(const JfrTicks& now, JfrThreadLocal* tl, Jav assert(!tl->has_enqueued_requests(), "invariant"); } -class SampleMonitor : public StackObj { - private: - JfrThreadLocal* const _tl; - Monitor* const _sample_monitor; - public: - SampleMonitor(JfrThreadLocal* tl) : _tl(tl), _sample_monitor(tl->sample_monitor()) { - assert(tl != nullptr, "invariant"); - assert(_sample_monitor != nullptr, "invariant"); - _sample_monitor->lock_without_safepoint_check(); - } - ~SampleMonitor() { - assert_lock_strong(_sample_monitor); - _tl->set_sample_state(NO_SAMPLE); - _sample_monitor->notify_all(); - _sample_monitor->unlock(); - } -}; - // Only entered by the JfrSampler thread. bool JfrThreadSampling::process_native_sample_request(JfrThreadLocal* tl, JavaThread* jt, Thread* sampler_thread) { assert(tl != nullptr, "invairant"); @@ -334,7 +317,9 @@ bool JfrThreadSampling::process_native_sample_request(JfrThreadLocal* tl, JavaTh assert(tl == jt->jfr_thread_local(), "invariant"); assert(jt != sampler_thread, "only asynchronous processing of native samples"); assert(jt->has_last_Java_frame(), "invariant"); - assert(tl->sample_state() == NATIVE_SAMPLE, "invariant"); + assert(tl->sample_state() >= NATIVE_SAMPLE, "invariant"); + + assert_lock_strong(Threads_lock); const JfrTicks start_time = JfrTicks::now(); @@ -342,7 +327,7 @@ bool JfrThreadSampling::process_native_sample_request(JfrThreadLocal* tl, JavaTh traceid sid; { - SampleMonitor sm(tl); + JfrSampleMonitor sm(tl); // Because the thread was in native, it is in a walkable state, because // it will hit a safepoint poll on the way back from native. To ensure timely @@ -384,10 +369,14 @@ void JfrThreadSampling::process_sample_request(JavaThread* jt) { for (;;) { const int sample_state = tl->sample_state(); if (sample_state == NATIVE_SAMPLE) { + tl->set_sample_state(WAITING_FOR_NATIVE_SAMPLE); // Wait until stack trace is processed. ml.wait(); } else if (sample_state == JAVA_SAMPLE) { tl->enqueue_request(); + } else if (sample_state == WAITING_FOR_NATIVE_SAMPLE) { + // Handle spurious wakeups. Again wait until stack trace is processed. + ml.wait(); } else { // State has been processed. break;