RFR: 8371475: HttpClient: Implement CUBIC congestion controller

[Volkan Yazici]

On Fri, 7 Nov 2025 13:38:23 GMT, Daniel Jeliński <djelinski at openjdk.org> wrote:

>> CUBIC is a standard TCP congestion control algorithm that uses a cubic function instead of a linear congestion window increase function to improve scalability and stability over fast and long-distance networks. CUBIC has been adopted as the default TCP congestion control algorithm by the Linux, Windows, and Apple stacks.
> 
> This PR adds a new congestion controller algorithm. It reuses a large part of the QuicRenoCongestionController, which was refactored to two classes - QuicBaseCongestionController, containing the shared code, and QuicRenoCongestionController, containing only the code that is unique to Reno.
> 
> CUBIC is now the default congestion controller. Reno can still be selected by setting the system property `jdk.httpclient.quic.congestionController` to `reno`.
> 
> A new test was added to exercise the new congestion controller. Existing tests continue to pass.

Really neat work @djelinski! I liked the surgery you carried out in `QuicBaseCC`. 💯

I've dropped some minor remarks. I guess we will need some time to wrap our mind around the math involved in the PR.

src/java.net.http/share/classes/jdk/internal/net/http/quic/QuicConnectionImpl.java line 371:

> 369:     private static QuicCongestionController createCongestionController
> 370:             (String dbgTag, QuicRttEstimator rttEstimator) {
> 371:         String algo = System.getProperty("jdk.httpclient.quic.congestionController", "cubic");

Can we keep this internal? That is, can we use `jdk.internal.httpclient.quic.congestionController` instead?

src/java.net.http/share/classes/jdk/internal/net/http/quic/QuicCubicCongestionController.java line 41:

> 39:  *       RFC 9438: CUBIC for Fast and Long-Distance Networks
> 40:  */
> 41: public class QuicCubicCongestionController extends QuicBaseCongestionController {

I guess this can be `final`.

src/java.net.http/share/classes/jdk/internal/net/http/quic/QuicCubicCongestionController.java line 62:

> 60:     public QuicCubicCongestionController(String dbgTag, QuicRttEstimator rttEstimator) {
> 61:         super(dbgTag, rttEstimator);
> 62:         this.rttEstimator = rttEstimator;

Can we make `rttEstimator` a `protected` field of `QuicBaseCongestionController`?

src/java.net.http/share/classes/jdk/internal/net/http/quic/QuicCubicCongestionController.java line 112:

> 110:     protected boolean congestionAvoidanceAcked(int packetBytes, Deadline sentTime) {
> 111:         boolean isAppLimited;
> 112:         isAppLimited = sentTime.isAfter(lastFullWindow);

Suggestion:

        boolean isAppLimited = sentTime.isAfter(lastFullWindow);

src/java.net.http/share/classes/jdk/internal/net/http/quic/QuicCubicCongestionController.java line 122:

> 120:             // target = Wcubic(t)
> 121:             // this is less aggressive than RFC 9438, which uses target=Wcubic(t+RTT),
> 122:             // but seems to work well enough

Shall we also document why we deviate from the RFC?

src/java.net.http/share/classes/jdk/internal/net/http/quic/QuicRenoCongestionController.java line 40:

> 38:  *      RFC 9002: QUIC Loss Detection and Congestion Control
> 39:  */
> 40: class QuicRenoCongestionController extends QuicBaseCongestionController {

I guess thins can be `final` and consequently `protected` modifier can be removed from the implemented methods.

src/java.net.http/share/classes/jdk/internal/net/http/quic/QuicRenoCongestionController.java line 47:

> 45:     protected boolean congestionAvoidanceAcked(int packetBytes, Deadline sentTime) {
> 46:         boolean isAppLimited;
> 47:         isAppLimited = congestionWindow > maxBytesInFlight + 2L * maxDatagramSize;

Suggestion:

        boolean isAppLimited = congestionWindow > maxBytesInFlight + 2L * maxDatagramSize;


@djelinski, I see you verbatim copied these lines – which is fine, and makes things easier to review. Nevertheless, I want to double-check: do we need to guard against any arithmetic overflows here?

test/jdk/java/net/httpclient/quic/CubicTest.java line 42:

> 40: /*
> 41:  * @test
> 42:  * @run testng/othervm -Djdk.httpclient.HttpClient.log=trace,quic:cc CubicTest

Shall we use JUnit instead?

-------------

PR Review: https://git.openjdk.org/jdk/pull/28195#pullrequestreview-3435245896
PR Review Comment: https://git.openjdk.org/jdk/pull/28195#discussion_r2504693701
PR Review Comment: https://git.openjdk.org/jdk/pull/28195#discussion_r2504645030
PR Review Comment: https://git.openjdk.org/jdk/pull/28195#discussion_r2504653829
PR Review Comment: https://git.openjdk.org/jdk/pull/28195#discussion_r2504673398
PR Review Comment: https://git.openjdk.org/jdk/pull/28195#discussion_r2504672752
PR Review Comment: https://git.openjdk.org/jdk/pull/28195#discussion_r2504622773
PR Review Comment: https://git.openjdk.org/jdk/pull/28195#discussion_r2504632463
PR Review Comment: https://git.openjdk.org/jdk/pull/28195#discussion_r2504680316