On Wed, 19 Nov 2025 19:03:28 GMT, Daniel Fuchs <[email protected]> wrote:
>> Each HttpClient instance creates an additional platform thread for its >> SelectorManager. With recent updates to NIO/VirtualThreads that thread could >> now become a VirtualThread. This would avoid having each HttpClient instance >> use up one platform thread. >> This is similar to what was done for the HttpClient QuicSelectorThread in >> [JDK-8369920](https://bugs.openjdk.org/browse/JDK-8369920). >> This should be transparent for users of the API. >> An undocumented internal system property is introduced that can revert the >> change in case of unforeseen trouble. > > Daniel Fuchs has updated the pull request incrementally with one additional > commit since the last revision: > > Revert changes to SelectorManager::shutdown src/java.net.http/share/classes/jdk/internal/net/http/HttpClientImpl.java line 131: > 129: > 130: static final UseVTForSelector USE_VT_FOR_SELECTOR = > 131: > Utils.useVTForSelector("jdk.internal.httpclient.tcp.selector.useVirtualThreads", > "default"); Hello Daniel, I vaguely remember we discussed whether or not to use a value called "default" for this property. But I don't remember what we decided. Looking at this now, I am wondering whether we should just do something like: System.getProperty("jdk.internal.httpclient.tcp.selector.useVirtualThreads", "true") to keep it simple? test/jdk/java/net/httpclient/ReferenceTracker.java line 393: > 391: } > 392: > 393: private boolean isSelectorManager(Thread t) { The copyright year on this file will need an update. test/jdk/java/net/httpclient/http2/H2SelectorVTTest.java line 145: > 143: throw new AssertionError("Unexpected null sslContext"); > 144: } > 145: // create an H3 only server The use of "H3" here looks like a typo. test/jdk/java/net/httpclient/http2/H2SelectorVTTest.java line 175: > 173: final HttpRequest req1 = reqBuilder.copy().GET().build(); > 174: System.out.println("\nIssuing request: " + req1); > 175: final HttpResponse<?> resp1 = client.send(req1, > BodyHandlers.ofString()); Nit - this line and the other 3 send() lines could perhaps use `BodyHandlers.discarding()`? If you prefer to use `ofString()`, that's fine too - it wasn't clear to me if this was an oversight. test/jdk/java/net/httpclient/http2/H2SelectorVTTest.java line 195: > 193: } > 194: > 195: private static void assertSelectorThread(HttpClient client) { I think a comment on what this method checks would be useful for future. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/28395#discussion_r2545102282 PR Review Comment: https://git.openjdk.org/jdk/pull/28395#discussion_r2545125815 PR Review Comment: https://git.openjdk.org/jdk/pull/28395#discussion_r2545108417 PR Review Comment: https://git.openjdk.org/jdk/pull/28395#discussion_r2545116878 PR Review Comment: https://git.openjdk.org/jdk/pull/28395#discussion_r2545124094
