8371475: HttpClient: Implement CUBIC congestion controller #28195

djelinski · 2025-11-07T13:38:23Z

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.

Progress

Change must be properly reviewed (1 review required, with at least 1 Reviewer)
Change must not contain extraneous whitespace
Commit message must refer to an issue

Issue

JDK-8371475: HttpClient: Implement CUBIC congestion controller (Enhancement - P4)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/28195/head:pull/28195
$ git checkout pull/28195

Update a local copy of the PR:
$ git checkout pull/28195
$ git pull https://git.openjdk.org/jdk.git pull/28195/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 28195

View PR using the GUI difftool:
$ git pr show -t 28195

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/28195.diff

Using Webrev

Link to Webrev Comment

bridgekeeper · 2025-11-07T13:40:10Z

👋 Welcome back djelinski! A progress list of the required criteria for merging this PR into pr/28156 will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2025-11-07T13:42:10Z

❗ This change is not yet ready to be integrated.
See the Progress checklist in the description for automated requirements.

openjdk · 2025-11-07T13:43:09Z

@djelinski The following label will be automatically applied to this pull request:

net

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

mlbridge · 2025-11-07T14:01:21Z

Webrevs

AlanBateman · 2025-11-07T14:09:31Z

If jdk.httpclient.quic.congestionController is exposed then we'll need to track this with a CSR.

vy

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.

vy · 2025-11-07T17:05:05Z

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

-
-    private final QuicPacer pacer;
-
+class QuicRenoCongestionController extends QuicBaseCongestionController {


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

Changed the class to final. The protected modifier is still needed on overridden methods.

vy · 2025-11-07T17:07:37Z

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

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


Suggested change

boolean isAppLimited;

isAppLimited = congestionWindow > maxBytesInFlight + 2L * maxDatagramSize;

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?

Suggestion applied.

MaxBytesInFlight is limited by the amount of available memory, and soft-limited to 16M (see MAX_BYTES_IN_FLIGHT in base congestion controller). While it can cross the limit briefly, I don't expect it to get anywhere near the arithmetic limits.

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

test/jdk/java/net/httpclient/quic/CubicTest.java

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

openjdk-notifier · 2025-11-12T12:40:22Z

The parent pull request that this pull request depends on has now been integrated and the target branch of this pull request has been updated. This means that changes from the dependent pull request can start to show up as belonging to this pull request, which may be confusing for reviewers. To remedy this situation, simply merge the latest changes from the new target branch into this pull request by running commands similar to these in the local repository for your personal fork:

git checkout quic-cubic
git fetch https://git.openjdk.org/jdk.git master
git merge FETCH_HEAD
# if there are conflicts, follow the instructions given by git merge
git commit -m "Merge master"
git push

openjdk · 2025-11-12T12:42:48Z

@djelinski this pull request can not be integrated into master due to one or more merge conflicts. To resolve these merge conflicts and update this pull request you can run the following commands in the local repository for your personal fork:

git checkout quic-cubic
git fetch https://git.openjdk.org/jdk.git master
git merge FETCH_HEAD
# resolve conflicts and follow the instructions given by git merge
git commit -m "Merge master"
git push

djelinski

Thanks @AlanBateman @vy for the reviews.

The property was not meant to be exposed; IMO CUBIC is superior to Reno in all aspects that matter. I renamed it to include internal.

We will probably need to revisit the configuration if and when we implement BBR, but we aren't quite there yet.

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

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

djelinski · 2025-11-12T14:07:43Z

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

-
-    private final QuicPacer pacer;
-
+class QuicRenoCongestionController extends QuicBaseCongestionController {


Changed the class to final. The protected modifier is still needed on overridden methods.

djelinski · 2025-11-12T14:16:57Z

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

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


Suggestion applied.

MaxBytesInFlight is limited by the amount of available memory, and soft-limited to 16M (see MAX_BYTES_IN_FLIGHT in base congestion controller). While it can cross the limit briefly, I don't expect it to get anywhere near the arithmetic limits.

test/jdk/java/net/httpclient/quic/CubicTest.java

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

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

dfuch · 2025-11-14T12:18:19Z

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

+    protected long congestionWindow = INITIAL_WINDOW;
+    protected int maxDatagramSize = QuicConnectionImpl.DEFAULT_DATAGRAM_SIZE;
+    protected int minimumWindow = 2 * maxDatagramSize;
+    protected long bytesInFlight;
+    // maximum bytes in flight seen since the last congestion event
+    protected long maxBytesInFlight;
+    protected Deadline congestionRecoveryStartTime;
+    protected long ssThresh = Long.MAX_VALUE;


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

dfuch · 2025-11-14T12:30:39Z

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

+    protected QuicBaseCongestionController(String dbgTag, QuicRttEstimator rttEstimator) {
+        this.dbgTag = dbgTag;
+        this.timeSource = TimeSource.source();
+        this.pacer = new QuicPacer(rttEstimator, this);
+    }
+
+    // for testing
+    protected QuicBaseCongestionController(TimeLine source, QuicRttEstimator rttEstimator) {
+        this.dbgTag = "TEST";
+        this.timeSource = source;
+        this.pacer = new QuicPacer(rttEstimator, this);
+    }


Suggested change

protected QuicBaseCongestionController(String dbgTag, QuicRttEstimator rttEstimator) {

this.dbgTag = dbgTag;

this.timeSource = TimeSource.source();

this.pacer = new QuicPacer(rttEstimator, this);

}

// for testing

protected QuicBaseCongestionController(TimeLine source, QuicRttEstimator rttEstimator) {

this.dbgTag = "TEST";

this.timeSource = source;

this.pacer = new QuicPacer(rttEstimator, this);

}

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);

}

dfuch · 2025-11-14T12:32:56Z

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

+
+    // for testing
+    public QuicCubicCongestionController(TimeLine source, QuicRttEstimator rttEstimator) {
+        super(source, rttEstimator);


Suggested change

super(source, rttEstimator);

super("TEST", source, rttEstimator);

dfuch · 2025-11-14T12:53:28Z

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

+        boolean isAppLimited = sentTime.isAfter(lastFullWindow);
+        if (!isAppLimited) {
+            if (wEstBytes < cwndPriorBytes) {
+                wEstBytes += Math.max((long) (ALPHA * maxDatagramSize * packetBytes / congestionWindow), 1);


should we assert that congestionWindow is > 2 ?

dfuch · 2025-11-14T12:59:59Z

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

+            if (dblTargetBytes > 1.5 * congestionWindow) {
+                targetBytes = (long) (1.5 * congestionWindow);


Suggested change

if (dblTargetBytes > 1.5 * congestionWindow) {

targetBytes = (long) (1.5 * congestionWindow);

// targetLimit is 1.5 * congestionWindow

long targetLimit = congestionWindow + (congestionWindow >> 1)

if (dblTargetBytes > targetLimit) {

targetBytes = targetLimit;

dfuch · 2025-11-14T13:03:44Z

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

+                targetBytes = (long)dblTargetBytes;
+            }
+            if (targetBytes > congestionWindow) {
+                congestionWindow += Math.max((targetBytes - congestionWindow) * packetBytes / congestionWindow, 1L);


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

Should we assert that congestionWindow is >= minimumWindow after this operation?

dfuch · 2025-11-14T13:22:37Z

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

+        // set lastFullWindow to prevent rapid timeNanos growth
+        lastFullWindow = congestionRecoveryStartTime;
+        // ((wmax_segments - cwnd_segments) / C) ^ (1/3) seconds
+        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?

dfuch · 2025-11-14T15:29:43Z

And another question: after this change - do we still have tests for the Reno congestion controller? Or is everthing using Cubic?

djelinski added 17 commits November 5, 2025 15:45

Implement packet pacing

dacbb90

Less verbose CC logging

9478fe6

Keep track of pacer-limited events

f2393d0

Fix CRLF

be15a20

Fix race in pacer updates, increase low-RTT quota

dab80e5

Initial cubic commit

274c13a

Implement cubic

8e8515b

Log K in milliseconds

859b7ab

Cubic tests, more aggressive Reno window increase

de7ebe9

Documentation updates

98e26c3

Use custom timeline for testing

fcd9a71

Test the cubic curve

2d82033

Revert: more aggressive window increases

8ecbf0d

Update test

060678c

Add comments

f1fdc21

Implement fast convergence

eda81c7

Add a system property to select congestion controller

b95950f

openjdk bot changed the title ~~8371475~~ 8371475: HttpClient: Implement CUBIC congestion controller Nov 7, 2025

openjdk bot added the net net-dev@openjdk.org label Nov 7, 2025

djelinski marked this pull request as ready for review November 7, 2025 13:56

openjdk bot added the rfr Pull request is ready for review label Nov 7, 2025

vy reviewed Nov 7, 2025

View reviewed changes

djelinski added 2 commits November 12, 2025 12:32

Rename system property to internal

894a394

Make classes final

fde4d86

openjdk-notifier bot changed the base branch from pr/28156 to master November 12, 2025 12:40

openjdk bot added the merge-conflict Pull request has merge conflict with target branch label Nov 12, 2025

djelinski added 5 commits November 12, 2025 14:16

Merge remote-tracking branch 'origin/master' into quic-cubic

61c96fe

More aggressive target growth

74b80eb

Merge declaration and assignment

d4e3e60

Convert CubicTest to JUnit

ff7ecf6

Update test comments

195b0f8

openjdk bot removed the merge-conflict Pull request has merge conflict with target branch label Nov 12, 2025

djelinski commented Nov 12, 2025

View reviewed changes

vy reviewed Nov 13, 2025

View reviewed changes

src/java.net.http/share/classes/jdk/internal/net/http/quic/QuicBaseCongestionController.java Show resolved Hide resolved

dfuch reviewed Nov 14, 2025

View reviewed changes


		private final QuicPacer pacer;

		class QuicRenoCongestionController extends QuicBaseCongestionController {

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

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

	super(source, rttEstimator);
	super("TEST", source, rttEstimator);

		if (dblTargetBytes > 1.5 * congestionWindow) {
		targetBytes = (long) (1.5 * congestionWindow);

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

8371475: HttpClient: Implement CUBIC congestion controller #28195

Are you sure you want to change the base?

8371475: HttpClient: Implement CUBIC congestion controller #28195

Uh oh!

Conversation

djelinski commented Nov 7, 2025 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issue

Reviewing

Uh oh!

bridgekeeper bot commented Nov 7, 2025

Uh oh!

openjdk bot commented Nov 7, 2025

Uh oh!

openjdk bot commented Nov 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mlbridge bot commented Nov 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Webrevs

Uh oh!

AlanBateman commented Nov 7, 2025

Uh oh!

vy left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

openjdk-notifier bot commented Nov 12, 2025

Uh oh!

openjdk bot commented Nov 12, 2025

Uh oh!

djelinski left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dfuch commented Nov 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

djelinski commented Nov 7, 2025 •

edited by openjdk bot

Loading

openjdk bot commented Nov 7, 2025 •

edited

Loading

mlbridge bot commented Nov 7, 2025 •

edited

Loading

dfuch commented Nov 14, 2025 •

edited

Loading