Fixes #8007 - Support Loom. #8360
Conversation
Alternative implementation of support for virtual threads for HTTP/1.1, HTTP/2 and HTTP/3. The virtual thread support is in AdaptiveExecutionStrategy. When virtual threads are supported and enabled, reserved threads are disabled and blocking tasks are run in a virtual thread instead that being executed by the Executor. Signed-off-by: Simone Bordet <[email protected]>
|
@gregw @lorban please review this alternative support for Loom, more in line with what suggested by @gregw. It's not complete as we need to verify that all code paths that call application code are actually invoked with a virtual thread (so more testing needed), but it's functional enough to deserve a review to see if it is the right direction before doing more work. |
|
Keep in mind that the prior Loom support PR #8035 still exists and is still open |
There was a problem hiding this comment.
This is looking much better.
I've made a couple of comments / niggles.
But I also don't like the way useVirtualThreads boolean is passed around.
Rather than configure connectors, I'm wondering if it could be a filed/attribute/property associated with the thread pool? The strategy could then look at the threadpool it has been given and work out how to act from that.
jetty-util/src/main/java/org/eclipse/jetty/util/thread/strategy/AdaptiveExecutionStrategy.java
Outdated
Show resolved
Hide resolved
jetty-util/src/main/java/org/eclipse/jetty/util/thread/strategy/AdaptiveExecutionStrategy.java
Outdated
Show resolved
Hide resolved
jetty-util/src/main/java/org/eclipse/jetty/util/thread/strategy/AdaptiveExecutionStrategy.java
Outdated
Show resolved
Hide resolved
jetty-quic/quic-server/src/main/java/org/eclipse/jetty/quic/server/QuicServerConnector.java
Outdated
Show resolved
Hide resolved
jetty-server/src/main/java/org/eclipse/jetty/server/Connector.java
Outdated
Show resolved
Hide resolved
Now virtual thread configuration is done in ThreadPool implementation. ReservedThreadExecutor heuristics has been changed to take into account virtual thread support, so that by default the number of reserved threads is 0 when using virtual threads. Signed-off-by: Simone Bordet <[email protected]>
Signed-off-by: Simone Bordet <[email protected]>
There was a problem hiding this comment.
This looks pretty good.... but other than the niggles I've commented on, you also need to modify jetty-threadpool.xml and module to have a property for useVirtualThreads.
Or if you like, you could have a loom.mod that depended on thread pool and set the property.
jetty-util/src/main/java/org/eclipse/jetty/util/thread/strategy/AdaptiveExecutionStrategy.java
Outdated
Show resolved
Hide resolved
jetty-util/src/main/java/org/eclipse/jetty/util/thread/strategy/AdaptiveExecutionStrategy.java
Show resolved
Hide resolved
Updated threadpool Jetty module and XML files. Retrieving whether to use virtual threads from AdaptiveExecutionStrategy.doStart() to allow for late configuration of the thread pool. Signed-off-by: Simone Bordet <[email protected]>
| if (isUseVirtualThreads()) | ||
| VirtualThreads.startVirtualThread(task); | ||
| else | ||
| _executor.execute(task); |
There was a problem hiding this comment.
Should have a test in AdaptiveExecutionStrategyTest for this. It should start a billion threads and be OK :)
There was a problem hiding this comment.
Billions are a thing of the past. I'll use trillions, because that's what's needed these days with 0.75 CPUs in the cloud.
Alternative implementation of support for virtual threads for HTTP/1.1, HTTP/2 and HTTP/3.
The virtual thread support is in AdaptiveExecutionStrategy.
When virtual threads are supported and enabled, reserved threads are disabled and
blocking tasks are run in a virtual thread instead that being executed by the Executor.
Signed-off-by: Simone Bordet [email protected]