RFR: 8369238: Allow virtual thread preemption on some common class initialization paths [v7]

[Coleen Phillimore]

On Thu, 23 Oct 2025 22:25:53 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 three additional commits since the last revision:
> 
>  - rename _monitor to _init_lock
>  - Extra comments from Coleen
>  - define methods in resolve_from_cache with TRAPS and use CHECK_AND_CLEAR_PREEMPTED

Almost done. Thank you for making suggested changes.

src/hotspot/cpu/aarch64/continuationFreezeThaw_aarch64.inline.hpp line 347:

> 345:   intptr_t* sp = enterSpecial.sp();
> 346: 
> 347:   sp[-1] = (intptr_t)StubRoutines::cont_preempt_stub();

push_cleanup_continuation sets sp[-2].  This doesn't have to set that?

src/hotspot/cpu/aarch64/stackChunkFrameStream_aarch64.inline.hpp line 116:

> 114:   InterpreterOopMap mask;
> 115:   frame f = to_frame();
> 116:   f.interpreted_frame_oop_map(&mask);

There are two uses of this function left in continuationHelper.inline.hpp and continuationFreezeThaw.cpp under verification code.  Maybe they can be removed?  Do the places that call this in verification code still valid for preempted class initialization?  Do they need to count arguments now?

src/hotspot/cpu/x86/continuationFreezeThaw_x86.inline.hpp line 220:

> 218:     intptr_t* sp = _top_frame.sp();
> 219:     if (sp != _last_sp_from_frame) {
> 220:       sp[-1] = (intptr_t)_top_frame.pc();

Same coment as aarch64, does this need to set sp[-2] to fp like above?  Or should it preserve the value?  Can you add a comment for each telling why?

src/hotspot/cpu/x86/continuationFreezeThaw_x86.inline.hpp line 334:

> 332:   intptr_t* sp = enterSpecial.sp();
> 333: 
> 334:   sp[-1] = (intptr_t)StubRoutines::cont_preempt_stub();

Same here sp-2 ?

src/hotspot/share/runtime/frame.cpp line 951:

> 949:       //       code in the interpreter calls a blocking runtime
> 950:       //       routine which can cause this code to be executed).
> 951:       //       (was bug gri 7/27/98)

I like the refactoring of this condition.  It may be finally time to remove this line from 1998.  I, for one, will miss it but it doesn't really help anyone with anything.

test/jdk/java/lang/Thread/virtual/KlassInit.java line 63:

> 61:  * @library /test/lib
> 62:  * @requires vm.debug == true & vm.continuations
> 63:  * @run junit/othervm/timeout=480 -XX:+UnlockDiagnosticVMOptions -XX:+FullGCALot -XX:FullGCALotInterval=1000 -XX:CompileCommand=exclude,KlassInit::lambda$testReleaseAtKlassInit* -XX:CompileCommand=exclude,KlassInit$$Lambda*::run -XX:CompileCommand=exclude,KlassInit$1Driver::foo KlassInit

I think you can use multiple lines for the run command and not have to wrap like this.

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

PR Review: https://git.openjdk.org/jdk/pull/27802#pullrequestreview-3376098254
PR Review Comment: https://git.openjdk.org/jdk/pull/27802#discussion_r2460039130
PR Review Comment: https://git.openjdk.org/jdk/pull/27802#discussion_r2460624836
PR Review Comment: https://git.openjdk.org/jdk/pull/27802#discussion_r2460575662
PR Review Comment: https://git.openjdk.org/jdk/pull/27802#discussion_r2460577616
PR Review Comment: https://git.openjdk.org/jdk/pull/27802#discussion_r2460697549
PR Review Comment: https://git.openjdk.org/jdk/pull/27802#discussion_r2460721081