From c2a4fed98c4e17880dd40c19cb73072efea8c583 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20Gr=C3=B6nlund?= Date: Wed, 26 Mar 2025 17:30:45 +0000 Subject: [PATCH] 8348907: Stress times out when is executed with ZGC Reviewed-by: egahlin, aboldtch, eosterlund --- .../build/tools/jfr/GenerateJfrFiles.java | 36 +++++++++++- src/hotspot/share/gc/z/zTracer.cpp | 4 +- .../checkpoint/jfrCheckpointManager.cpp | 4 ++ .../checkpoint/jfrCheckpointManager.hpp | 1 + .../jfr/recorder/checkpoint/types/jfrType.cpp | 17 ++++++ .../jfr/recorder/checkpoint/types/jfrType.hpp | 8 +++ .../checkpoint/types/jfrTypeManager.cpp | 11 ++++ .../checkpoint/types/jfrTypeManager.hpp | 3 +- .../share/jfr/support/jfrThreadLocal.cpp | 58 ++++++++++++++----- .../share/jfr/support/jfrThreadLocal.hpp | 10 +++- 10 files changed, 129 insertions(+), 23 deletions(-) diff --git a/make/src/classes/build/tools/jfr/GenerateJfrFiles.java b/make/src/classes/build/tools/jfr/GenerateJfrFiles.java index 7532a446aa1..09f3a9f5e3f 100644 --- a/make/src/classes/build/tools/jfr/GenerateJfrFiles.java +++ b/make/src/classes/build/tools/jfr/GenerateJfrFiles.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2018, 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2018, 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 @@ -768,9 +768,10 @@ public class GenerateJfrFiles { out.write("#ifndef JFRFILES_JFREVENTCLASSES_HPP"); out.write("#define JFRFILES_JFREVENTCLASSES_HPP"); out.write(""); - out.write("#include \"oops/klass.hpp\""); out.write("#include \"jfrfiles/jfrTypes.hpp\""); out.write("#include \"jfr/utilities/jfrTypes.hpp\""); + out.write("#include \"oops/klass.hpp\""); + out.write("#include \"runtime/thread.hpp\""); out.write("#include \"utilities/macros.hpp\""); out.write("#include \"utilities/ticks.hpp\""); out.write("#if INCLUDE_JFR"); @@ -789,6 +790,7 @@ public class GenerateJfrFiles { out.write(" */"); out.write(""); printTypes(out, metadata, false); + printHelpers(out, false); out.write(""); out.write(""); out.write("#else // !INCLUDE_JFR"); @@ -806,6 +808,7 @@ public class GenerateJfrFiles { out.write("};"); out.write(""); printTypes(out, metadata, true); + printHelpers(out, true); out.write(""); out.write(""); out.write("#endif // INCLUDE_JFR"); @@ -813,6 +816,35 @@ public class GenerateJfrFiles { } } + private static void printHelpers(Printer out, boolean empty) { + out.write("template "); + out.write("class JfrNonReentrant : public EventType {"); + if (!empty) { + out.write(" private:"); + out.write(" Thread* const _thread;"); + out.write(" int32_t _previous_nesting;"); + } + out.write(" public:"); + out.write(" JfrNonReentrant(EventStartTime timing = TIMED)"); + if (empty) { + out.write(" {}"); + } else { + out.write(" : EventType(timing), _thread(Thread::current()), _previous_nesting(JfrThreadLocal::make_non_reentrant(_thread)) {}"); + out.write(""); + out.write(" JfrNonReentrant(Thread* thread, EventStartTime timing = TIMED)"); + out.write(" : EventType(timing), _thread(thread), _previous_nesting(JfrThreadLocal::make_non_reentrant(_thread)) {}"); + } + if (!empty) { + out.write(""); + out.write(" ~JfrNonReentrant() {"); + out.write(" if (_previous_nesting != -1) {"); + out.write(" JfrThreadLocal::make_reentrant(_thread, _previous_nesting);"); + out.write(" }"); + out.write(" }"); + } + out.write("}; "); + } + private static void printTypes(Printer out, Metadata metadata, boolean empty) { for (TypeElement t : metadata.getStructs()) { printType(out, t, empty); diff --git a/src/hotspot/share/gc/z/zTracer.cpp b/src/hotspot/share/gc/z/zTracer.cpp index 6ab16f6c886..81bea81bb60 100644 --- a/src/hotspot/share/gc/z/zTracer.cpp +++ b/src/hotspot/share/gc/z/zTracer.cpp @@ -124,7 +124,7 @@ void ZTracer::initialize() { void ZTracer::send_stat_counter(const ZStatCounter& counter, uint64_t increment, uint64_t value) { NoSafepointVerifier nsv; - EventZStatisticsCounter e; + JfrNonReentrant e; if (e.should_commit()) { e.set_id(counter.id()); e.set_increment(increment); @@ -136,7 +136,7 @@ void ZTracer::send_stat_counter(const ZStatCounter& counter, uint64_t increment, void ZTracer::send_stat_sampler(const ZStatSampler& sampler, uint64_t value) { NoSafepointVerifier nsv; - EventZStatisticsSampler e; + JfrNonReentrant e; if (e.should_commit()) { e.set_id(sampler.id()); e.set_value(value); diff --git a/src/hotspot/share/jfr/recorder/checkpoint/jfrCheckpointManager.cpp b/src/hotspot/share/jfr/recorder/checkpoint/jfrCheckpointManager.cpp index 26b5116a599..8dbc3d4fcea 100644 --- a/src/hotspot/share/jfr/recorder/checkpoint/jfrCheckpointManager.cpp +++ b/src/hotspot/share/jfr/recorder/checkpoint/jfrCheckpointManager.cpp @@ -677,6 +677,10 @@ void JfrCheckpointManager::write_checkpoint(Thread* thread, traceid tid /* 0 */, JfrTypeManager::write_checkpoint(thread, tid, vthread); } +void JfrCheckpointManager::write_simplified_vthread_checkpoint(traceid vtid) { + JfrTypeManager::write_simplified_vthread_checkpoint(vtid); +} + class JfrNotifyClosure : public ThreadClosure { public: void do_thread(Thread* thread) { diff --git a/src/hotspot/share/jfr/recorder/checkpoint/jfrCheckpointManager.hpp b/src/hotspot/share/jfr/recorder/checkpoint/jfrCheckpointManager.hpp index 8a8582eb6f2..53fa064e267 100644 --- a/src/hotspot/share/jfr/recorder/checkpoint/jfrCheckpointManager.hpp +++ b/src/hotspot/share/jfr/recorder/checkpoint/jfrCheckpointManager.hpp @@ -108,6 +108,7 @@ class JfrCheckpointManager : public JfrCHeapObj { public: static JfrBlobHandle create_thread_blob(JavaThread* jt, traceid tid = 0, oop vthread = nullptr); static void write_checkpoint(Thread* t, traceid tid = 0, oop vthread = nullptr); + static void write_simplified_vthread_checkpoint(traceid vtid); size_t flush_type_set(); friend class Jfr; diff --git a/src/hotspot/share/jfr/recorder/checkpoint/types/jfrType.cpp b/src/hotspot/share/jfr/recorder/checkpoint/types/jfrType.cpp index 01cad49e183..9179395a451 100644 --- a/src/hotspot/share/jfr/recorder/checkpoint/types/jfrType.cpp +++ b/src/hotspot/share/jfr/recorder/checkpoint/types/jfrType.cpp @@ -312,6 +312,23 @@ void JfrThreadConstant::serialize(JfrCheckpointWriter& writer) { // VirtualThread threadgroup already serialized invariant. } +// This serializer is used when the vthread name cannot +// be determined because we cannot access any oops. +void JfrSimplifiedVirtualThreadConstant::serialize(JfrCheckpointWriter & writer) { + writer.write_key(_vtid); + // Write the null string categorically as the os name for virtual threads. + writer.write((const char*)nullptr); // os name + writer.write(0); // os id + // vthread name cannot be determined for this simplified version. + // This is because we cannot access any oops. + writer.write_empty_string(); + writer.write(_vtid); // java tid + // java thread group - VirtualThread threadgroup reserved id 1 + writer.write(1); + writer.write(true); // isVirtual + // VirtualThread threadgroup already serialized invariant. +} + void BytecodeConstant::serialize(JfrCheckpointWriter& writer) { static const u4 nof_entries = Bytecodes::number_of_codes; writer.write_count(nof_entries); diff --git a/src/hotspot/share/jfr/recorder/checkpoint/types/jfrType.hpp b/src/hotspot/share/jfr/recorder/checkpoint/types/jfrType.hpp index 680629aaa69..de35ecaf917 100644 --- a/src/hotspot/share/jfr/recorder/checkpoint/types/jfrType.hpp +++ b/src/hotspot/share/jfr/recorder/checkpoint/types/jfrType.hpp @@ -117,6 +117,14 @@ class JfrThreadConstant : public JfrSerializer { void serialize(JfrCheckpointWriter& writer); }; +class JfrSimplifiedVirtualThreadConstant : public JfrSerializer { + private: + traceid _vtid; + public: + JfrSimplifiedVirtualThreadConstant(traceid vtid) : _vtid(vtid) {} + void serialize(JfrCheckpointWriter & writer); +}; + class BytecodeConstant : public JfrSerializer { public: void serialize(JfrCheckpointWriter& writer); diff --git a/src/hotspot/share/jfr/recorder/checkpoint/types/jfrTypeManager.cpp b/src/hotspot/share/jfr/recorder/checkpoint/types/jfrTypeManager.cpp index 334bdc40cb3..493942bade4 100644 --- a/src/hotspot/share/jfr/recorder/checkpoint/types/jfrTypeManager.cpp +++ b/src/hotspot/share/jfr/recorder/checkpoint/types/jfrTypeManager.cpp @@ -132,6 +132,17 @@ void JfrTypeManager::write_checkpoint(Thread* t, traceid tid /* 0 */, oop vthrea type_thread.serialize(writer); } +void JfrTypeManager::write_simplified_vthread_checkpoint(traceid vtid) { + Thread* const current = Thread::current(); + assert(current != nullptr, "invariant"); + ResourceMark rm(current); + JfrCheckpointWriter writer(current, true, THREADS, JFR_VIRTUAL_THREADLOCAL); + // TYPE_THREAD and count is written later as part of vthread bulk serialization. + writer.set_count(1); // Only a logical marker for the checkpoint header. + JfrSimplifiedVirtualThreadConstant type_simple_vthread(vtid); + type_simple_vthread.serialize(writer); +} + class SerializerRegistrationGuard : public StackObj { private: static Semaphore _mutex_semaphore; diff --git a/src/hotspot/share/jfr/recorder/checkpoint/types/jfrTypeManager.hpp b/src/hotspot/share/jfr/recorder/checkpoint/types/jfrTypeManager.hpp index 90c314b0549..9e02aee0b03 100644 --- a/src/hotspot/share/jfr/recorder/checkpoint/types/jfrTypeManager.hpp +++ b/src/hotspot/share/jfr/recorder/checkpoint/types/jfrTypeManager.hpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2016, 2022, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2016, 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 @@ -41,6 +41,7 @@ class JfrTypeManager : public AllStatic { static void write_threads(JfrCheckpointWriter& writer); static JfrBlobHandle create_thread_blob(JavaThread* jt, traceid tid = 0, oop vthread = nullptr); static void write_checkpoint(Thread* t, traceid tid = 0, oop vthread = nullptr); + static void write_simplified_vthread_checkpoint(traceid vtid); static void write_static_types(JfrCheckpointWriter& writer); }; diff --git a/src/hotspot/share/jfr/support/jfrThreadLocal.cpp b/src/hotspot/share/jfr/support/jfrThreadLocal.cpp index 0a14e23b87c..31fd9892a31 100644 --- a/src/hotspot/share/jfr/support/jfrThreadLocal.cpp +++ b/src/hotspot/share/jfr/support/jfrThreadLocal.cpp @@ -70,6 +70,7 @@ JfrThreadLocal::JfrThreadLocal() : _wallclock_time(os::javaTimeNanos()), _stackdepth(0), _entering_suspend_flag(0), + _non_reentrant_nesting(0), _vthread_epoch(0), _vthread_excluded(false), _jvm_thread_excluded(false), @@ -365,28 +366,32 @@ typedef JfrOopTraceId AccessThreadTraceId; void JfrThreadLocal::set_vthread_epoch(const JavaThread* jt, traceid tid, u2 epoch) { assert(jt != nullptr, "invariant"); assert(is_vthread(jt), "invariant"); - // To support event recursion, we update the native side first, - // this provides the terminating case. + assert(!is_non_reentrant(), "invariant"); + Atomic::store(&jt->jfr_thread_local()->_vthread_epoch, epoch); - /* - * The java side, i.e. the vthread object, can now be updated. - * Accessing the vthread object itself is a recursive case, - * because it can trigger additional events, e.g. - * loading the oop through load barriers. - * Note there is a potential problem with this solution: - * The recursive write hitting the terminating case will - * use the thread id _before_ the checkpoint is committed. - * Hence, the periodic thread can possibly flush that event - * to a segment that does not include an associated checkpoint. - * Considered rare and quite benign for now. The worst case is - * that thread information for that event is not resolvable, i.e. null. - */ + oop vthread = jt->vthread(); assert(vthread != nullptr, "invariant"); + AccessThreadTraceId::set_epoch(vthread, epoch); JfrCheckpointManager::write_checkpoint(const_cast(jt), tid, vthread); } +void JfrThreadLocal::set_vthread_epoch_checked(const JavaThread* jt, traceid tid, u2 epoch) { + assert(jt != nullptr, "invariant"); + assert(is_vthread(jt), "invariant"); + + // If the event is marked as non reentrant, write only a simplified version of the vthread info. + // Essentially all the same info except the vthread name, because we cannot touch the oop. + // Since we cannot touch the oop, we also cannot update its vthread epoch. + if (is_non_reentrant()) { + JfrCheckpointManager::write_simplified_vthread_checkpoint(tid); + return; + } + + set_vthread_epoch(jt, tid, epoch); +} + traceid JfrThreadLocal::vthread_id(const Thread* t) { assert(t != nullptr, "invariant"); return Atomic::load(&t->jfr_thread_local()->_vthread_id); @@ -416,7 +421,7 @@ traceid JfrThreadLocal::thread_id(const Thread* t) { if (!tl->is_vthread_excluded()) { const u2 current_epoch = AccessThreadTraceId::current_epoch(); if (vthread_epoch(jt) != current_epoch) { - set_vthread_epoch(jt, tid, current_epoch); + set_vthread_epoch_checked(jt, tid, current_epoch); } } return tid; @@ -480,6 +485,26 @@ bool JfrThreadLocal::is_vthread(const JavaThread* jt) { return Atomic::load_acquire(&jt->jfr_thread_local()->_vthread) && jt->last_continuation() != nullptr; } +int32_t JfrThreadLocal::make_non_reentrant(Thread* t) { + assert(t != nullptr, "invariant"); + if (!t->is_Java_thread() || !is_vthread(JavaThread::cast(t))) { + return -1; + } + return t->jfr_thread_local()->_non_reentrant_nesting++; +} + +void JfrThreadLocal::make_reentrant(Thread* t, int32_t previous_nesting) { + assert(t->is_Java_thread() && is_vthread(JavaThread::cast(t)), "invariant"); + assert(previous_nesting >= 0, "invariant"); + t->jfr_thread_local()->_non_reentrant_nesting = previous_nesting; +} + +bool JfrThreadLocal::is_non_reentrant() { + Thread* const current_thread = Thread::current(); + assert(current_thread != nullptr, "invariant"); + return current_thread->jfr_thread_local()->_non_reentrant_nesting > 0; +} + inline bool is_virtual(const JavaThread* jt, oop thread) { assert(jt != nullptr, "invariant"); return thread != jt->threadObj(); @@ -493,6 +518,7 @@ void JfrThreadLocal::on_set_current_thread(JavaThread* jt, oop thread) { Atomic::release_store(&tl->_vthread, false); return; } + assert(tl->_non_reentrant_nesting == 0, "invariant"); Atomic::store(&tl->_vthread_id, AccessThreadTraceId::id(thread)); const u2 epoch_raw = AccessThreadTraceId::epoch(thread); const bool excluded = epoch_raw & excluded_bit; diff --git a/src/hotspot/share/jfr/support/jfrThreadLocal.hpp b/src/hotspot/share/jfr/support/jfrThreadLocal.hpp index d2f6ef8d5d5..5e25de56ec0 100644 --- a/src/hotspot/share/jfr/support/jfrThreadLocal.hpp +++ b/src/hotspot/share/jfr/support/jfrThreadLocal.hpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2012, 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2012, 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 @@ -64,6 +64,7 @@ class JfrThreadLocal { jlong _wallclock_time; mutable u4 _stackdepth; volatile jint _entering_suspend_flag; + int32_t _non_reentrant_nesting; u2 _vthread_epoch; bool _vthread_excluded; bool _jvm_thread_excluded; @@ -81,7 +82,8 @@ class JfrThreadLocal { static void set(bool* excluded_field, bool state); static traceid assign_thread_id(const Thread* t, JfrThreadLocal* tl); static traceid vthread_id(const Thread* t); - static void set_vthread_epoch(const JavaThread* jt, traceid id, u2 epoch); + static void set_vthread_epoch(const JavaThread* jt, traceid tid, u2 epoch); + static void set_vthread_epoch_checked(const JavaThread* jt, traceid tid, u2 epoch); static traceid jvm_thread_id(const JfrThreadLocal* tl); bool is_vthread_excluded() const; static void exclude_vthread(const JavaThread* jt); @@ -89,6 +91,7 @@ class JfrThreadLocal { static bool is_jvm_thread_excluded(const Thread* t); static void exclude_jvm_thread(const Thread* t); static void include_jvm_thread(const Thread* t); + static bool is_non_reentrant(); public: JfrThreadLocal(); @@ -266,6 +269,9 @@ class JfrThreadLocal { return _dead; } + static int32_t make_non_reentrant(Thread* thread); + static void make_reentrant(Thread* thread, int32_t previous_nesting); + bool is_excluded() const; bool is_included() const; static bool is_excluded(const Thread* thread);