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

Thu Oct 23 13:54:26 UTC 2025

On Mon, 20 Oct 2025 20:26:44 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:
> 
>   PPC support

Nice work!  This is a first pass with comments.  I haven't looked at the platform specific code or tests yet.

src/hotspot/cpu/aarch64/smallRegisterMap_aarch64.inline.hpp line 34:

> 32: 
> 33: // Java frames don't have callee saved registers (except for rfp), so we can use a smaller RegisterMap
> 34: template <bool IncludeArgs>

Can we have a follow-on RFE to make SmallRegisterMap and it's new template in shared code.  And only have the platform specific functions here?

src/hotspot/cpu/aarch64/smallRegisterMap_aarch64.inline.hpp line 73:

> 71:   bool update_map()    const { return false; }
> 72:   bool walk_cont()     const { return false; }
> 73:   bool include_argument_oops() const { return IncludeArgs; }

You made this a template rather than having an _include_argument_oops property for performance?

src/hotspot/share/interpreter/interpreterRuntime.cpp line 837:

> 835:         note_trap(current, Deoptimization::Reason_null_check);
> 836:       }
> 837:       CLEAR_PENDING_PREEMPTED_EXCEPTION;

You clear this because we want the preempted exception to cause a return to here, but not return an exception to the interpreter to rethrow?  Can you add a comment why, especially if I've got this wrong.

src/hotspot/share/interpreter/linkResolver.hpp line 192:

> 190: };
> 191: 
> 192: enum class ClassInitMode : uint8_t {

If this is just a parameter, does this need to be uint8_t ?

src/hotspot/share/oops/instanceKlass.cpp line 810:

> 808: void InstanceKlass::initialize_preemptable(TRAPS) {
> 809:   if (this->should_be_initialized()) {
> 810:     PREEMPT_ON_INIT_SUPPORTED_ONLY(PreemptableInitCall pic(THREAD, this);)

Are there any other uses of this macro?

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

> 1121:       assert(!call.is_valid() || call.is_invokestatic(), "only invokestatic allowed");
> 1122:       if (call.is_invokestatic() && call.size_of_parameters() > 0) {
> 1123:         assert(top_frame.interpreter_frame_expression_stack_size() > 0, "should have parameters in exp stack");

The "this" pointer will be on the top of the interpreter frame expression stack right?  Are size of parameters ever 0 here then?

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

> 1651:   }
> 1652:   inline intptr_t* anchor_mark_set_pd();
> 1653:   inline void anchor_mark_clear_pd();

The code is really confused whether "pd" is the beginning of the method name or the end.  Both are used but mostly in the beginning of the method. The freeze/thaw code uses it at the end so this is okay.

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

> 1729:   int bci;
> 1730:   if (preempt_kind == Continuation::monitorenter) {
> 1731:     assert(top.is_interpreted_frame() || top.is_runtime_frame(), "");

So could you use precond() here since it's a precondition and you have no assert message?

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

> 2059: 
> 2060:   // Only used for preemption on ObjectLocker
> 2061:   ObjectMonitor* _monitor;

Rather than calling it monitor, you could call it _init_lock so that it makes more sense in the following code.  The other reason to give it the same name as in initialization is so in the future we'll see the connection easily.

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

> 2684: 
> 2685:   {
> 2686:     HandleMarkCleaner hmc(current);  // Cleanup so._conth Handle

Why doesn't a plain HandleMark do this?
I think you chose HandleMarkCleaner because this is going to back to the interpreter code, so to be like JRT_ENTRY code.
So add something like before going back to the interpreted Java code.

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

> 2694: 
> 2695:     // These InterpreterRuntime entry points use JRT_ENTRY which uses a HandleMarkCleaner.
> 2696:     // Create a HandeMark to avoid destroying so._conth.

I see. The comment is helpful.

src/hotspot/share/runtime/javaThread.hpp line 540:

> 538:     }
> 539:   };
> 540: #endif

Nit: add // ASSERT to the endif.
I always wonder if these big blocks of code added to thread.hpp and javaThread.hpp should be in some new file, and just referenced from this.  Not for this change. Just a general comment.

src/hotspot/share/runtime/javaThread.hpp line 1372:

> 1370: };
> 1371: 
> 1372: class PreemptableInitCall {

If this is only used in instanceKlass.cpp, I think the definition should be there and not in javaThread.hpp.  It's not using private methds in javaThread.hpp as far as I can tell.

src/hotspot/share/runtime/javaThread.hpp line 1421:

> 1419: };
> 1420: 
> 1421: class ThreadWaitingForClassInit : public StackObj {

I think this should be in instanceKlass.cpp also near the single caller (unless there's another caller that I don't see).

src/hotspot/share/runtime/objectMonitor.cpp line 319:

> 317:   check_object_context();
> 318:   if (_object_strong.is_empty()) {
> 319:     if (Thread::TrySpinAcquire(&_object_strong_lock)) {

@toxaart Here's a new use of this SpinAcquire/SpinRelease as a TrySpinAcquire.

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

PR Review: https://git.openjdk.org/jdk/pull/27802#pullrequestreview-3367219722
PR Review Comment: https://git.openjdk.org/jdk/pull/27802#discussion_r2454882893
PR Review Comment: https://git.openjdk.org/jdk/pull/27802#discussion_r2454888028
PR Review Comment: https://git.openjdk.org/jdk/pull/27802#discussion_r2453118945
PR Review Comment: https://git.openjdk.org/jdk/pull/27802#discussion_r2453122999
PR Review Comment: https://git.openjdk.org/jdk/pull/27802#discussion_r2454901203
PR Review Comment: https://git.openjdk.org/jdk/pull/27802#discussion_r2455074172
PR Review Comment: https://git.openjdk.org/jdk/pull/27802#discussion_r2455083202
PR Review Comment: https://git.openjdk.org/jdk/pull/27802#discussion_r2455091984
PR Review Comment: https://git.openjdk.org/jdk/pull/27802#discussion_r2455104045
PR Review Comment: https://git.openjdk.org/jdk/pull/27802#discussion_r2455123338
PR Review Comment: https://git.openjdk.org/jdk/pull/27802#discussion_r2455138736
PR Review Comment: https://git.openjdk.org/jdk/pull/27802#discussion_r2455164468
PR Review Comment: https://git.openjdk.org/jdk/pull/27802#discussion_r2454898223
PR Review Comment: https://git.openjdk.org/jdk/pull/27802#discussion_r2454906094
PR Review Comment: https://git.openjdk.org/jdk/pull/27802#discussion_r2455185797