Support Tyrus ExecutorService using virtual threads#927
Support Tyrus ExecutorService using virtual threads#927jansupol merged 1 commit intoeclipse-ee4j:2.0.xfrom
Conversation
03d4f62 to
e906e0c
Compare
Signed-off-by: jansupol <[email protected]>
| final ScheduledExecutorService scheduler = Executors.newSingleThreadScheduledExecutor(); | ||
| final ExecutorService executorService = Executors.newThreadPerTaskExecutor(threadFactory); | ||
|
|
||
| @Override | ||
| public ScheduledFuture<?> schedule(Runnable command, long delay, TimeUnit unit) { | ||
| return scheduler.schedule(() -> executorService.submit(command).get(), delay, unit); |
There was a problem hiding this comment.
Perhaps I am misreading this, but would this not block the single (traditional) thread managed by scheduler, forcing all such tasks to be run serially even though executorService can use as many (virtual) threads as it likes? (and similarly for the other schedule overload)
While this suggestion looks similar, it does not involve blocking the heavyweight thread as the code here does.
There was a problem hiding this comment.
I have not tried it but I suspect the right approach is to create the newSingleThreadScheduledExecutor as here (but preferably passing it a thread factory creating a virtual thread as well), and then implement the schedule overloads something like
class ScheduledCompletableFuture extends CompletableFuture<?> {
// implement getDelay somehow
}
var scf = new ScheduledCompletableFuture();
scheduler.schedule(() -> {
var f = executorService.submit(command);
executorService.execute(() -> {
try {
scf.complete(f.get());
} catch (Throwable x) {
scf.completeExceptionally(x);
}
});
}, delay, unit);
return scf;There was a problem hiding this comment.
@jglick What is the difference between this PR and the suggestion?
The idea is that the platform thread only schedules tasks executed by the virtual threads. Scheduling tasks should not block the platform thread, as the long-running operations are executed by the virtual threads. Or am I missing something?
There was a problem hiding this comment.
Ah, I see what you mean. Will look into that.
There was a problem hiding this comment.
@jglick It occurs that the ScheduledExecutorService using virtual threads is missing in JDK on purpose. The complexity of handling every corner-case of using the virtual threads for ScheduledExecutorService (the replanning of the tasks to start after the existing task is finished late, error-handling, ...) is too great for now.
I am reverting the ScheduledExecutorService to the platform thread version (#928).
There was a problem hiding this comment.
But thank you for reviewing this PR.
No description provided.