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

Integration blocker

⚠️ Dependency #28156 must be integrated first

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

00: Full (b95950f2)

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.

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?

vy · 2025-11-07T17:10:42Z

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

+ * @spec https://www.rfc-editor.org/rfc/rfc9438.html
+ *       RFC 9438: CUBIC for Fast and Long-Distance Networks
+ */
+public class QuicCubicCongestionController extends QuicBaseCongestionController {


I guess this can be final.

vy · 2025-11-07T17:13:09Z

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

+        super(dbgTag, rttEstimator);
+        this.rttEstimator = rttEstimator;


Can we make rttEstimator a protected field of QuicBaseCongestionController?

vy · 2025-11-07T17:18:16Z

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

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


Shall we also document why we deviate from the RFC?

vy · 2025-11-07T17:18:28Z

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

+        boolean isAppLimited;
+        isAppLimited = sentTime.isAfter(lastFullWindow);


Suggested change

boolean isAppLimited;

isAppLimited = sentTime.isAfter(lastFullWindow);

boolean isAppLimited = sentTime.isAfter(lastFullWindow);

vy · 2025-11-07T17:20:09Z

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

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


Shall we use JUnit instead?

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

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


+    private static QuicCongestionController createCongestionController
+            (String dbgTag, QuicRttEstimator rttEstimator) {
+        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?

djelinski added 12 commits November 6, 2025 12:09

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


		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(dbgTag, rttEstimator);
		this.rttEstimator = rttEstimator;

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

		boolean isAppLimited;
		isAppLimited = sentTime.isAfter(lastFullWindow);

	boolean isAppLimited;
	isAppLimited = sentTime.isAfter(lastFullWindow);
	boolean isAppLimited = sentTime.isAfter(lastFullWindow);

8371475: HttpClient: Implement CUBIC congestion controller #28195

Are you sure you want to change the base?

8371475: HttpClient: Implement CUBIC congestion controller #28195

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

Integration blocker

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

Uh oh!

mlbridge bot commented Nov 7, 2025

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!

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!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

djelinski commented Nov 7, 2025 •

edited by openjdk bot

Loading