Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support Loom Virtual Threads #268

Closed
Tracked by #458
smillidge opened this issue Dec 19, 2022 · 39 comments · Fixed by #349
Closed
Tracked by #458

Support Loom Virtual Threads #268

smillidge opened this issue Dec 19, 2022 · 39 comments · Fixed by #349
Labels
enhancement New feature or request
Milestone

Comments

@smillidge
Copy link
Contributor

Support Loom Virtual Threads across the apis where appropriate.

@smillidge smillidge added this to the 3.1 milestone Dec 19, 2022
@smillidge smillidge added the enhancement New feature or request label Dec 19, 2022
@njr-11
Copy link
Contributor

njr-11 commented Feb 7, 2023

We need to decide if on Java 21+ managed executors should just always use virtual threads to run async work, or if the application will want the opportunity to decide/configure which type of threads. From the Jakarta EE call this morning where members from the Java team went over virtual threads, it sounds like the main risk of switching to virtual threads is if applications are using thread locals in a way that expects a small number of threads to generally be reused over and over rather than a different thread each time a task runs. I would expect this sort of usage to be more rare, but there are probably applications out there that do it.

@hantsy
Copy link

hantsy commented May 25, 2023

Structured concurrency is also available in Java 21, I think it will resolve the thread local issue.

@njr-11
Copy link
Contributor

njr-11 commented May 25, 2023

Structured concurrency is also available in Java 21, I think it will resolve the thread local issue.

The last I saw, Structured Concurrency is proposed as a preview feature for Java 21, not stable production-ready function, in which case it would not be appropriate or the Jakarta Concurrency specification to build upon it yet, although we should be thinking about it for Jakarta EE 12 timeframe. If you have links to any information indicating plans for Structured Concurrency to be brought in as fully supported in Java 21, please share them, because it will definitely have impacts on the Jakarta Concurrency spec.

I will also point out that, as currently proposed, Structured Concurrency must be opted into by the user deciding to take advantage of the API, so it won't solve issues on its own. Application code would need to change to leverage it.

@aubi
Copy link
Contributor

aubi commented May 30, 2023

ad "We need to decide if on Java 21+ managed executors should just always use virtual threads..."

There are several issues we need to address -- they are breaking TCK:

Can we remove the requirement to implement the interface and change priority from TCK?

Also:

  • Virtual threads will easily influence other parts of JVM (e.g. other applications on the same server) when they are used by one application for CPU intensive calculations.

It would be nice to allow users to make the platform/virtual decision based on their needs, for example by setup of ManagedThreadFactory or ManagedExecutorService. I suppose it is good to have just one type of thread in one factory/service and not to mix them.

@njr-11
Copy link
Contributor

njr-11 commented May 30, 2023

It would be nice to allow users to make the platform/virtual decision based on their needs, for example by setup of ManagedThreadFactory or ManagedExecutorService. I suppose it is good to have just one type of thread in one factory/service and not to mix them.

I agree, given the behavior differences and other impacts, it seems like a good approach will be to let the user configure between virtual vs platform threads, which could easily be done at the level of the ManagedThreadFactoryDefinition/ManagedExecutorDefinition annotations that we added in EE 10.

One way to address the ManageableThread incompatibility without causing a breaking change could be to clarify in the documentation that the requirement for it only applies when using platform threads. We could also deprecate ManageableThread and isCurrentThreadShutdown if we plan to remove these in the future.

Regarding behavior differences, another one to note is that virtual threads have a fixed isDaemon value that cannot be changed.

@starksm64
Copy link
Contributor

We need to decide if on Java 21+ managed executors should just always use virtual threads to run async work, or if the application will want the opportunity to decide/configure which type of threads. From the Jakarta EE call this morning where members from the Java team went over virtual threads, it sounds like the main risk of switching to virtual threads is if applications are using thread locals in a way that expects a small number of threads to generally be reused over and over rather than a different thread each time a task runs. I would expect this sort of usage to be more rare, but there are probably applications out there that do it.

We don't think that this is that rare, for example, popular frameworks like Jackson do this.

@starksm64
Copy link
Contributor

In general, we are concerned about moving too fast to support Loom as we have a number of issues around how to integrate Loom with existing reactive and NIO based implementations in internal discussions regarding both Quarkus and Willdfly. Ideally we would like changes in the Loom SPI that it is not clear we will get or be able to implement externally.

@njr-11
Copy link
Contributor

njr-11 commented May 30, 2023

@starksm64 Would you be willing to share the issues you are running into and what you are requesting to be changed/added in Loom? If others expect they might run into the same issues, that could help build support for the Loom SPI that you are asking for.

@starksm64
Copy link
Contributor

I'm checking on whether we have something that can be made public, or if we are working on this with someone from the IBM team I can have you contact.

@starksm64
Copy link
Contributor

So here is a public repo illustrating some of the changes we are looking at requesting. Still very much a proof of concept, but illustrates the types of integration we currently think is needed to support integration with our Vert.x event loop code.

https://github.com/loomania/loomania

@hantsy
Copy link

hantsy commented Jun 3, 2023

@starksm64 In the loomania codes, I can not find something like the Kotlin CoroutinesContext when starting a scope, how to pass through or share the state data between contexts.

@njr-11
Copy link
Contributor

njr-11 commented Jun 12, 2023

One possibility could be to add a boolean config attribute to ManagedThreadFactoryDefinition, ManagedExecutorDefinition and ManagedScheduledExecutorDefinition that would allow the user to opt in to virtual threads where they find virtual threads advantageous, and otherwise use platform threads. For example,

@ManagedExecutorDefinition(name = "java:app/MyExecutor", ..., virtual = true)

@aubi
Copy link
Contributor

aubi commented Jun 28, 2023

I agree, just two notes:

  1. I vote to make the virtual behavior optional -- it's ok to provide platform threads if the server decides (or is configured).
  2. Can we come with a name related to expected load? In the future, when Java will come with some other options, virtual will be too limiting. I don't have good options, just thinking in a way "cpuIntensive", "computational"...

@njr-11
Copy link
Contributor

njr-11 commented Jun 28, 2023

  1. I vote to make the virtual behavior optional -- it's ok to provide platform threads if the server decides (or is configured).

We can certainly do that. We can define the meaning of the virtual option as a request for virtual threads, that a server might override with other configuration or mechanisms of its own. We actually have this exact same scenario where one of our users contacted us about making sure we provide an override to force the use of platform threads where we have control over it.

  1. Can we come with a name related to expected load? In the future, when Java will come with some other options, virtual will be too limiting. I don't have good options, just thinking in a way "cpuIntensive", "computational"...

Yes, that's certainly a possibility that Java could add other options in the future. I'm not able to think of a good name either, but maybe someone else will. We also won't know until new options from Java are proposed whether or not they will follow all of the same rules as virtual threads such that a user could seamlessly switch between the two. That will impact whether the more generic term can be used for both versus whether there will need to be a distinction between virtual and a new option because some users will care about coding to one vs the other. Another approach we could take here would be an enumeration that could accommodate additional values,

@ManagedExecutorDefinition(name = "java:app/MyExecutor", ..., threading = Threading.VIRTUAL)

In addition to not knowing what to name the enumeration either without knowing the future, the other obvious downside of the enumeration is that it is less concise than the boolean,

@ManagedExecutorDefinition(name = "java:app/MyExecutor", ..., virtual = true)

@starksm64
Copy link
Contributor

This document from Quarkus talks about three problematic areas with virtual threads we are currently seeing, monopolization, pinning and ThreadLocal usage:
https://github.com/quarkusio/quarkus/blob/main/docs/src/main/asciidoc/virtual-threads.adoc

I think at this point we would strongly recommend against inclusion of virtual thread support in EE11 due to the fact that insufficient usage and experience will have occurred with virtual threads in Java SE 21.

@starksm64
Copy link
Contributor

@starksm64 In the loomania codes, I can not find something like the Kotlin CoroutinesContext when starting a scope, how to pass through or share the state data between contexts.

StructuredConcurrency is the mechanism, but this will not be final in SE 21.

@aubi
Copy link
Contributor

aubi commented Jul 26, 2023

@starksm64
So far we are discussing an attribute virtual, which is just a hint for the server, that it is possible to use virtual threads.

E.g. the developer has to decide himself if the code should run in virtual thread. Also, the server doesn't have to use them, either not support them at all or based on some settings.

Do you think this is still a big risk?

@hantsy
Copy link

hantsy commented Jul 27, 2023

It seems Spring 6.1 focused on Virtual Thread.

Will provides seamless VT support in the embedded server(Tomcat/Jetty), and also provide compatibility with reactive stack.

@smillidge
Copy link
Contributor Author

I think a hint to the server is low risk especially if the default is to retain current behaviour and use real threads

@starksm64
Copy link
Contributor

But fundamentally this is an implementation detail of concurrent task execution. How is this going to be tested in a TCK? If it cannot be done portably, what is the point?

@njr-11
Copy link
Contributor

njr-11 commented Jul 27, 2023

But fundamentally this is an implementation detail of concurrent task execution. How is this going to be tested in a TCK? If it cannot be done portably, what is the point?

The TCK will specify @ManagedExecutorDefinition(..., virtual=true) and submit tasks to the corresponding executor, verifying that those tasks run concurrently and successfully accomplish the work that they were given to perform. If we wanted to enforce running on a virtual thread, there are some behavioral differences that we could test for. However, I thought you were not in favor of requiring vendors to support virtual threads and so we were planning to accommodate that by making the virtual option into a hint.

@starksm64
Copy link
Contributor

However, I thought you were not in favor of requiring vendors to support virtual threads and so we were planning to accommodate that by making the virtual option into a hint.

Exactly correct. So my question is, what is the benefit to expose this in public API since it is at best a hint?

@njr-11
Copy link
Contributor

njr-11 commented Jul 27, 2023

Exactly correct. So my question is, what is the benefit to expose this in public API since it is at best a hint?

The benefit is that the vendors/users who do want virtual threads in Java 21/Jakarta EE 11 will have a way of indicating to do so that is standardized in the specification for applications to take advantage of. I agree being optional is not ideal; it's a compromise. If we don't compromise we could make it required instead, but wouldn't that be worse for you?

Also, it looks like Quarkus has an annotation of its own called @RunOnVirtualThread whose JavaDoc starts out "If supported, ..." which also sounds optional.

@starksm64
Copy link
Contributor

Also, it looks like Quarkus has an annotation of its own called @RunOnVirtualThread whose JavaDoc starts out "If supported, ..." which also sounds optional.

So this captures the heart of why we don't want virtual threads exposed in EE. Quarkus is not just a user facing application framework. It is also for framework creators who understand integration with concurrent environments, and a Quarkus build produces a locked down runtime of the concurrency uses. The "if supported" aspect is the result of composition and depends on execution paths in combination with other asynchronous usage that is documented via the Blocking/NonBlocking annotations.

What we fully expect to happen in an EE environment where some component author has called for a virtual thread is that the application assembler will include a JDBC driver that uses thread pinning to control synchronization and component requests end up deadlocked.

@njr-11
Copy link
Contributor

njr-11 commented Jul 27, 2023

What we fully expect to happen in an EE environment where some component author has called for a virtual thread is that the application assembler will include a JDBC driver that uses thread pinning to control synchronization and component requests end up deadlocked.

Unfortunately there will be uninformed users trying to do that regardless of whether Jakarta EE 11 does anything about virtual threads. If Jakarta EE doesn't provide API around virtual threads, users will just request virtual threads directly from the Java SE APIs for it. That won't make things better or worse for the scenario you mentioned, but it will be worse for those who use virtual threads in well-behaved ways and lose out on running with the correct Jakarta EE context present and other value-add capability that the concurrency specification provides.

@smillidge
Copy link
Contributor Author

I think the risks are low in exposing a virtual hint given that platform threads will remain the default. A developer will have to explicitly turn on virtual threads for a managed executor they are explicitly defining therefore that developer should be aware of the downside risks described in the referenced doc. The default resources should have the same behaviour as before so no existing applications will be affected. As @njr-11 says if we do not provide then people will work around it. One of the goals of Jakarta EE concurrency is to mirror Java SE concurrency so this capability aligns with that goal.

@smillidge
Copy link
Contributor Author

Committer ballot started by @njr-11 see https://www.eclipse.org/lists/cu-dev/msg00452.html

@hantsy
Copy link

hantsy commented Aug 30, 2023

How about the ManagedThreadFactory, is there a virtual thread version for it?

For example, if possible to produce a Executor bean (instead of @ManagedExecutorDefinition) from a virtual ManagedThreadFactory?

@njr-11
Copy link
Contributor

njr-11 commented Aug 30, 2023

How about the ManagedThreadFactory, is there a virtual thread version for it?

For example, if possible to produce a Executor bean (instead of @ManagedExecutorDefinition) from a virtual ManagedThreadFactory?

If the vote passes to have a hint for requesting virtual threads, it will apply to @ManagedThreadFactoryDefinition as well as @ManagedExecutorService. And yes, you could have a bean that implements Executor and runs its tasks on threads from this ManagedThreadFactory.

njr-11 added a commit to njr-11/concurrency-api that referenced this issue Sep 28, 2023
njr-11 added a commit to njr-11/concurrency-api that referenced this issue Sep 28, 2023
@njr-11
Copy link
Contributor

njr-11 commented Oct 5, 2023

Oops, I didn't mean for the pull to close this out. Reopening.

@njr-11 njr-11 reopened this Oct 5, 2023
@jamezp
Copy link
Contributor

jamezp commented Dec 20, 2023

Given this will end up being a hint for virtual threads, is there any reason why the API should be compiled to Java SE 21? It seems reasonable for the API to compile to Java SE 17. An implementation can use Java SE 21 if they want or use a multi-release JAR if they want to support Java SE 17 and Java SE 21.

There has been an issue filed against the platform to allow implementations to certify against Java SE 17 and/or Java SE 21. If the minimum JVM requirement is reduced to Java SE 17, this would help and likely help adoption as well.

@njr-11
Copy link
Contributor

njr-11 commented Dec 20, 2023

Given this will end up being a hint for virtual threads, is there any reason why the API should be compiled to Java SE 21? It seems reasonable for the API to compile to Java SE 17. An implementation can use Java SE 21 if they want or use a multi-release JAR if they want to support Java SE 17 and Java SE 21.

There has been an issue filed against the platform to allow implementations to certify against Java SE 17 and/or Java SE 21. If the minimum JVM requirement is reduced to Java SE 17, this would help and likely help adoption as well.

If the issue is accepted, we could switch back to Java SE 17 compilation for the API. Although it is likely the TCK might want to use Java SE 21 methods to ensure that assertions match based on whether virtual thread or platform threads are used. For example, thread.getPriority() on a virtual thread is always NORM_PRIORITY, but with platform threads it will be the priority that was set on the thread.

@jamezp
Copy link
Contributor

jamezp commented Dec 20, 2023

I don't think the TCK could assume that though because support for Virtual Threads from an implementation is optional. However, maybe there is something that could be done with profiles and enabling those tests if Java SE 21 is used.

I can say for WildFly we would appreciate being able to certify with both Java SE 17 and Java SE 21. There are many users out there just now adopting Java SE 17 and likely not able to adopt, for various reason, Java SE 21 any time soon. Being able to certify both is a good move for adoption in my personal opinion.

@njr-11
Copy link
Contributor

njr-11 commented Dec 20, 2023

I don't think the TCK could assume that though because support for Virtual Threads from an implementation is optional. However, maybe there is something that could be done with profiles and enabling those tests if Java SE 21 is used.

The point is that the TCK will want to check if virtual threads are used so that it can avoid assuming incorrect things. In the example I gave the TCK needs to make an assertion on the the thread priority. Should it assert the priority to be what was set on the thread, or should it assert the priority to be NORM_PRIORITY? The correct way to make that assertion is to find out if the thread is virtual (via Java SE 21 API) and use that information to decide on the correct assertion.

@jamezp
Copy link
Contributor

jamezp commented Dec 20, 2023

Ah, okay, I see your point now. I'm sure there is a reasonable way to accomplish this and it looks like you've thought about it already :)

@starksm64
Copy link
Contributor

There is not a problem to validate Java SE 21 behavior when that is the detected test VM. The point of the platform issue is challenge the current baseline Java SE 21 requirement as meaning that is the minimal runtime for certifications. I suppose there should be a platform issue on what the behavior of the virtual thread flag should be at the platform level. If Java SE 17 is an allowed TCK runtime then it would have to be not supported at the platform. If Java SE 21 was required then either supported or not supported could be required. I think Red Hat would still prefer not required at the platform level that even if we had to certify under Java SE 21 we could still have a build supporting Java SE 17 and pass the concurrency TCK.

@starksm64
Copy link
Contributor

@njr-11 I'm stilling trying to understand the requirements with respect to virtual threads for 3.1. After reviewing the current project specification and javadoc, the only references to virtual threads are on the ManagedExecutorDefinition annotations. I cannot see a statement that requires a Concurrency provider to support the creation of VirtualThreads even if the ManagedExecutorDefinition#virtual member is true. Is there going to be a requirement in the TCK to configure a ManagedExecutorService to use VirtualThreads?

@njr-11
Copy link
Contributor

njr-11 commented Jan 9, 2024

@njr-11 I'm stilling trying to understand the requirements with respect to virtual threads for 3.1. After reviewing the current project specification and javadoc, the only references to virtual threads are on the Managed_ExecutorDefinition annotations. I cannot see a statement that requires a Concurrency provider to support the creation of VirtualThreads even if the Managed_ExecutorDefinition#virtual member is true. Is there going to be a requirement in the TCK to configure a ManagedExecutorService to use VirtualThreads?

That's good - what you stated is consistent with the outcome of the vote on this. While providers are required to support both true and false values for the virtual attribute, the meaning of true is carefully defined to serve as a hint/request, not a requirement to create virtual threads. So a provider complies with virtual=true by creating either type of thread. The TCK would test that true and false can be specified, and either way a thread is created that can be started and runs the Runnable given to it with the expected context on the thread. Any assertions that are made around aspects like the priority or isDaemon will of course depend on whether the thread is virtual or not, which the TCK can easily determine by checking thread.isVirtual() and make the appropriate assertions.

njr-11 added a commit to njr-11/concurrency-api that referenced this issue Jan 9, 2024
njr-11 added a commit to njr-11/concurrency-api that referenced this issue Jan 9, 2024
@KyleAure KyleAure mentioned this issue Mar 18, 2024
19 tasks
@njr-11
Copy link
Contributor

njr-11 commented Mar 19, 2024

Closing because this has been added in Concurrency 3.1.

@njr-11 njr-11 closed this as completed Mar 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants