<div style="font-family: Arial, sans-serif; font-size: 14px;"><div style="scrollbar-width:thin;scrollbar-color:rgba(0, 0, 0, 0.35) rgba(0, 0, 0, 0)"><p data-start="308" data-end="317" style="scrollbar-width:thin;scrollbar-color:rgba(0, 0, 0, 0.35) rgba(0, 0, 0, 0)">Hi all,</p><p data-start="319" data-end="594" style="scrollbar-width:thin;scrollbar-color:rgba(0, 0, 0, 0.35) rgba(0, 0, 0, 0)">I recently ran into an issue with<span> </span><code data-start="353" data-end="369" style="scrollbar-width:thin;scrollbar-color:rgba(0, 0, 0, 0.35) rgba(0, 0, 0, 0)">HttpClientImpl</code><span> </span>where I wanted to handle the reply right after calling<span> </span><code data-start="425" data-end="436" style="scrollbar-width:thin;scrollbar-color:rgba(0, 0, 0, 0.35) rgba(0, 0, 0, 0)">sendAsync</code>. What surprised me is that the returned<span> </span><code data-start="477" data-end="496" style="scrollbar-width:thin;scrollbar-color:rgba(0, 0, 0, 0.35) rgba(0, 0, 0, 0)">CompletableFuture</code><span> </span>already runs on the<span> </span><code data-start="517" data-end="529" style="scrollbar-width:thin;scrollbar-color:rgba(0, 0, 0, 0.35) rgba(0, 0, 0, 0)">commonPool</code>, instead of using the executor I provided to the<span> </span><code data-start="579" data-end="591" style="scrollbar-width:thin;scrollbar-color:rgba(0, 0, 0, 0.35) rgba(0, 0, 0, 0)">HttpClient</code>.</p><p data-start="596" data-end="660" style="scrollbar-width:thin;scrollbar-color:rgba(0, 0, 0, 0.35) rgba(0, 0, 0, 0)">Looking into the implementation, I noticed this piece of code:</p><span style="scrollbar-width:thin;scrollbar-color:rgba(0, 0, 0, 0.35) rgba(0, 0, 0, 0)"><i style="scrollbar-width:thin;scrollbar-color:rgba(0, 0, 0, 0.35) rgba(0, 0, 0, 0)">// makes sure that any dependent actions happen in the CF default</i></span><div style="scrollbar-width:thin;scrollbar-color:rgba(0, 0, 0, 0.35) rgba(0, 0, 0, 0)"><span style="scrollbar-width:thin;scrollbar-color:rgba(0, 0, 0, 0.35) rgba(0, 0, 0, 0)"><i style="scrollbar-width:thin;scrollbar-color:rgba(0, 0, 0, 0.35) rgba(0, 0, 0, 0)">// executor. This is only needed for sendAsync(...), when</i></span></div><div style="scrollbar-width:thin;scrollbar-color:rgba(0, 0, 0, 0.35) rgba(0, 0, 0, 0)"><span style="scrollbar-width:thin;scrollbar-color:rgba(0, 0, 0, 0.35) rgba(0, 0, 0, 0)"><i style="scrollbar-width:thin;scrollbar-color:rgba(0, 0, 0, 0.35) rgba(0, 0, 0, 0)">// exchangeExecutor is non-null.</i></span></div><div style="scrollbar-width:thin;scrollbar-color:rgba(0, 0, 0, 0.35) rgba(0, 0, 0, 0)"><span style="scrollbar-width:thin;scrollbar-color:rgba(0, 0, 0, 0.35) rgba(0, 0, 0, 0)"><i style="scrollbar-width:thin;scrollbar-color:rgba(0, 0, 0, 0.35) rgba(0, 0, 0, 0)">if (exchangeExecutor != null) {</i></span></div><div style="scrollbar-width:thin;scrollbar-color:rgba(0, 0, 0, 0.35) rgba(0, 0, 0, 0)"><span style="scrollbar-width:thin;scrollbar-color:rgba(0, 0, 0, 0.35) rgba(0, 0, 0, 0)"><i style="scrollbar-width:thin;scrollbar-color:rgba(0, 0, 0, 0.35) rgba(0, 0, 0, 0)">    res = res.whenCompleteAsync((r, t) -> { /* do nothing */}, ASYNC_POOL);</i></span></div><div style="scrollbar-width:thin;scrollbar-color:rgba(0, 0, 0, 0.35) rgba(0, 0, 0, 0)"><span style="scrollbar-width:thin;scrollbar-color:rgba(0, 0, 0, 0.35) rgba(0, 0, 0, 0)"><i style="scrollbar-width:thin;scrollbar-color:rgba(0, 0, 0, 0.35) rgba(0, 0, 0, 0)">}</i></span></div><span style="scrollbar-width:thin;scrollbar-color:rgba(0, 0, 0, 0.35) rgba(0, 0, 0, 0)"></span><br style="scrollbar-width:thin;scrollbar-color:rgba(0, 0, 0, 0.35) rgba(0, 0, 0, 0)"></div><div style="scrollbar-width:thin;scrollbar-color:rgba(0, 0, 0, 0.35) rgba(0, 0, 0, 0)"><p data-start="944" data-end="1231" style="scrollbar-width:thin;scrollbar-color:rgba(0, 0, 0, 0.35) rgba(0, 0, 0, 0)">I understand that this<span> </span><code data-start="967" data-end="985" style="scrollbar-width:thin;scrollbar-color:rgba(0, 0, 0, 0.35) rgba(0, 0, 0, 0)">exchangeExecutor</code><span> </span>is meant to cover the request/response exchange itself, not necessarily the<span> </span><code data-start="1062" data-end="1081" style="scrollbar-width:thin;scrollbar-color:rgba(0, 0, 0, 0.35) rgba(0, 0, 0, 0)">CompletableFuture</code><span> </span>chaining. But the fact that we always force the returned future back onto the<span> </span><code data-start="1160" data-end="1172" style="scrollbar-width:thin;scrollbar-color:rgba(0, 0, 0, 0.35) rgba(0, 0, 0, 0)">commonPool,</code> without any way to change this, feels quite limiting.</p><p data-start="1233" data-end="1382" style="scrollbar-width:thin;scrollbar-color:rgba(0, 0, 0, 0.35) rgba(0, 0, 0, 0)">In environments where the<span> </span><code data-start="1259" data-end="1271" style="scrollbar-width:thin;scrollbar-color:rgba(0, 0, 0, 0.35) rgba(0, 0, 0, 0)">commonPool</code><span> </span>is already heavily loaded, this can easily introduce performance issues or unpredictable behavior. And since</p><i style="scrollbar-width:thin;scrollbar-color:rgba(0, 0, 0, 0.35) rgba(0, 0, 0, 0)"><span style="scrollbar-width:thin;scrollbar-color:rgba(0, 0, 0, 0.35) rgba(0, 0, 0, 0)">private static final Executor ASYNC_POOL = new CompletableFuture<Void>().defaultExecutor();</span><br style="scrollbar-width:thin;scrollbar-color:rgba(0, 0, 0, 0.35) rgba(0, 0, 0, 0)"></i><span style="scrollbar-width:thin;scrollbar-color:rgba(0, 0, 0, 0.35) rgba(0, 0, 0, 0)"></span><br style="scrollbar-width:thin;scrollbar-color:rgba(0, 0, 0, 0.35) rgba(0, 0, 0, 0)"></div><div style="scrollbar-width:thin;scrollbar-color:rgba(0, 0, 0, 0.35) rgba(0, 0, 0, 0)"><p data-start="1491" data-end="1561" style="scrollbar-width:thin;scrollbar-color:rgba(0, 0, 0, 0.35) rgba(0, 0, 0, 0)">is fixed and cannot be replaced, users don’t have any way around it. For comparison, the default executor for <code data-start="1667" data-end="1698">CompletableFuture.supplyAsync</code> or for <code data-start="1706" data-end="1724">parallelStream()</code> is also the <code data-start="1737" data-end="1749">commonPool</code>, but in those cases it’s easy to override it with a custom executor. It would be nice if <code data-start="1839" data-end="1855">HttpClientImpl</code> offered the same flexibility.</p><p data-start="1563" data-end="1759" style="scrollbar-width:thin;scrollbar-color:rgba(0, 0, 0, 0.35) rgba(0, 0, 0, 0)">This is why I’d like to propose a change: when creating an<span> </span><code data-start="1610" data-end="1626" style="scrollbar-width:thin;scrollbar-color:rgba(0, 0, 0, 0.35) rgba(0, 0, 0, 0)">HttpClientImpl</code>, it should be possible to specify not only the exchange executor, but also the executor used for the returned<span> </span><code data-start="1737" data-end="1756" style="scrollbar-width:thin;scrollbar-color:rgba(0, 0, 0, 0.35) rgba(0, 0, 0, 0)">CompletableFuture</code></p><p data-start="1761" data-end="1883" style="scrollbar-width:thin;scrollbar-color:rgba(0, 0, 0, 0.35) rgba(0, 0, 0, 0)">This would be backwards compatible (just an additional optional builder parameter), and it could bring several benefits:</p><ul data-start="1884" data-end="2246" style="padding-inline-start:2.85714em;margin-block:0px;scrollbar-width:thin;scrollbar-color:rgba(0, 0, 0, 0.35) rgba(0, 0, 0, 0)"><li data-start="1884" data-end="1982" style="scrollbar-width:thin;scrollbar-color:rgba(0, 0, 0, 0.35) rgba(0, 0, 0, 0)"><p data-start="1886" data-end="1982" style="scrollbar-width:thin;scrollbar-color:rgba(0, 0, 0, 0.35) rgba(0, 0, 0, 0)">reduced context switching for clients that care about which thread executes response handling,</p></li><li data-start="1983" data-end="2082" style="scrollbar-width:thin;scrollbar-color:rgba(0, 0, 0, 0.35) rgba(0, 0, 0, 0)"><p data-start="1985" data-end="2082" style="scrollbar-width:thin;scrollbar-color:rgba(0, 0, 0, 0.35) rgba(0, 0, 0, 0)">more predictable performance in environments where<span> </span><code data-start="2036" data-end="2048" style="scrollbar-width:thin;scrollbar-color:rgba(0, 0, 0, 0.35) rgba(0, 0, 0, 0)">commonPool</code><span> </span>is shared with other workloads,</p></li><li data-start="2083" data-end="2162" style="scrollbar-width:thin;scrollbar-color:rgba(0, 0, 0, 0.35) rgba(0, 0, 0, 0)"><p data-start="2085" data-end="2162" style="scrollbar-width:thin;scrollbar-color:rgba(0, 0, 0, 0.35) rgba(0, 0, 0, 0)">easier integration with frameworks that already manage their own executors,</p></li><li data-start="2083" data-end="2162" style="scrollbar-width:thin;scrollbar-color:rgba(0, 0, 0, 0.35) rgba(0, 0, 0, 0)"><div data-start="2085" data-end="2162" style="scrollbar-width:thin;scrollbar-color:rgba(0, 0, 0, 0.35) rgba(0, 0, 0, 0);margin-top:14px;margin-bottom:14px"><span>clearer control and observability over where callbacks are executed.</span><br></div></li></ul><p data-start="2248" data-end="2360" style="scrollbar-width:thin;scrollbar-color:rgba(0, 0, 0, 0.35) rgba(0, 0, 0, 0)">Would such a change make sense, or is there a strong reason why we<span> </span><span data-start="2315" data-end="2321" style="scrollbar-width:thin;scrollbar-color:rgba(0, 0, 0, 0.35) rgba(0, 0, 0, 0)">must</span> always fallback to the<span> </span><code data-start="2345" data-end="2357" style="scrollbar-width:thin;scrollbar-color:rgba(0, 0, 0, 0.35) rgba(0, 0, 0, 0)">commonPool</code>?</p></div></div><div style="font-family: Arial, sans-serif; font-size: 14px;" class="protonmail_signature_block">
</div>