RFR: 8371475: HttpClient: Implement CUBIC congestion controller [v3]

[Daniel Fuchs]

On Wed, 12 Nov 2025 15:13:18 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.
>
> Daniel Jeliński has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 24 commits:
> 
>  - Update test comments
>  - Convert CubicTest to JUnit
>  - Merge declaration and assignment
>  - More aggressive target growth
>  - Merge remote-tracking branch 'origin/master' into quic-cubic
>  - Make classes final
>  - Rename system property to internal
>  - Add a system property to select congestion controller
>  - Implement fast convergence
>  - Add comments
>  - ... and 14 more: https://git.openjdk.org/jdk/compare/1f1f7bb4...195b0f89

src/java.net.http/share/classes/jdk/internal/net/http/quic/QuicBaseCongestionController.java line 69:

> 67:     protected long maxBytesInFlight;
> 68:     protected Deadline congestionRecoveryStartTime;
> 69:     protected long ssThresh = Long.MAX_VALUE;

Can we remove the `protected` keyword on the mutable fields? All the subclasses are in the same package.

src/java.net.http/share/classes/jdk/internal/net/http/quic/QuicBaseCongestionController.java line 84:

> 82:         this.timeSource = source;
> 83:         this.pacer = new QuicPacer(rttEstimator, this);
> 84:     }

Suggestion:

    protected QuicBaseCongestionController(String dbgTag, QuicRttEstimator rttEstimator) {
        this(dbgTag, TimeSource.source(), rttEstimator);
    }

    // Allows to pass a custom TimeLine when testing
    QuicBaseCongestionController(String dbgTag, TimeLine source, QuicRttEstimator rttEstimator) {
        this.dbgTag = dbgTag;
        this.timeSource = source;
        this.pacer = new QuicPacer(rttEstimator, this);
    }

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

> 65:     // for testing
> 66:     public QuicCubicCongestionController(TimeLine source, QuicRttEstimator rttEstimator) {
> 67:         super(source, rttEstimator);

Suggestion:

        super("TEST", source, rttEstimator);

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

> 112:         if (!isAppLimited) {
> 113:             if (wEstBytes < cwndPriorBytes) {
> 114:                 wEstBytes += Math.max((long) (ALPHA * maxDatagramSize * packetBytes / congestionWindow), 1);

should we assert that congestionWindow is > 2 ?

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

> 122:             // not sure if dblTarget can overflow a long, but 1.5 congestionWindow can not.
> 123:             if (dblTargetBytes > 1.5 * congestionWindow) {
> 124:                 targetBytes = (long) (1.5 * congestionWindow);

Suggestion:

            // targetLimit is 1.5 * congestionWindow
            long targetLimit = congestionWindow + (congestionWindow >> 1)
            if (dblTargetBytes > targetLimit) {
                targetBytes = targetLimit;

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

> 127:             }
> 128:             if (targetBytes > congestionWindow) {
> 129:                 congestionWindow += Math.max((targetBytes - congestionWindow) * packetBytes / congestionWindow, 1L);

Can `(targetBytes - congestionWindow) * packetBytes / congestionWindow` overflow?

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

> 160:         lastFullWindow = congestionRecoveryStartTime;
> 161:         // ((wmax_segments - cwnd_segments) / C) ^ (1/3) seconds
> 162:         kNanos = (long)(Math.cbrt((wMaxBytes - congestionWindow) / C / maxDatagramSize) * 1_000_000_000);

should we assert that wMaxBytes >=  congestionWindow ? It is not immediately obvious to me that this is guaranteed.

In particular:


        if (congestionWindow < wMaxBytes) {
            // fast convergence
            wMaxBytes = (long) ((1 + BETA) * congestionWindow / 2);


seems to imply that `wMaxBytes` will be less than congestion window since 1.7 < 2

So we would end up with a negative value for kNanos, which is defined as:

> The time period in seconds it takes to increase the congestion window size at the beginning of the current congestion avoidance stage to Wmax.

so presumably negative values for kNanos are not expected?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/28195#discussion_r2527297674
PR Review Comment: https://git.openjdk.org/jdk/pull/28195#discussion_r2527332413
PR Review Comment: https://git.openjdk.org/jdk/pull/28195#discussion_r2527340117
PR Review Comment: https://git.openjdk.org/jdk/pull/28195#discussion_r2527400131
PR Review Comment: https://git.openjdk.org/jdk/pull/28195#discussion_r2527418638
PR Review Comment: https://git.openjdk.org/jdk/pull/28195#discussion_r2527429228
PR Review Comment: https://git.openjdk.org/jdk/pull/28195#discussion_r2527493627