On Fri, 14 Nov 2025 13:22:37 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/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? Well that's an interesting question. The RFC does not suggest clamping wMaxBytes in any way, so we sometimes do end up with wMaxBytes < congestionWindow, and kNanos < 0, even in the `testReduction` test. That seems to be in line with the RFC. A negative `k` value is pretty well tolerated by the algorithm. It just means that we will start increasing the window slightly faster than we would otherwise. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/28195#discussion_r2533944799
