feat: DPLPMTUD #1903
feat: DPLPMTUD #1903
Conversation
Failed Interop TestsQUIC Interop Runner, client vs. server
All resultsSucceeded Interop TestsQUIC Interop Runner, client vs. server
Unsupported Interop TestsQUIC Interop Runner, client vs. server
|
As far as I can tell the trait function Does that make sense @larseggert? |
Benchmark resultsPerformance differences relative to 7d610ed. coalesce_acked_from_zero 1+1 entries: Change within noise threshold. time: [193.61 ns 194.06 ns 194.53 ns]
change: [+0.0719% +0.4048% +0.7829%] (p = 0.02 < 0.05)
coalesce_acked_from_zero 3+1 entries: No change in performance detected. time: [234.67 ns 235.73 ns 237.08 ns]
change: [+0.6006% +2.3382% +6.2565%] (p = 0.06 > 0.05)
coalesce_acked_from_zero 10+1 entries: No change in performance detected. time: [233.27 ns 234.03 ns 234.96 ns]
change: [-0.1847% +0.3385% +0.8310%] (p = 0.21 > 0.05)
coalesce_acked_from_zero 1000+1 entries: No change in performance detected. time: [215.28 ns 217.58 ns 222.98 ns]
change: [-0.5398% +6.4762% +20.010%] (p = 0.44 > 0.05)
RxStreamOrderer::inbound_frame(): 💔 Performance has regressed. time: [120.23 ms 120.29 ms 120.37 ms]
change: [+1.1136% +1.1966% +1.2758%] (p = 0.00 < 0.05)
transfer/Run multiple transfers with varying seeds: 💚 Performance has improved. time: [55.737 ms 59.125 ms 62.506 ms]
thrpt: [63.994 MiB/s 67.653 MiB/s 71.766 MiB/s]
change:
time: [-53.858% -51.136% -48.358%] (p = 0.00 < 0.05)
thrpt: [+93.642% +104.65% +116.72%]
transfer/Run multiple transfers with the same seed: 💚 Performance has improved. time: [65.695 ms 70.817 ms 75.893 ms]
thrpt: [52.706 MiB/s 56.484 MiB/s 60.887 MiB/s]
change:
time: [-45.571% -41.490% -37.459%] (p = 0.00 < 0.05)
thrpt: [+59.896% +70.912% +83.726%]
1-conn/1-100mb-resp (aka. Download)/client: 💚 Performance has improved. time: [154.81 ms 160.68 ms 166.89 ms]
thrpt: [599.19 MiB/s 622.35 MiB/s 645.97 MiB/s]
change:
time: [-86.471% -85.914% -85.283%] (p = 0.00 < 0.05)
thrpt: [+579.47% +609.92% +639.15%]
1-conn/10_000-parallel-1b-resp (aka. RPS)/client: Change within noise threshold. time: [430.16 ms 433.60 ms 437.05 ms]
thrpt: [22.880 Kelem/s 23.063 Kelem/s 23.247 Kelem/s]
change:
time: [-2.5302% -1.4809% -0.4847%] (p = 0.00 < 0.05)
thrpt: [+0.4870% +1.5031% +2.5959%]
1-conn/1-1b-resp (aka. HPS)/client: 💚 Performance has improved. time: [43.569 ms 44.108 ms 44.652 ms]
thrpt: [22.395 elem/s 22.671 elem/s 22.952 elem/s]
change:
time: [-3.7644% -2.5274% -1.2515%] (p = 0.00 < 0.05)
thrpt: [+1.2674% +2.5930% +3.9116%]
Client/server transfer resultsTransfer of 33554432 bytes over loopback.
|
There was a problem hiding this comment.
I'm not seeing PMTUD tests, which would be necessary for this.
The big question I have is the one that Christian makes about PTMUD generally: how do you know that the bytes you use on PMTUD pay you back?
There is probably a case for sending probes when you have spare sending capacity and nothing better to send. Indeed, successfully probing will let us push congestion windows up more and could even improve performance.
What I'm seeing here displaces other data. I'd like to see something that doesn't do that. There's a fundamental problem that needs analysis though. You can't predict that a connection will be used for uploads, so you don't know when probes will really help. I see a few cases:
- The connection is short-lived or low volume. Probes are strictly wasteful.
- The connection is long-lived and high volume, with ample idle time for probing. Probes can use gaps. This might be a video stream, where probing can fit into a warmup period. Probes are therefore easy and super-helpful.
- The connection exists only to support a smaller upload. The upload is small enough that probes are wasteful.
- The connection exists only to support a larger upload. The upload is large enough that spending bytes on probing early on is a good investment.
Case 1 and 2 are easy to deal with. We could probe on an idle connection and tolerate a small amount of waste for case 1 if it makes case 2 appreciably better.
The split between 3 and 4 is rough. There is an uncertain zone between the two as well where some probing is justified, but successive rounds of probing might be wasteful as the throughput gain over the remaining time diminishes relative to the wasted effort of extra probes.
Right now, you don't send real data in probes. You are effectively betting on the probes being lost. But you could send data, which would reduce the harm in case 3. It might even make the code slightly simpler.
Co-authored-by: Martin Thomson <mt@lowentropy.net> Signed-off-by: Lars Eggert <lars@eggert.org>
Co-authored-by: Max Inden <mail@max-inden.de> Signed-off-by: Lars Eggert <lars@eggert.org>
Co-authored-by: Max Inden <mail@max-inden.de> Signed-off-by: Lars Eggert <lars@eggert.org>
Co-authored-by: Max Inden <mail@max-inden.de> Signed-off-by: Lars Eggert <lars@eggert.org>
Co-authored-by: Max Inden <mail@max-inden.de> Signed-off-by: Lars Eggert <lars@eggert.org>
Co-authored-by: Max Inden <mail@max-inden.de> Signed-off-by: Lars Eggert <lars@eggert.org>
Co-authored-by: Martin Thomson <mt@lowentropy.net> Signed-off-by: Lars Eggert <lars@eggert.org>
Follow-up on mozilla#1903 (comment)
Follow-up on mozilla#1903 (comment)
Follow-up on mozilla#1903 (comment)
Follow-up on mozilla#1903 (comment)
`Probe` is a small simple enum on the stack, thus convention is to implement `Copy` instead of only `Clone` with a call to `clone()`. The following helped me in the past: > When should my type be Copy? > > Generally speaking, if your type can implement Copy, it should. Keep in mind, > though, that implementing Copy is part of the public API of your type. If the > type might become non-Copy in the future, it could be prudent to omit the Copy > implementation now, to avoid a breaking API change. https://doc.rust-lang.org/std/marker/trait.Copy.html#when-should-my-type-be-copy
This implements a simplified variant of PLDPMTUD (aka RFC8899), which by default probes for increasingly larger PMTUs using an MTU table.
There is currently no attempt to repeat the PMTUD at intervals.There is also no attempt to detect PMTUs that are in between values in the table.There is no attempt to handle the case where the PMTU shrinks.A lot of the existing tests (~50%) break when PMTUD is enabled, so this PR disables it by default. New tests that cover PMTUD were added to this PR.
Fixes #243