Skip to content

Conversation

dmlloyd
Copy link
Member

@dmlloyd dmlloyd commented Sep 16, 2025

This works by reducing the amount of time spent by the JVM calling back into the class loader, which is generally all pinned time. Instead of having the JVM recursively load supertypes of the loaded type, we do it ourselves on the Java side ahead of time so when the JVM loads the superclass, it is already present.

See #43022 for more discussion on the topic.

@quarkus-bot quarkus-bot bot added the area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins label Sep 16, 2025
This works by reducing the amount of time spent by the JVM calling back into the class loader, which is generally all pinned time. Instead of having the JVM recursively load supertypes of the loaded type, we do it ourselves on the Java side ahead of time so when the JVM loads the superclass, it is already present.
@franz1981
Copy link
Contributor

franz1981 commented Sep 17, 2025

Instead of having the JVM recursively load supertypes of the loaded type, we do it ourselves on the Java side ahead of time

This is beneficial only if the cost we will pay on C2 to warmup exceed the cost of the orignal JVM code (likely a mix of native calling into java and forth? I don't remember):

  • when the JVM does it via native code it doesn't requires any form of hotspot compilation: it's "always slow/fast" (and it's not great it keep on getting in/out from JNI code to java back and forth)
  • when we do it instead it's Java, it means that we will spend cycles to compile it assuming it happens so frequently that the usually big compiler cost (in term of memory and cpu time) pay off itself with faster code (and no pinning)

I'm checking the code in the meantime 🙏

@franz1981
Copy link
Contributor

franz1981 commented Sep 17, 2025

Thinking about it twice, I need to recap what pinning really is, at the lowest possible level..
I've re-read what trigger the existing changes in the class loader i.e. #40917 (comment)

And checking where the JVM currently issue a pinning event i.e.:

  1. https://github.com/openjdk/jdk/blob/e1071797a4f0ab1a6af29824a777a7800d729b0e/src/hotspot/share/runtime/objectMonitor.cpp#L645 after a failed attempt to preempt the current continuation (i.e. unmount it) while entering in a contended monitor (see https://github.com/openjdk/jdk/blame/e1071797a4f0ab1a6af29824a777a7800d729b0e/src/hotspot/share/runtime/objectMonitor.cpp#L549 too)
  2. https://github.com/openjdk/jdk/blob/master/src/hotspot/share/runtime/javaThread.cpp#L2300-L2324 evaluate the result of the failure to preempt the continuation (which check the nature of the caller frame)

And, looking the stack trace at #40917 (comment) (which doesn't have monitor involved, it seems) it could be summarized in:
whenever the attempt to preempt a continuation fail and the carrier is parked instead, it's pinning.

The reason why is dangerous is that, similar to monipolization, it prevents the carrier to make progress and run other virtual threads.
We can say that it "consumes" the available capacity/concurrency of the default scheduler - and if exhausted, none can make progress.
For why class loading or native frames fail to preempt a continuation, it's because their stack cannot safely be unmounted and/or they could hold important JDK monitors (class loading maybe, IDK).

Now: does the existing class loader pin a virtual thread performing class loading to a carrier?
AFAIK, nope, see

// Virtual threads needs to load the jarfile synchronously to avoid blocking. This means that eventually
// multiple threads could load the same jarfile in parallel and this duplication has to be reconciled

In short, in the new class loader we never park a virtual thread but we eventually try to define, failing with linkage error.
This is not great, but is not pinning, because in a finite number of steps (attempt to define, failure and exit) the virtual thread can make progress and let others continue.

Said that, I need to wrap my head better re-phrasing what this PR does:
how the stack trace of class loading will look like after this? it saves many (failed) concurrent attempts to class define to happen?

@dmlloyd
Copy link
Member Author

dmlloyd commented Sep 17, 2025

Instead of having the JVM recursively load supertypes of the loaded type, we do it ourselves on the Java side ahead of time

This is beneficial only if the cost we will pay on C2 to warmup exceed the cost of the orignal JVM code (likely a mix of native calling into java and forth? I don't remember):

I think the tradeoff is likely to be different, but we'll have to measure to see for sure.

  • when the JVM does it via native code it doesn't requires any form of hotspot compilation: it's "always slow/fast" (and it's not great it keep on getting in/out from JNI code to java back and forth)

I think the big cost will be the initial I/O to load the class bytes. I think that iterating the class bytes is going to be a relatively small slice even when not fully warmed up. But only measuring will tell us for sure.

  • when we do it instead it's Java, it means that we will spend cycles to compile it assuming it happens so frequently that the usually big compiler cost (in term of memory and cpu time) pay off itself with faster code (and no pinning)

I'm checking the code in the meantime 🙏

I'll comment on those threads separately.

@dmlloyd
Copy link
Member Author

dmlloyd commented Sep 17, 2025

Thinking about it twice, I need to recap what pinning really is, at the lowest possible level.. I've re-read what trigger the existing changes in the class loader i.e. #40917 (comment)

And checking where the JVM currently issue a pinning event i.e.:

  1. https://github.com/openjdk/jdk/blob/e1071797a4f0ab1a6af29824a777a7800d729b0e/src/hotspot/share/runtime/objectMonitor.cpp#L645 after a failed attempt to preempt the current continuation (i.e. unmount it) while entering in a contended monitor (see https://github.com/openjdk/jdk/blame/e1071797a4f0ab1a6af29824a777a7800d729b0e/src/hotspot/share/runtime/objectMonitor.cpp#L549 too)
  2. https://github.com/openjdk/jdk/blob/master/src/hotspot/share/runtime/javaThread.cpp#L2300-L2324 evaluate the result of the failure to preempt the continuation (which check the nature of the caller frame)

And, looking the stack trace at #40917 (comment) (which doesn't have monitor involved, it seems) it could be summarized in: whenever the attempt to preempt a continuation fail and the carrier is parked instead, it's pinning.

I'm not 100% sure I agree, or maybe I misunderstand you. In that stack trace, there are hidden frames (native IIRC): the JVM is late-linking a class method.

In short, in the new class loader we never park a virtual thread but we eventually try to define, failing with linkage error. This is not great, but is not pinning, because in a finite number of steps (attempt to define, failure and exit) the virtual thread can make progress and let others continue.

Said that, I need to wrap my head better re-phrasing what this PR does: how the stack trace of class loading will look like after this? it saves many (failed) concurrent attempts to class define to happen?

If the JVM calls loadClass, I believe we're already pinned (because it's native code that does lazy linking), no matter what, so this patch doesn't make any difference. But for other cases, if we load a class, then we can do some of the work before defineClass pins and recursively loads other classes.

I guess we could check to see if the current thread is virtual, and if so, check to see if it is pinned, and if so, skip doing the preload work because it won't help anyway.

Like I said in another issue, the best fix for pinning is to probably somehow reschedule the virtual thread to a separate carrier pool before the JVM initiates the class load. Everything else is a bandage.

@franz1981
Copy link
Contributor

franz1981 commented Sep 17, 2025

In that stack trace, there are hidden frames (native IIRC): the JVM is late-linking a class method.

Yep, I think I've written too much, making my point less clear 🙏
The way I've intended "pinning" is === "emitting a pinning event", which happen when the JIT fail to preempt a continuation where it needs to do it (e.g. on a contended monitor, LockSuppor.unpark, LockSupport.yield, etc etc) for several reasons (recorded in the pinning event).
In this regard, the current classloader algorithm is designed to not emit such event, because it doesn't perform any blocking operation which requires the continuation to be preempted.
But as you (correctly) said, since you intend w pinning the state which would prevent the preemption to happen, if needed, regardless if there is or not any reason to do it, just entering in loadClass is pinning the virtual thread to the carrier.

Like I said in another issue, the best fix for pinning is to probably somehow reschedule the virtual thread to a separate carrier pool before the JVM initiates the class load. Everything else is a bandage.

This is very true, but at the same time, since the reasons which make a preemption necessary are removed (we removed in the algorithm any blocking operations AFAIK), it shouldn't be a big deal, unless I'm not considering some other hidden blocking reason/contention.
Said that, the progress guarantees of what I've designed are still poor, because a deep enough series of class loads can mount for quite long time the virtual thread to the carrier, still preventing other VT to make progress.

It's still a torrent of words from me, sorry, but I hope clarify a bit my point

@dmlloyd
Copy link
Member Author

dmlloyd commented Sep 22, 2025

I think this probably won't improve much, since the vast majority of class loading cases are going to come from run-time linking, which will cause the thread to pin before we even get a chance to do anything. So, closing this one for now. We can revisit later if it turns out there's a bigger win than expected somehow.

@dmlloyd dmlloyd closed this Sep 22, 2025
@quarkus-bot quarkus-bot bot added the triage/invalid This doesn't seem right label Sep 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins triage/invalid This doesn't seem right

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants