eclipse-ee4j/jersey

Project Loom/JDK 21 compatible

Closed this issue · 12 comments

We are preparing ourselves to upgrade to jdk 21, and was excited to see the addition of the JavaNetHttpConnectorProvider, because it would result in less dependencies to manage and automatically support virtual threads. However, after scanning the code in this repo, I see usages of synchronized methods which will result in pinned threads. Is there work under way to replace these locks with locking strategies that will not result in pinning the thread? If so, what version can I look forward to trying out that is more "project loom" friendly?

That's a good point, we planned to revisit synchronized blocks all over the code.

Will the use of ThreadLocals migrate to using some of the new scoping functionality in 21? ie ScopedValue and StructuredTaskScope? Will this lead to just a change in behaviour for Jersey's own @requestScope?

ThreadLocals might or might not be an issue, depending on what the ThreadLocal holds. If the hold value is a subject just for a single virtual thread, it is not that an issue, afaik.

But yes, the ThreadLocal use should be revisited, too.

We have replaced the synchronized blocks for 3.1.6, along with minor ThreadLocal changes to clear the storage. However, right now we do not plan to drop the ThreadLocals as they work with virtual threads. We revisited the usage and it should not cause any harm with virtual threads. The Jersey @RequestScope behavior should not be affected. We do not plan to use ScopedValues (still maintaining support for JDK 11) unless there would be a reason. Do you have any actual bottleneck in mind?

That said, Jersey works quite well with Loom in Helidon 4 (JDK 21) already.

@jansupol when is the 3.1.6 release happening? We (https://github.com/trinodb/trino) would love to try it out with virtual threads :)

@wendigo The next week, likely.

Great! Thanks @jansupol

What would be the appropriate way to enable virtual threads when using jersey-container-jetty-http or jersey-container-jetty-http2?

We're currently using JettyHttpContainerFactory, but the thread pool is apparently hardcoded https://github.com/eclipse-ee4j/jersey/blob/3.1/containers/jetty-http/src/main/java17/org/glassfish/jersey/jetty/JettyHttpContainerFactory.java#L257

We have the CommonProperties#THREAD_FACTORY and CommonProperties#USE_VIRTUAL_THREADS properties now. Does it suffice your needs @celtric ?

mkarg commented

I wonder why this issue is still open, as apparently the intended work is done in Jersey 3.1.6+

We have the CommonProperties#THREAD_FACTORY and CommonProperties#USE_VIRTUAL_THREADS properties now. Does it suffice your needs @celtric ?

Sorry for the late reply, missed your comment. I confirm CommonProperties#USE_VIRTUAL_THREADS works, thank you!

Closing as resolved