On Tue, 28 Oct 2025 19:37:34 GMT, Patricio Chilano Mateo
<[email protected]> 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:
>
> add const to references
A few additional comments/suggestions, but overall this looks good (to the
extent I understand the details). Thanks.
src/hotspot/share/runtime/continuationFreezeThaw.cpp line 2735:
> 2733: HandleMarkCleaner hm(current); // Cleanup all handles (including
> so._conth) before returning to Java.
> 2734: ContinuationWrapper::SafepointOp so(current, _cont);
> 2735: AnchorMark am(current, top); // Set the anchor so that the stack is
> walkable.
Shouldn't you delete the `clear_anchor` at line 2739 below?
src/hotspot/share/runtime/javaCalls.cpp line 61:
> 59: assert(!thread->owns_locks(), "must release all locks when leaving VM");
> 60: guarantee(thread->can_call_java(), "cannot make java calls from the
> native compiler");
> 61: assert(!thread->preempting(), "");
I'm not sure why this is checked here, and there is no error message to tell
me. If we did get here with `preempting` set what would that mean?
src/hotspot/share/runtime/thread.cpp line 578:
> 576:
> 577: bool Thread::TrySpinAcquire(volatile int * adr) {
> 578: return AtomicAccess::cmpxchg(adr, 0, 1) == 0;
How is this a try-spin-acquire operation ??? I don't think we need this, we can
just inline the `cmpxchg` where needed.
src/hotspot/share/utilities/exceptions.cpp line 350:
> 348: // the exception is propagated we might make an upcall to
> 349: // Java to initialize the object with the cause of exception.
> 350: NoPreemptMark npm(thread);
Could you explain the control flow in more detail here please. I'm unclear both
how we get here and exactly what the affect of the NoPreemptMark is.
-------------
PR Review: https://git.openjdk.org/jdk/pull/27802#pullrequestreview-3391793337
PR Review Comment: https://git.openjdk.org/jdk/pull/27802#discussion_r2471824170
PR Review Comment: https://git.openjdk.org/jdk/pull/27802#discussion_r2471827558
PR Review Comment: https://git.openjdk.org/jdk/pull/27802#discussion_r2471843917
PR Review Comment: https://git.openjdk.org/jdk/pull/27802#discussion_r2471852020
