Skip to content

Support Tyrus ExecutorService using virtual threads#927

Merged
jansupol merged 1 commit intoeclipse-ee4j:2.0.xfrom
jansupol:virtual.threads
May 30, 2025
Merged

Support Tyrus ExecutorService using virtual threads#927
jansupol merged 1 commit intoeclipse-ee4j:2.0.xfrom
jansupol:virtual.threads

Conversation

@jansupol
Copy link
Copy Markdown
Contributor

No description provided.

@jansupol jansupol force-pushed the virtual.threads branch 2 times, most recently from 03d4f62 to e906e0c Compare May 29, 2025 19:33
@jansupol jansupol added this to the 2.0.8 milestone May 30, 2025
@jansupol jansupol merged commit fe54f5b into eclipse-ee4j:2.0.x May 30, 2025
3 checks passed
@jansupol jansupol deleted the virtual.threads branch May 30, 2025 06:13
Comment on lines +122 to +127
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);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@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?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ah, I see what you mean. Will look into that.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@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).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

But thank you for reviewing this PR.

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.

3 participants