On Fri, 14 Nov 2025 12:18:19 GMT, Daniel Fuchs <[email protected]> wrote:
>> 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. Done. > 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); > } Done. > 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; Refactored the code, I think it's easier to follow now. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/28195#discussion_r2549245195 PR Review Comment: https://git.openjdk.org/jdk/pull/28195#discussion_r2549245648 PR Review Comment: https://git.openjdk.org/jdk/pull/28195#discussion_r2549248271
