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

FISH-7670 Integerate Latest Concurro with Virtual Threads #6582

Merged
merged 16 commits into from Mar 11, 2024

Conversation

aubi
Copy link
Contributor

@aubi aubi commented Mar 4, 2024

Description

We donated Fork&Join Pool to Concurro plus one fix, so we can start using the upstream project again

This PR integrates the latest version and also implements usage of virtual threads

Testing

Testing Performed

I uploaded a testing application to the Jira ticket. The app defines its own MES:

@ManagedExecutorDefinition(name = "java:app/VirtMES", maxAsync = 10, virtual = true)
  1. Compile Payara 7 and the app, both with Java 21.
  2. Deploy the app to Payara
  3. Open http://localhost:8080/ConcurrencyVirtual-1.0-SNAPSHOT/rest/test
    It displays 3 stacktraces of the tasks, all of the ending with
java.base/java.lang.VirtualThread.run(VirtualThread.java:309)

E.g. the threads are executed in virtual threads!

Testing Environment

Linux, OpenJDK 21

Notes for Reviewers

I deployed concurro, version 3.1.0-SNAPSHOT to our snapshot nexus repository. We will use this until Concurro will not release a new version with all required changes.
The required change:
eclipse-ee4j/glassfish-concurro#82

@svendiedrichsen
Copy link
Contributor

There already is a Concurro Version 3.1.0-M2.

@Pandrex247 Pandrex247 self-requested a review March 8, 2024 13:49
Copy link
Member

@Pandrex247 Pandrex247 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to update copyright on various files.

There's also a dangling reference to the jakarta.enterprise.concurrent.jar artifact in appserver/pom.xml
https://github.com/aubi/Payara/blob/FISH-7670-use-concurro/appserver/pom.xml#L153

Do we not need to update the ManagedExecutorServiceManager and ManagedScheduledExecutorServiceManager, and ManagedThreadFactoryManager classes with the relevant changes for the new virtual threads option?

We also need to add the requisite options to the asadmin commands:

When doing the above, we'll also need to adjust the create & edit admin console pages respectively.

@aubi
Copy link
Contributor Author

aubi commented Mar 8, 2024

We also need to add the requisite options to the asadmin commands:

This is exactly the idea of task FISH-8302. We will have to store the setup in domain.xml, exactly as it done in Fork&Join pool.

This implementation uses the ManagedExecutorDefinition.virtual() flag to use virtual threads, e.g. enough to pass TCK and test this feature.

@aubi
Copy link
Contributor Author

aubi commented Mar 8, 2024

There already is a Concurro Version 3.1.0-M2.

@svendiedrichsen Yes. It was released the day before I finished this and added a fix for a bug, which causes this to fail: eclipse-ee4j/glassfish-concurro#82 :-(

I just merged this PR, so if you want to test this PR without our nexus, you can compile concurro/master locally.

@aubi aubi requested a review from Pandrex247 March 9, 2024 23:55
appserver/pom.xml Outdated Show resolved Hide resolved
Comment on lines +180 to +182
<groupId>org.glassfish.concurro</groupId>
<artifactId>concurro</artifactId>
</dependency>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still outstanding - you're adding 5 spaces here rather than 4.
I'd prefer if we fix the formatting of the other misaligned dependencies rather than compounding it.

aubi and others added 2 commits March 11, 2024 10:54
Co-authored-by: Andrew Pielage <pandrex247@hotmail.com>
Signed-off-by: Andrew Pielage <pandrex247@hotmail.com>
@aubi aubi merged commit 6cd8eb5 into payara:Payara7 Mar 11, 2024
1 check passed
@aubi aubi deleted the FISH-7670-use-concurro branch March 11, 2024 15:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants