RFR: 8369238: Allow virtual thread preemption on some common class initialization paths [v3]
David Holmes
dholmes at openjdk.org
Fri Oct 17 06:53:20 UTC 2025
On Thu, 16 Oct 2025 16:13:42 GMT, Patricio Chilano Mateo <pchilanomate at openjdk.org> wrote:
>> If a thread tries to initialize a class that is already being initialized by another thread, it will block until notified. Since at this blocking point there are native frames on the stack, a virtual thread cannot be unmounted and is pinned to its carrier. Besides harming scalability, this can, in some pathological cases, lead to a deadlock, for example, if the thread executing the class initialization method is blocked waiting for some unmounted virtual thread to run, but all carriers are blocked waiting for that class to be initialized.
>>
>> As of JDK-8338383, virtual threads blocked in the VM on `ObjectMonitor` operations can be unmounted. Since synchronization on class initialization is implemented using `ObjectLocker`, we can reuse the same mechanism to unmount virtual threads on these cases too.
>>
>> This patch adds support for unmounting virtual threads on some of the most common class initialization paths, specifically when calling `InterpreterRuntime::_new` (`new` bytecode), and `InterpreterRuntime::resolve_from_cache` for `invokestatic`, `getstatic` or `putstatic` bytecodes. In the future we might consider extending this mechanism to include initialization calls originating from native methods such as `Class.forName0`.
>>
>> ### Summary of implementation
>>
>> The ObjectLocker class was modified to not pin the continuation if we are coming from a preemptable path, which will be the case when calling `InstanceKlass::initialize_impl` from new method `InstanceKlass::initialize_preemptable`. This means that for these cases, a virtual thread can now be unmounted either when contending for the init_lock in the `ObjectLocker` constructor, or in the call to `wait_uninterruptibly`. Also, since the call to initialize a class includes a previous call to `link_class` which also uses `ObjectLocker` to protect concurrent calls from multiple threads, we will allow preemption there too.
>>
>> If preempted, we will throw a pre-allocated exception which will get propagated with the `TRAPS/CHECK` macros all the way back to the VM entry point. The exception will be cleared and on return back to Java the virtual thread will go through the preempt stub and unmount. When running again, at the end of the thaw call we will identify this preemption case and redo the original VM call (either `InterpreterRuntime::_new` or `InterpreterRuntime::resolve_from_cache`).
>>
>> ### Notes
>>
>> `InterpreterRuntime::call_VM_preemptable` used previously only for `InterpreterRuntime::mon...
>
> Patricio Chilano Mateo has updated the pull request incrementally with one additional commit since the last revision:
>
> fix verify_frame_kind
Great piece of work @pchilano ! I've taken an initial pass through the main class/thread/objectMonitor pieces.
src/hotspot/share/interpreter/linkResolver.cpp line 1122:
> 1120: resolved_klass->initialize(CHECK);
> 1121: } else if (static_mode == StaticMode::initialize_klass_preemptable) {
> 1122: resolved_klass->initialize_preemptable(CHECK);
Why is this not CHECK_AND_CLEAR_PREEMPTED?
src/hotspot/share/interpreter/linkResolver.hpp line 192:
> 190: };
> 191:
> 192: enum class StaticMode : uint8_t {
Need to think of a better name for this ... `ClassInitMode`?
src/hotspot/share/interpreter/linkResolver.hpp line 195:
> 193: dont_initialize_klass,
> 194: initialize_klass,
> 195: initialize_klass_preemptable
And maybe more concisely:
Suggestion:
dont_init
init
init_preemptable
?
src/hotspot/share/oops/instanceKlass.cpp line 1284:
> 1282: // Block preemption once we are the initializer thread. Unmounting now
> 1283: // would complicate the reentrant case (identity is platform thread).
> 1284: NoPreemptMark npm(THREAD);
How does this affect unrelated "preemption" that may occur in the Java code called as part of `<clinit>`?
src/hotspot/share/oops/klass.hpp line 583:
> 581: // initializes the klass
> 582: virtual void initialize(TRAPS);
> 583: virtual void initialize_preemptable(TRAPS);
Can we not define these on `instanceKlass` instead of `klass`? Seems really odd to declare virtual methods for which the base class version should never be called (they should be pure virtuals in that case I would think?).
src/hotspot/share/runtime/continuationEntry.cpp line 117:
> 115: assert(thread->has_last_Java_frame(), "Wrong place to use this assertion");
> 116:
> 117: if (preempted) return true;
I don't see the point of adding a new parameter just so the method can check it and return immediately. Shouldn't the caller be checking this?
src/hotspot/share/runtime/continuationFreezeThaw.cpp line 196:
> 194: static void do_deopt_after_thaw(JavaThread* thread);
> 195: static bool do_verify_after_thaw(JavaThread* thread, stackChunkOop chunk, outputStream* st);
> 196: static void log_frames(JavaThread* thread, bool dolog = true);
Suggestion:
static void log_frames(JavaThread* thread, bool do_log = true);
src/hotspot/share/runtime/javaThread.hpp line 492:
> 490: // throw IE at the end of thawing before returning to Java.
> 491: bool _pending_interrupted_exception;
> 492: // We allow preemption on some klass initializion calls.
Suggestion:
// We allow preemption on some klass initialization calls.
src/hotspot/share/runtime/javaThread.hpp line 507:
> 505:
> 506: bool at_preemptable_init() { return _at_preemptable_init; }
> 507: void set_at_preemptable_init(bool b) { _at_preemptable_init = b; }
If you are going align 503 and 504 then you may as well align these two lines as well
src/hotspot/share/runtime/javaThread.hpp line 522:
> 520: JavaThread* _thread;
> 521: public:
> 522: AtRedoVMCall(JavaThread *t) : _thread(t) {
Suggestion:
AtRedoVMCall(JavaThread* t) : _thread(t) {
src/hotspot/share/runtime/javaThread.hpp line 524:
> 522: AtRedoVMCall(JavaThread *t) : _thread(t) {
> 523: _thread->_interp_at_preemptable_vmcall_cnt++;
> 524: assert(_thread->_interp_at_preemptable_vmcall_cnt > 0, "");
Suggestion:
assert(_thread->_interp_at_preemptable_vmcall_cnt > 0, "Unexpected count: %d", _thread->_interp_at_preemptable_vmcall_cnt);
src/hotspot/share/runtime/javaThread.hpp line 528:
> 526: ~AtRedoVMCall() {
> 527: _thread->_interp_at_preemptable_vmcall_cnt--;
> 528: assert(_thread->_interp_at_preemptable_vmcall_cnt >= 0, "");
Suggestion:
assert(_thread->_interp_at_preemptable_vmcall_cnt > 0, "Unexpected count: %d", _thread->_interp_at_preemptable_vmcall_cnt);
src/hotspot/share/runtime/objectMonitor.cpp line 315:
> 313: }
> 314:
> 315: // Keep object protected during ObjectLocker preemption.
I don't understand why this is necessary.
src/hotspot/share/runtime/synchronizer.cpp line 489:
> 487: if (_thread->preempting()) {
> 488: // If preemption was cancelled we acquired the monitor after freezing
> 489: // the frames. Redoing the vm call later in thaw will require us to
I don't quite follow the control flow here - at what point have we frozen any frames? Was that deep inside `enter`?
src/hotspot/share/runtime/synchronizer.hpp line 230:
> 228: bool _skip_exit;
> 229: public:
> 230: ObjectLocker(Handle obj, TRAPS);
I wonder if we should declare `PREEMPTABLE_TRAPS` as an indicator that the only exception expected to come out of a call is the preempted-exception?
src/java.base/share/classes/java/lang/VirtualThread.java line 172:
> 170:
> 171: // true when waiting in Object.wait, false for VM internal uninterruptible Object.wait
> 172: private volatile boolean interruptableWait;
Suggestion:
private volatile boolean interruptibleWait;
src/java.base/share/classes/java/lang/VirtualThread.java line 605:
> 603: if (s == WAITING || s == TIMED_WAITING) {
> 604: int newState;
> 605: boolean interruptable = interruptableWait;
Suggestion:
boolean interruptible = interruptibleWait;
src/java.base/share/classes/jdk/internal/vm/PreemptedException.java line 31:
> 29: * Internal exception used only by the VM.
> 30: */
> 31: public class PreemptedException extends Exception {
Should probably extend `RuntimeException` so that it is not considered a checked-exception
-------------
PR Review: https://git.openjdk.org/jdk/pull/27802#pullrequestreview-3348320781
PR Review Comment: https://git.openjdk.org/jdk/pull/27802#discussion_r2438580798
PR Review Comment: https://git.openjdk.org/jdk/pull/27802#discussion_r2438444689
PR Review Comment: https://git.openjdk.org/jdk/pull/27802#discussion_r2438447733
PR Review Comment: https://git.openjdk.org/jdk/pull/27802#discussion_r2438469910
PR Review Comment: https://git.openjdk.org/jdk/pull/27802#discussion_r2438473899
PR Review Comment: https://git.openjdk.org/jdk/pull/27802#discussion_r2438480176
PR Review Comment: https://git.openjdk.org/jdk/pull/27802#discussion_r2438482435
PR Review Comment: https://git.openjdk.org/jdk/pull/27802#discussion_r2438506706
PR Review Comment: https://git.openjdk.org/jdk/pull/27802#discussion_r2438516134
PR Review Comment: https://git.openjdk.org/jdk/pull/27802#discussion_r2438518828
PR Review Comment: https://git.openjdk.org/jdk/pull/27802#discussion_r2438523339
PR Review Comment: https://git.openjdk.org/jdk/pull/27802#discussion_r2438524180
PR Review Comment: https://git.openjdk.org/jdk/pull/27802#discussion_r2438533049
PR Review Comment: https://git.openjdk.org/jdk/pull/27802#discussion_r2438542039
PR Review Comment: https://git.openjdk.org/jdk/pull/27802#discussion_r2438549455
PR Review Comment: https://git.openjdk.org/jdk/pull/27802#discussion_r2438568359
PR Review Comment: https://git.openjdk.org/jdk/pull/27802#discussion_r2438568876
PR Review Comment: https://git.openjdk.org/jdk/pull/27802#discussion_r2438571309
More information about the core-libs-dev
mailing list