feature(transport/cubic): dynamic changing of alpha value#2985
feature(transport/cubic): dynamic changing of alpha value#2985omansfeld wants to merge 1 commit intomozilla:mainfrom
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2985 +/- ##
==========================================
- Coverage 93.39% 93.35% -0.04%
==========================================
Files 124 124
Lines 35972 35989 +17
Branches 35972 35989 +17
==========================================
+ Hits 33595 33599 +4
- Misses 1532 1545 +13
Partials 845 845
|
|
#2973 is in, will this progress now? |
In #2973 we split off #3043 which was the part the test here depends on. Because we don't increase our Apart from that I believe we also decided that this is one of the CUBIC parts that we will rather defer to later, similar to #3043. It would make sense to rebase all the changes in here so it is in a similar state as #3043, ready to be picked up again when time comes. I will update the description and rebase once all CUBIC changes that can be merged now are in. |
This implements changing CUBIC_ALPHA to `1.0` when `w_est >= cwnd_prior` as per <https://datatracker.ietf.org/doc/html/rfc9438#section-4.3-11>. - added `self.alpha` field - added `self.cwnd_prior` field - existing functions now use `self.alpha` instead of constant value - `self.alpha` changes and resets as per RFC - added test helpers - added test for alpha - made tests pass
omansfeld
force-pushed
the
cubic_alpha_value_change
branch
from
404055e to
d4fc13c
Compare
omansfeld
changed the title
I added our byte-tracking logic to the test and thus made it not depend on #3043 anymore. This can easily be removed again if at some point they are both supposed to land. The description has been updated and the PR is theoretically now in a review-ready state and ready to be picked up again later when we want to get back to the task of de-risking landing it. |
Failed Interop TestsQUIC Interop Runner, client vs. server, differences relative to 7334a18. neqo-latest as client
neqo-latest as server
All resultsSucceeded Interop TestsQUIC Interop Runner, client vs. server neqo-latest as client
neqo-latest as server
Unsupported Interop TestsQUIC Interop Runner, client vs. server neqo-latest as client
neqo-latest as server
|
Client/server transfer resultsPerformance differences relative to e94d8c6. Transfer of 33554432 bytes over loopback, min. 100 runs. All unit-less numbers are in milliseconds.
Download data for |
Benchmark resultsPerformance differences relative to e94d8c6. 1-conn/1-100mb-resp/mtu-1504 (aka. Download)/client: No change in performance detected. time: [192.98 ms 193.38 ms 193.88 ms]
thrpt: [515.79 MiB/s 517.12 MiB/s 518.18 MiB/s]
change:
time: [−0.1993% +0.0432% +0.3243%] (p = 0.76 > 0.05)
thrpt: [−0.3233% −0.0432% +0.1997%]
1-conn/10_000-parallel-1b-resp/mtu-1504 (aka. RPS)/client: No change in performance detected. time: [285.38 ms 287.06 ms 288.72 ms]
thrpt: [34.635 Kelem/s 34.836 Kelem/s 35.041 Kelem/s]
change:
time: [−1.2028% −0.3932% +0.4380%] (p = 0.36 > 0.05)
thrpt: [−0.4361% +0.3947% +1.2174%]
1-conn/1-1b-resp/mtu-1504 (aka. HPS)/client: No change in performance detected. time: [28.317 ms 28.423 ms 28.544 ms]
thrpt: [35.034 B/s 35.183 B/s 35.314 B/s]
change:
time: [−0.4902% +0.0046% +0.5130%] (p = 0.99 > 0.05)
thrpt: [−0.5104% −0.0046% +0.4926%]
1-conn/1-100mb-req/mtu-1504 (aka. Upload)/client: Change within noise threshold. time: [198.20 ms 198.41 ms 198.63 ms]
thrpt: [503.44 MiB/s 504.00 MiB/s 504.54 MiB/s]
change:
time: [−0.6511% −0.3911% −0.1689%] (p = 0.00 < 0.05)
thrpt: [+0.1692% +0.3926% +0.6553%]
decode 4096 bytes, mask ff: No change in performance detected. time: [11.593 µs 11.632 µs 11.676 µs]
change: [−0.3928% −0.0382% +0.3103%] (p = 0.84 > 0.05)
decode 1048576 bytes, mask ff: No change in performance detected. time: [3.0219 ms 3.0314 ms 3.0425 ms]
change: [−0.7213% −0.1190% +0.4766%] (p = 0.70 > 0.05)
decode 4096 bytes, mask 7f: No change in performance detected. time: [19.931 µs 19.975 µs 20.026 µs]
change: [−0.3069% +0.0605% +0.4111%] (p = 0.75 > 0.05)
decode 1048576 bytes, mask 7f: Change within noise threshold. time: [5.0427 ms 5.0536 ms 5.0653 ms]
change: [−1.2209% −0.6402% −0.1484%] (p = 0.02 < 0.05)
decode 4096 bytes, mask 3f: No change in performance detected. time: [8.2551 µs 8.2822 µs 8.3164 µs]
change: [−1.3890% −0.6119% +0.0921%] (p = 0.12 > 0.05)
decode 1048576 bytes, mask 3f: No change in performance detected. time: [1.5892 ms 1.5962 ms 1.6046 ms]
change: [−0.5329% +0.1607% +0.7735%] (p = 0.67 > 0.05)
1-streams/each-1000-bytes/wallclock-time: Change within noise threshold. time: [580.57 µs 581.50 µs 582.65 µs]
change: [−1.8494% −1.2910% −0.7572%] (p = 0.00 < 0.05)
1000-streams/each-1-bytes/wallclock-time: Change within noise threshold. time: [13.640 ms 13.657 ms 13.675 ms]
change: [+0.8522% +1.0673% +1.2766%] (p = 0.00 < 0.05)
1000-streams/each-1-bytes/simulated-time: No change in performance detected. time: [14.994 s 15.009 s 15.025 s]
thrpt: [66.557 B/s 66.625 B/s 66.691 B/s]
change:
time: [−0.1418% −0.0068% +0.1358%] (p = 0.92 > 0.05)
thrpt: [−0.1356% +0.0068% +0.1420%]
1000-streams/each-1000-bytes/wallclock-time: 💔 Performance has regressed. time: [49.262 ms 49.416 ms 49.569 ms]
change: [+1.4701% +1.9490% +2.4264%] (p = 0.00 < 0.05)
coalesce_acked_from_zero 1+1 entries: No change in performance detected. time: [88.589 ns 88.949 ns 89.296 ns]
change: [−0.1860% +0.2697% +0.7209%] (p = 0.25 > 0.05)
coalesce_acked_from_zero 3+1 entries: No change in performance detected. time: [106.67 ns 107.14 ns 107.72 ns]
change: [−0.9975% −0.2920% +0.2987%] (p = 0.42 > 0.05)
coalesce_acked_from_zero 10+1 entries: No change in performance detected. time: [105.95 ns 106.25 ns 106.67 ns]
change: [−1.7550% −0.1445% +1.1194%] (p = 0.87 > 0.05)
coalesce_acked_from_zero 1000+1 entries: No change in performance detected. time: [89.689 ns 89.957 ns 90.399 ns]
change: [−0.8266% +0.0430% +0.8241%] (p = 0.92 > 0.05)
RxStreamOrderer::inbound_frame(): 💚 Performance has improved. time: [107.48 ms 107.55 ms 107.62 ms]
change: [−1.8247% −1.6587% −1.5106%] (p = 0.00 < 0.05)
sent::Packets::take_ranges: No change in performance detected. time: [4.5052 µs 4.5920 µs 4.6751 µs]
change: [−3.2382% −0.6077% +2.0519%] (p = 0.66 > 0.05)
transfer/pacing-false/varying-seeds/wallclock-time/run: Change within noise threshold. time: [25.177 ms 25.228 ms 25.291 ms]
change: [−1.4225% −1.1537% −0.8688%] (p = 0.00 < 0.05)
transfer/pacing-false/varying-seeds/simulated-time/run: 💔 Performance has regressed. time: [25.512 s 25.550 s 25.589 s]
thrpt: [160.07 KiB/s 160.31 KiB/s 160.55 KiB/s]
change:
time: [+1.3518% +1.5482% +1.7551%] (p = 0.00 < 0.05)
thrpt: [−1.7248% −1.5246% −1.3337%]
transfer/pacing-true/varying-seeds/wallclock-time/run: No change in performance detected. time: [25.800 ms 25.860 ms 25.921 ms]
change: [−0.5627% −0.2698% +0.0305%] (p = 0.09 > 0.05)
transfer/pacing-true/varying-seeds/simulated-time/run: Change within noise threshold. time: [25.225 s 25.269 s 25.311 s]
thrpt: [161.82 KiB/s 162.10 KiB/s 162.38 KiB/s]
change:
time: [+0.9340% +1.1549% +1.3701%] (p = 0.00 < 0.05)
thrpt: [−1.3516% −1.1417% −0.9254%]
transfer/pacing-false/same-seed/wallclock-time/run: Change within noise threshold. time: [25.355 ms 25.383 ms 25.412 ms]
change: [−0.9983% −0.8127% −0.6499%] (p = 0.00 < 0.05)
transfer/pacing-false/same-seed/simulated-time/run: Change within noise threshold. time: [25.510 s 25.510 s 25.510 s]
thrpt: [160.56 KiB/s 160.56 KiB/s 160.56 KiB/s]
change:
time: [−0.7766% −0.7766% −0.7766%] (p = 0.00 < 0.05)
thrpt: [+0.7827% +0.7827% +0.7827%]
transfer/pacing-true/same-seed/wallclock-time/run: Change within noise threshold. time: [26.047 ms 26.079 ms 26.125 ms]
change: [−2.5826% −2.4172% −2.2540%] (p = 0.00 < 0.05)
transfer/pacing-true/same-seed/simulated-time/run: 💔 Performance has regressed. time: [25.964 s 25.964 s 25.964 s]
thrpt: [157.76 KiB/s 157.76 KiB/s 157.76 KiB/s]
change:
time: [+1.1249% +1.1249% +1.1249%] (p = 0.00 < 0.05)
thrpt: [−1.1124% −1.1124% −1.1124%]
Download data for |
2 participants
This implements changing CUBIC_ALPHA to
1.0whenw_est >= cwnd_prioras per https://datatracker.ietf.org/doc/html/rfc9438#section-4.3-11.self.alphafieldself.cwnd_priorfieldself.alphainstead of constant valueself.alphachanges and resets as per RFCWe had decided that this is one of the CUBIC RFC9438 parts where we aren't as confident to land it right now and will defer de-risking to later. Thus this PR stays a draft.
It is in a review-ready state though and can easily be picked back up when time comes.
See https://ntap.github.io/rfc8312bis/issues.html#n(2) for some context and discussions on why this was added to the spec. In there it was said that this will also be added to the Linux TCP CUBIC implementation, but as far as I can see that was never done (link to Linux Reno-friendly region)
Part of #3053