Skip to content

Use virtual threads by default on Java 21+ via Multi-Release JAR#60

Draft
Copilot wants to merge 7 commits intomainfrom
copilot/use-virtual-threads-default
Draft

Use virtual threads by default on Java 21+ via Multi-Release JAR#60
Copilot wants to merge 7 commits intomainfrom
copilot/use-virtual-threads-default

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 7, 2026

Resolves #98


Before the change?

  • All internal SDK threads (JSON-RPC reader, CLI stderr forwarder) use platform daemon threads via direct new Thread() and Executors.newSingleThreadExecutor() calls, regardless of JVM version.

After the change?

  • On Java 21+, SDK internal I/O threads automatically use virtual threads via a Multi-Release JAR (JEP 238) overlay. On Java 17–20, platform daemon threads are used. No user configuration required.

Core abstraction:

  • ThreadFactoryProvider in src/main/java/ — baseline returning platform threads
  • ThreadFactoryProvider in src/main/java21/ — overlay using Thread.ofVirtual().name(...) with Executors.newSingleThreadExecutor() backed by a virtual-thread factory

Call sites updated:

  • JsonRpcClient — reader executor now via ThreadFactoryProvider.newSingleThreadExecutor("jsonrpc-reader")
  • CliServerManager — stderr thread now via ThreadFactoryProvider.newThread(..., "cli-stderr-reader")
  • CopilotSession ScheduledThreadPoolExecutorunchanged (no virtual-thread scheduled executor in JDK)

Build:

  • Multi-Release: true manifest entry
  • JDK 21+ Maven profile adds maven-compiler-plugin execution with --release 21 + multiReleaseOutput=true
  • Spotless includes src/main/java21/**/*.java
  • Builds cleanly on JDK 17 (JAR just omits the overlay)

Code review fixes:

  • Removed unused LOG field and java.util.logging.Logger import from both ThreadFactoryProvider classes
  • Java 21 overlay now uses Executors.newSingleThreadExecutor(Thread.ofVirtual().name(name).factory()) instead of newThreadPerTaskExecutor(...) to provide a true single-thread executor matching the method contract
  • Replaced empty "Verifying Virtual Thread Usage" section in advanced.md with actionable guidance (using jcmd thread dumps and IDE thread inspectors)

Docs: README, site docs (index, getting-started, advanced), jbang example, Javadoc on CopilotClient/CopilotSession.

Pull request checklist

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been reviewed and added / updated if needed (for bug fixes / features)
  • mvn spotless:apply has been run to format the code
  • mvn clean verify passes locally

Does this introduce a breaking change?

  • Yes
  • No

Copilot AI linked an issue Apr 7, 2026 that may be closed by this pull request
Copilot AI changed the title [WIP] Use virtual threads by default on Java 21 Use virtual threads by default on Java 21+ via Multi-Release JAR Apr 7, 2026
Copilot AI requested a review from brunoborges April 7, 2026 14:06
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR introduces a multi-release JAR approach to automatically use virtual threads on Java 21+ for the SDK’s internal I/O threading while keeping Java 17 as the baseline runtime.

Changes:

  • Add a ThreadFactoryProvider abstraction in src/main/java/ and a Java 21+ override in src/main/java21/ to switch internal threads to virtual threads via MR-JAR.
  • Update build configuration (manifest Multi-Release: true, compile src/main/java21 on JDK 21+ profile, Spotless includes).
  • Update internal thread creation call sites and refresh docs/examples to describe virtual thread behavior.
Show a summary per file
File Description
pom.xml Enables MR-JAR manifest entry; compiles Java 21 sources under the JDK 21+ profile; expands Spotless includes to cover src/main/java21.
src/main/java/com/github/copilot/sdk/ThreadFactoryProvider.java Baseline (Java 17) thread/executor factory implementation using platform daemon threads.
src/main/java21/com/github/copilot/sdk/ThreadFactoryProvider.java Java 21+ MR-JAR overlay switching to virtual threads / virtual-thread executors.
src/main/java/com/github/copilot/sdk/JsonRpcClient.java Routes JSON-RPC reader loop executor creation through ThreadFactoryProvider.
src/main/java/com/github/copilot/sdk/CliServerManager.java Routes CLI stderr forwarding thread creation through ThreadFactoryProvider.
src/test/java/com/github/copilot/sdk/ThreadFactoryProviderTest.java Adds unit tests for the provider.
src/main/java/com/github/copilot/sdk/CopilotClient.java Javadoc update documenting Java 21+ virtual thread behavior and scheduler exception.
src/main/java/com/github/copilot/sdk/CopilotSession.java Javadoc update clarifying scheduled timeout threads remain platform threads.
README.md Updates requirements wording and adds a “Virtual Threads” section.
src/site/markdown/index.md Recommends Java 21+ for automatic virtual thread support.
src/site/markdown/getting-started.md Recommends Java 21+ for automatic virtual thread support.
src/site/markdown/advanced.md Adds an advanced “Virtual Threads (Java 21+)” section.
jbang-example.java Notes automatic virtual thread usage on Java 21+.

Copilot's findings

  • Files reviewed: 13/13 changed files
  • Comments generated: 6

Comment on lines +25 to +30
final class ThreadFactoryProvider {

private static final Logger LOG = Logger.getLogger(ThreadFactoryProvider.class.getName());

private ThreadFactoryProvider() {
}
Copy link

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

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

LOG is declared but never used in this class. Please remove it (and any now-unneeded logging import) to avoid dead code and future confusion.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@copilot apply changes based on this feedback

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.

Removed unused LOG field and java.util.logging.Logger import from both ThreadFactoryProvider files (baseline and Java 21 overlay). Also fixed the Java 21 overlay to use Executors.newSingleThreadExecutor(...) instead of newThreadPerTaskExecutor(...) for a true single-thread executor, and replaced the empty verification section in advanced.md with actionable guidance. See commit 9d456f6.

Comment on lines +21 to +26
final class ThreadFactoryProvider {

private static final Logger LOG = Logger.getLogger(ThreadFactoryProvider.class.getName());

private ThreadFactoryProvider() {
}
Copy link

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

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

LOG is declared but never used in this class. Please remove it (and any now-unneeded logging import) to avoid dead code and future confusion.

Copilot uses AI. Check for mistakes.
Comment on lines +1263 to +1270
### Verifying Virtual Thread Usage

You can check at runtime whether the SDK is using virtual threads:

```java
// Thread names are preserved for debuggability regardless of thread type.
// On Java 21+, the jsonrpc-reader and cli-stderr-reader threads will be virtual.
```
Copy link

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

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

This section claims users can verify virtual thread usage but the code block is empty, and ThreadFactoryProvider is package-private so it can’t be referenced from user code anyway. Either provide a concrete, user-accessible verification method/example (e.g., documented logging output / thread dump indicators), or remove the section.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@copilot apply changes based on this feedback

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.

Already addressed in commit 9d456f6 — replaced the empty code block with concrete verification guidance (using jcmd thread dumps and IDE thread inspectors), and removed the reference to the package-private ThreadFactoryProvider class.

brunoborges and others added 2 commits April 7, 2026 10:07
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
….java

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
if (javaVersion < 21) {
assertFalse(result, "Expected platform threads on Java < 21");
} else if (result) {
assertTrue(javaVersion >= 21, "Virtual threads are only supported on Java 21+");
…rlay, fix empty docs section

Agent-Logs-Url: https://github.com/github/copilot-sdk-java/sessions/46706dd3-402c-41c9-9150-3cfae7db8c3f

Co-authored-by: brunoborges <129743+brunoborges@users.noreply.github.com>
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.

Use virtual threads by default on Java 21+

3 participants