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

JGRP-2851 Use thread factory for virtual threads #863

Conversation

cfredri4
Copy link
Contributor

@cfredri4 cfredri4 commented Nov 8, 2024

This change makes ThreadPool use the supplied ThreadFactory also for virtual threads, which means they will have correct names etc

@cfredri4 cfredri4 changed the title Use thread factory for virtual threads JGRP-2851 Use thread factory for virtual threads Nov 8, 2024
@cfredri4 cfredri4 force-pushed the use-thread-factory-for-virtual-threads branch from 699c92b to 401a236 Compare November 14, 2024 18:51
@belaban
Copy link
Owner

belaban commented Nov 19, 2024

OK, I don't really care about JDKs < 17. But since 17 is LTS (IIRC), I'd like to support JDK 17 and higher. Your code searches for Executors.newThreadPerTaskExecutor(Factory), which is fine in JDKs > 17, but I cannot find this method in JDK 17.
Thoughts?

@cfredri4
Copy link
Contributor Author

JDK 17 is LTS, but virtual threads are only available in Loom early access builds for JDK 15/16/17/18 (and as preview feature from JDK 19).
The code previously supported Loom early access builds so I tried to replicate that. But maybe support for Loom early access builds can be dropped and instead virtual threads only need to be supported for real builds (>=JDK19)? This would make ThreadPool+ThreadCreator simpler.

@belaban
Copy link
Owner

belaban commented Nov 20, 2024

I agree that virtual thread support doesn't need to be in JDKs in which they were preview features. Nobody would run with preview enabled in production...

This change makes ThreadPool use the supplied ThreadFactory also for virtual threads, which means they will have correct names etc
@cfredri4 cfredri4 force-pushed the use-thread-factory-for-virtual-threads branch from 401a236 to 382f960 Compare November 20, 2024 11:25
@cfredri4
Copy link
Contributor Author

Rebased+pushed with simplifications to only support real builds.

@belaban
Copy link
Owner

belaban commented Nov 20, 2024

Applied

@belaban belaban closed this Nov 20, 2024
@cfredri4 cfredri4 deleted the use-thread-factory-for-virtual-threads branch November 25, 2024 14:20
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.

2 participants