diff --git a/src/hotspot/share/jfr/writers/jfrJavaEventWriter.cpp b/src/hotspot/share/jfr/writers/jfrJavaEventWriter.cpp index 2bd66ce7c1c..d9610bcc970 100644 --- a/src/hotspot/share/jfr/writers/jfrJavaEventWriter.cpp +++ b/src/hotspot/share/jfr/writers/jfrJavaEventWriter.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2016, 2023, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2016, 2024, 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 @@ -36,6 +36,7 @@ #include "oops/instanceKlass.hpp" #include "oops/oop.inline.hpp" #include "runtime/fieldDescriptor.inline.hpp" +#include "runtime/globals.hpp" #include "runtime/handles.inline.hpp" #include "runtime/interfaceSupport.inline.hpp" #include "runtime/jniHandles.inline.hpp" @@ -49,6 +50,7 @@ static int max_pos_offset = invalid_offset; static int excluded_offset = invalid_offset; static int thread_id_offset = invalid_offset; static int valid_offset = invalid_offset; +static int pin_offset = invalid_offset; static bool setup_event_writer_offsets(TRAPS) { const char class_name[] = "jdk/jfr/internal/event/EventWriter"; @@ -98,6 +100,13 @@ static bool setup_event_writer_offsets(TRAPS) { assert(invalid_offset == valid_offset, "invariant"); JfrJavaSupport::compute_field_offset(valid_offset, klass, valid_sym, vmSymbols::bool_signature()); assert(valid_offset != invalid_offset, "invariant"); + + const char pin_name[] = "pinVirtualThread"; + Symbol* const pin_sym = SymbolTable::new_symbol(valid_name); + assert(pin_sym != nullptr, "invariant"); + assert(invalid_offset == pin_offset, "invariant"); + JfrJavaSupport::compute_field_offset(pin_offset, klass, pin_sym, vmSymbols::bool_signature()); + assert(pin_offset != invalid_offset, "invariant"); return true; } @@ -219,13 +228,18 @@ void JfrJavaEventWriter::notify(JavaThread* jt) { } } +static inline bool pin_virtual(const JavaThread* jt) { + assert(jt != nullptr, "invariant"); + return JfrThreadLocal::is_vthread(jt) && VMContinuations; +} + static jobject create_new_event_writer(JfrBuffer* buffer, JfrThreadLocal* tl, TRAPS) { assert(buffer != nullptr, "invariant"); DEBUG_ONLY(JfrJavaSupport::check_java_thread_in_vm(THREAD)); HandleMark hm(THREAD); static const char klass[] = "jdk/jfr/internal/event/EventWriter"; static const char method[] = ""; - static const char signature[] = "(JJJZZ)V"; + static const char signature[] = "(JJJZZZ)V"; JavaValue result(T_OBJECT); JfrJavaArguments args(&result, klass, method, signature, CHECK_NULL); @@ -234,6 +248,7 @@ static jobject create_new_event_writer(JfrBuffer* buffer, JfrThreadLocal* tl, TR args.push_long((jlong)buffer->end()); args.push_long((jlong)JfrThreadLocal::thread_id(THREAD)); args.push_int((jint)JNI_TRUE); // valid + args.push_int(pin_virtual(THREAD) ? (jint)JNI_TRUE : (jint)JNI_FALSE); args.push_int(tl->is_excluded() ? (jint)JNI_TRUE : (jint)JNI_FALSE); // excluded JfrJavaSupport::new_object_global_ref(&args, CHECK_NULL); return result.get_jobject(); @@ -249,9 +264,12 @@ jobject JfrJavaEventWriter::event_writer(JavaThread* jt) { const jlong event_writer_tid = writer->long_field(thread_id_offset); const jlong current_tid = static_cast(JfrThreadLocal::thread_id(jt)); if (event_writer_tid != current_tid) { + writer->long_field_put(thread_id_offset, current_tid); const bool excluded = tl->is_excluded(); writer->bool_field_put(excluded_offset, excluded); - writer->long_field_put(thread_id_offset, current_tid); + if (!excluded) { + writer->bool_field_put(pin_offset, pin_virtual(jt)); + } } } return h_writer; diff --git a/src/hotspot/share/opto/library_call.cpp b/src/hotspot/share/opto/library_call.cpp index 12cd005f93e..24901855b91 100644 --- a/src/hotspot/share/opto/library_call.cpp +++ b/src/hotspot/share/opto/library_call.cpp @@ -3233,10 +3233,12 @@ bool LibraryCallKit::inline_native_jvm_commit() { * oop threadObj = Thread::threadObj(); * oop vthread = java_lang_Thread::vthread(threadObj); * traceid tid; + * bool pinVirtualThread; * bool excluded; * if (vthread != threadObj) { // i.e. current thread is virtual * tid = java_lang_Thread::tid(vthread); * u2 vthread_epoch_raw = java_lang_Thread::jfr_epoch(vthread); + * pinVirtualThread = VMContinuations; * excluded = vthread_epoch_raw & excluded_mask; * if (!excluded) { * traceid current_epoch = JfrTraceIdEpoch::current_generation(); @@ -3248,13 +3250,15 @@ bool LibraryCallKit::inline_native_jvm_commit() { * } else { * tid = java_lang_Thread::tid(threadObj); * u2 thread_epoch_raw = java_lang_Thread::jfr_epoch(threadObj); + * pinVirtualThread = false; * excluded = thread_epoch_raw & excluded_mask; * } * oop event_writer = JNIHandles::resolve_non_null(h_event_writer); * traceid tid_in_event_writer = getField(event_writer, "threadID"); * if (tid_in_event_writer != tid) { - * setField(event_writer, "threadID", tid); + * setField(event_writer, "pinVirtualThread", pinVirtualThread); * setField(event_writer, "excluded", excluded); + * setField(event_writer, "threadID", tid); * } * return event_writer */ @@ -3325,6 +3329,10 @@ bool LibraryCallKit::inline_native_getEventWriter() { // Load the tid field from the vthread object. Node* vthread_tid = load_field_from_object(vthread, "tid", "J"); + // Continuation support determines if a virtual thread should be pinned. + Node* global_addr = makecon(TypeRawPtr::make((address)&VMContinuations)); + Node* continuation_support = make_load(control(), global_addr, TypeInt::BOOL, T_BOOLEAN, MemNode::unordered); + // Load the raw epoch value from the vthread. Node* vthread_epoch_offset = basic_plus_adr(vthread, java_lang_Thread::jfr_epoch_offset()); Node* vthread_epoch_raw = access_load_at(vthread, vthread_epoch_offset, TypeRawPtr::BOTTOM, TypeInt::CHAR, T_CHAR, @@ -3415,6 +3423,8 @@ bool LibraryCallKit::inline_native_getEventWriter() { record_for_igvn(tid); PhiNode* exclusion = new PhiNode(vthread_compare_rgn, TypeInt::BOOL); record_for_igvn(exclusion); + PhiNode* pinVirtualThread = new PhiNode(vthread_compare_rgn, TypeInt::BOOL); + record_for_igvn(pinVirtualThread); // Update control and phi nodes. vthread_compare_rgn->init_req(_true_path, _gvn.transform(exclude_compare_rgn)); @@ -3427,6 +3437,8 @@ bool LibraryCallKit::inline_native_getEventWriter() { tid->init_req(_false_path, _gvn.transform(thread_obj_tid)); exclusion->init_req(_true_path, _gvn.transform(vthread_is_excluded)); exclusion->init_req(_false_path, _gvn.transform(threadObj_is_excluded)); + pinVirtualThread->init_req(_true_path, _gvn.transform(continuation_support)); + pinVirtualThread->init_req(_false_path, _gvn.intcon(0)); // Update branch state. set_control(_gvn.transform(vthread_compare_rgn)); @@ -3450,6 +3462,9 @@ bool LibraryCallKit::inline_native_getEventWriter() { // Get the field offset to, conditionally, store an updated exclusion value later. Node* const event_writer_excluded_field = field_address_from_object(event_writer, "excluded", "Z", false); const TypePtr* event_writer_excluded_field_type = _gvn.type(event_writer_excluded_field)->isa_ptr(); + // Get the field offset to, conditionally, store an updated pinVirtualThread value later. + Node* const event_writer_pin_field = field_address_from_object(event_writer, "pinVirtualThread", "Z", false); + const TypePtr* event_writer_pin_field_type = _gvn.type(event_writer_pin_field)->isa_ptr(); RegionNode* event_writer_tid_compare_rgn = new RegionNode(PATH_LIMIT); record_for_igvn(event_writer_tid_compare_rgn); @@ -3470,6 +3485,9 @@ bool LibraryCallKit::inline_native_getEventWriter() { Node* tid_is_not_equal = _gvn.transform(new IfTrueNode(iff_tid_not_equal)); record_for_igvn(tid_is_not_equal); + // Store the pin state to the event writer. + store_to_memory(tid_is_not_equal, event_writer_pin_field, _gvn.transform(pinVirtualThread), T_BOOLEAN, event_writer_pin_field_type, MemNode::unordered); + // Store the exclusion state to the event writer. store_to_memory(tid_is_not_equal, event_writer_excluded_field, _gvn.transform(exclusion), T_BOOLEAN, event_writer_excluded_field_type, MemNode::unordered); diff --git a/src/java.base/share/classes/module-info.java b/src/java.base/share/classes/module-info.java index 52c1029dd3d..74a7451582c 100644 --- a/src/java.base/share/classes/module-info.java +++ b/src/java.base/share/classes/module-info.java @@ -256,7 +256,8 @@ module java.base { jdk.internal.jvmstat, jdk.management, jdk.management.agent, - jdk.internal.vm.ci; + jdk.internal.vm.ci, + jdk.jfr; exports jdk.internal.vm.annotation to java.instrument, jdk.internal.vm.ci, diff --git a/src/jdk.jfr/share/classes/jdk/jfr/internal/StringPool.java b/src/jdk.jfr/share/classes/jdk/jfr/internal/StringPool.java index 6513895069e..d16bd9ab2a9 100644 --- a/src/jdk.jfr/share/classes/jdk/jfr/internal/StringPool.java +++ b/src/jdk.jfr/share/classes/jdk/jfr/internal/StringPool.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2016, 2023, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2016, 2024, 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 @@ -26,6 +26,7 @@ package jdk.jfr.internal; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.AtomicLong; +import jdk.internal.vm.Continuation; public final class StringPool { public static final int MIN_LIMIT = 16; @@ -71,33 +72,49 @@ public final class StringPool { return internalSid >> 16; } - /* synchronized because of writing the string to the JVM. */ - private static synchronized long storeString(String s) { - Long lsid = cache.get(s); - long internalSid; - if (lsid != null) { - internalSid = lsid.longValue(); - if (isCurrentGeneration(internalSid)) { - // Someone already updated the cache. - return externalSid(internalSid); - } - internalSid = updateInternalSid(internalSid); - } else { - // Not yet added or the cache was cleared. - internalSid = nextInternalSid(); - currentSizeUTF16 += s.length(); + /* Explicitly pin a virtual thread before acquiring the string pool monitor + * because migrating the EventWriter onto another carrier thread is impossible. + */ + private static long storeString(String s, boolean pinVirtualThread) { + if (pinVirtualThread) { + assert(Thread.currentThread().isVirtual()); + Continuation.pin(); + } + try { + /* synchronized because of writing the string to the JVM. */ + synchronized (StringPool.class) { + Long lsid = cache.get(s); + long internalSid; + if (lsid != null) { + internalSid = lsid.longValue(); + if (isCurrentGeneration(internalSid)) { + // Someone already updated the cache. + return externalSid(internalSid); + } + internalSid = updateInternalSid(internalSid); + } else { + // Not yet added or the cache was cleared. + internalSid = nextInternalSid(); + currentSizeUTF16 += s.length(); + } + long extSid = externalSid(internalSid); + // Write the string to the JVM before publishing to the cache. + JVM.addStringConstant(extSid, s); + cache.put(s, internalSid); + return extSid; + } + } finally { + if (pinVirtualThread) { + assert(Thread.currentThread().isVirtual()); + Continuation.unpin(); + } } - long extSid = externalSid(internalSid); - // Write the string to the JVM before publishing to the cache. - JVM.addStringConstant(extSid, s); - cache.put(s, internalSid); - return extSid; } /* a string fetched from the string pool must be of the current generation */ - private static long ensureCurrentGeneration(String s, Long lsid) { + private static long ensureCurrentGeneration(String s, Long lsid, boolean pinVirtualThread) { long internalSid = lsid.longValue(); - return isCurrentGeneration(internalSid) ? externalSid(internalSid) : storeString(s); + return isCurrentGeneration(internalSid) ? externalSid(internalSid) : storeString(s, pinVirtualThread); } /* @@ -109,10 +126,10 @@ public final class StringPool { * effectively invalidating the fetched string id. The event restart mechanism * of the EventWriter ensures that committed strings are in the correct generation. */ - public static long addString(String s) { + public static long addString(String s, boolean pinVirtualThread) { Long lsid = cache.get(s); if (lsid != null) { - return ensureCurrentGeneration(s, lsid); + return ensureCurrentGeneration(s, lsid, pinVirtualThread); } if (!preCache(s)) { /* we should not pool this string */ @@ -120,9 +137,9 @@ public final class StringPool { } if (cache.size() > MAX_SIZE || currentSizeUTF16 > MAX_SIZE_UTF16) { /* pool was full */ - reset(); + reset(pinVirtualThread); } - return storeString(s); + return storeString(s, pinVirtualThread); } private static boolean preCache(String s) { @@ -143,8 +160,21 @@ public final class StringPool { return false; } - private static synchronized void reset() { - cache.clear(); - currentSizeUTF16 = 0; + private static void reset(boolean pinVirtualThread) { + if (pinVirtualThread) { + assert(Thread.currentThread().isVirtual()); + Continuation.pin(); + } + try { + synchronized (StringPool.class) { + cache.clear(); + currentSizeUTF16 = 0; + } + } finally { + if (pinVirtualThread) { + assert(Thread.currentThread().isVirtual()); + Continuation.unpin(); + } + } } } diff --git a/src/jdk.jfr/share/classes/jdk/jfr/internal/event/EventWriter.java b/src/jdk.jfr/share/classes/jdk/jfr/internal/event/EventWriter.java index 833fb087e24..970365ef73d 100644 --- a/src/jdk.jfr/share/classes/jdk/jfr/internal/event/EventWriter.java +++ b/src/jdk.jfr/share/classes/jdk/jfr/internal/event/EventWriter.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2016, 2023, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2016, 2024, 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 @@ -65,6 +65,7 @@ public final class EventWriter { private long currentPosition; private long maxPosition; private boolean valid; + private boolean pinVirtualThread; boolean excluded; private PlatformEventType eventType; @@ -144,7 +145,7 @@ public final class EventWriter { return; } if (length > StringPool.MIN_LIMIT && length < StringPool.MAX_LIMIT) { - long l = StringPool.addString(s); + long l = StringPool.addString(s, pinVirtualThread); if (l > 0) { putByte(StringParser.Encoding.CONSTANT_POOL.byteValue()); putLong(l); @@ -296,11 +297,12 @@ public final class EventWriter { return false; } - private EventWriter(long startPos, long maxPos, long threadID, boolean valid, boolean excluded) { + private EventWriter(long startPos, long maxPos, long threadID, boolean valid, boolean pinVirtualThread, boolean excluded) { startPosition = currentPosition = startPos; maxPosition = maxPos; this.threadID = threadID; this.valid = valid; + this.pinVirtualThread = pinVirtualThread; this.excluded = excluded; } diff --git a/test/jdk/jdk/jfr/threading/TestStringPoolVirtualThreadPinning.java b/test/jdk/jdk/jfr/threading/TestStringPoolVirtualThreadPinning.java new file mode 100644 index 00000000000..0dc1ef85661 --- /dev/null +++ b/test/jdk/jdk/jfr/threading/TestStringPoolVirtualThreadPinning.java @@ -0,0 +1,110 @@ +/* + * Copyright (c) 2024, 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. + */ +package jdk.jfr.threading; + +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.List; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ThreadFactory; + +import jdk.jfr.Event; +import jdk.jfr.Name; +import jdk.jfr.Recording; +import jdk.jfr.consumer.RecordedEvent; +import jdk.jfr.consumer.RecordedThread; +import jdk.jfr.consumer.RecordingFile; +import jdk.test.lib.Asserts; +import jdk.test.lib.Utils; + +/** + * @test + * @bug 8338417 + * @summary Tests pinning of virtual threads when the JFR string pool monitor is contended. + * @key jfr + * @requires vm.hasJFR & vm.continuations + * @library /test/lib /test/jdk + * @run main/othervm jdk.jfr.threading.TestStringPoolVirtualThreadPinning + */ +public class TestStringPoolVirtualThreadPinning { + + private static final int VIRTUAL_THREAD_COUNT = 100_000; + private static final int STARTER_THREADS = 10; + + @Name("test.Tester") + private static class TestEvent extends Event { + private String eventString = Thread.currentThread().getName(); + } + + /* + * During event commit, the thread is in a critical section because it has loaded a carrier thread local event writer object. + * For virtual threads, a contended monitor, such as a synchronized block, is a point where a thread could become unmounted. + * A monitor guards the JFR string pool, but because of the event writer, remounting a virtual thread onto another carrier is impossible. + * + * The test provokes JFR string pool monitor contention to exercise explicit pin constructs to ensure the pinning of virtual threads. + */ + public static void main(String... args) throws Exception { + try (Recording r = new Recording()) { + r.start(); + + ThreadFactory factory = Thread.ofVirtual().factory(); + CompletableFuture[] c = new CompletableFuture[STARTER_THREADS]; + for (int j = 0; j < STARTER_THREADS; j++) { + c[j] = CompletableFuture.runAsync(() -> { + for (int i = 0; i < VIRTUAL_THREAD_COUNT / STARTER_THREADS; i++) { + try { + Thread vt = factory.newThread(TestStringPoolVirtualThreadPinning::emitEvent); + // For an event field string to be placed in the JFR string pool, it must exceed 16 characters. + // We use the virtual thread name as the event field string so we can verify the result as a 1-1 mapping. + vt.setName("VirtualTestThread-" + i); + vt.start(); + vt.join(); + } catch (InterruptedException ie) { + ie.printStackTrace(); + } + } + }); + } + for (int j = 0; j < STARTER_THREADS; j++) { + c[j].get(); + } + + r.stop(); + Path p = Utils.createTempFile("test", ".jfr"); + r.dump(p); + List events = RecordingFile.readAllEvents(p); + Asserts.assertEquals(events.size(), VIRTUAL_THREAD_COUNT, "Expected " + VIRTUAL_THREAD_COUNT + " events"); + for (RecordedEvent e : events) { + RecordedThread t = e.getThread(); + Asserts.assertNotNull(t); + Asserts.assertTrue(t.isVirtual()); + Asserts.assertEquals(e.getString("eventString"), t.getJavaName()); + } + } + } + + private static void emitEvent() { + TestEvent t = new TestEvent(); + t.commit(); + } +}