On Tue, 14 Oct 2025 20:23:33 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 with a new target base 
> due to a merge or a rebase. The pull request now contains four commits:
> 
>  - Merge branch 'master' into JDK-8369238
>  - RISC-V support
>  - Fix whitespaces
>  - v1

Great enhancement indeed @pchilano! The ppc part of it is almost finished. 
Unfortunately I'm stuck with a problem in verification code already in initial 
testing. Please see my comment on `verify_frame_kind`.

src/hotspot/share/runtime/continuationFreezeThaw.cpp line 1751:

> 1749:                   RegisterMap::ProcessFrames::skip,
> 1750:                   RegisterMap::WalkContinuation::skip);
> 1751:       frame fr = top.sender(&reg_map);

I think there's a problem here. I get an assertion on ppc if `top` is a heap 
frame (calling from `log_preempt_after_freeze`) because in 
`frame::sender_raw()` we don't take the path we normally would for a frame on 
heap. Instead `sender_for_compiled_frame()` is called which uses a constructor 
that asserts alignment of `sp` (see 
[here](https://github.com/openjdk/jdk/blob/1bd814c3b24eb7ef5633ee34bb418e0981ca1708/src/hotspot/cpu/ppc/frame_ppc.inline.hpp#L81-L86)).
 The assertion fails because `_on_heap` is false but should be `true`.

I think in `sender_raw` `map->in_cont()` should return true if this frame is on 
heap.

It's not quite easy to fix though since `top` can also be on stack.

-------------

PR Review: https://git.openjdk.org/jdk/pull/27802#pullrequestreview-3341440114
PR Review Comment: https://git.openjdk.org/jdk/pull/27802#discussion_r2433336330

Reply via email to