-
Notifications
You must be signed in to change notification settings - Fork 126
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
perf(transport): auto-tune stream receive window #1868
base: main
Are you sure you want to change the base?
Conversation
This commit adds a basic smoke test using the `test-ficture` simulator, asserting that on a connection with unlimited bandwidth and 50ms round-trip-time Neqo can eventually achieve > 1 Gbit/s throughput. Showcases the potential a future stream flow-control auto-tuning algorithm can have. See mozilla#733.
Previously the stream send and receive window had a hard limit at 1MB. On high latency and/or high bandwidth connections, 1 MB is not enough to exhaust the available bandwidth. Sample scenario: ``` delay_s = 0.05 window_bits = 1 * 1024 * 1024 * 8 bandwidth_bits_s = window_bits / delay_s bandwidth_mbits_s = bandwidth_bits_s / 1024 / 1024 # 160.0 ``` In other words, on a 50 ms connection a 1 MB window can at most achieve 160 Mbit/s. This commit introduces an auto-tuning algorithm for the stream receive window, increasing the window towards the bandwidth-delay product of the connection.
Failed Interop TestsQUIC Interop Runner, client vs. server, differences relative to b6e4cfc. 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
|
This commit adds a basic smoke test using the `test-fixture` simulator, asserting the expected bandwidth on a 1 gbit link. Given mozilla#733, the current expected bandwidth is limited by the fixed sized stream receive buffer (1MiB).
A `Node` (e.g. a `Client`, `Server` or `TailDrop` router) can be in 3 states: ``` rust enum NodeState { /// The node just produced a datagram. It should be activated again as soon as possible. Active, /// The node is waiting. Waiting(Instant), /// The node became idle. Idle, } ``` `NodeHolder::ready()` determines whether a `Node` is ready to be processed again. When `NodeState::Waiting`, it should only be ready when `t <= now`, i.e. the waiting time has passed, not `t >= now`. ``` rust impl NodeHolder { fn ready(&self, now: Instant) -> bool { match self.state { Active => true, Waiting(t) => t <= now, // not >= Idle => false, } } } ``` The previous behavior lead to wastefull non-ready `Node`s being processed and thus a large test runtime when e.g. simulating a gbit connection (mozilla#2203).
neqo-transport/src/fc.rs
Outdated
// Auto-tune max_active, i.e. the flow control window. | ||
// | ||
// If the sending rate ( window_bytes used / elapsed ) exceeds the rate | ||
// allowed by the maximum flow control window and the current rtt ( | ||
// max_active / rtt ), try to increase the maximum flow control window ( | ||
// max_active ). | ||
if let Some(max_allowed_sent_at) = self.max_allowed_sent_at { | ||
let elapsed = now.duration_since(max_allowed_sent_at); | ||
let window_bytes_used = self.max_active - (self.max_allowed - self.retired); | ||
|
||
// Same as `elapsed / rtt < window_bytes_used / max_active` | ||
// without floating point division. | ||
if elapsed.as_micros() * u128::from(self.max_active) | ||
< rtt.as_micros() * u128::from(window_bytes_used) | ||
{ | ||
let prev_max_active = self.max_active; | ||
// Try doubling the flow control window. | ||
// | ||
// Note that the flow control window should grow at least as | ||
// fast as the congestion control window, in order to not | ||
// unnecessarily limit throughput. | ||
self.max_active = min(2 * self.max_active, MAX_RECV_WINDOW_SIZE); | ||
qdebug!( | ||
"Increasing max stream receive window: previous max_active: {} MiB new max_active: {} MiB last update: {:?} rtt: {rtt:?} stream_id: {}", | ||
prev_max_active / 1024 / 1024, self.max_active / 1024 / 1024, now-self.max_allowed_sent_at.unwrap(), self.subject, | ||
); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that this is not the exact algorithm suggested by @martinthomson in #733 (comment).
The algorithm proposed in this pull request adopts Martin's trigger mechanism, namely to increase the window based on the perceived BDP.
Therefore, I suggest that if the rate at which self.retired increases (that is, the change in that value, divided by the time elapsed) exceeds some function of self.max_active / path.rtt,
It does not adopt the increase mechanism, i.e. to increase by the amount of retired data. Instead, the window is simply doubled.
then we can increase self.max_active by the amount that self.retired has increased.
The rational is documented above.
// Try doubling the flow control window.
//
// Note that the flow control window should grow at least as
// fast as the congestion control window, in order to not
// unnecessarily limit throughput.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason I didn't recommend doubling is that our congestion control algorithms will increase the window at a rate greater than double at times. Then we'll lag here if the peer uses one of those. They might be tripling or more when they are at the steep part of a Cubic ramp. We'll be giving them less than that.
The other problem I see with this is that it doubles when the other side is close to matching the BDP. Fluctuations in rate (or RTT estimate) will cause the condition to be tripped (retiring 1/4 of the window in ~1/4 of the RTT is expected). Whereas retiring 1/2 of the data in 1/4 of an RTT is cause to increase much more.
On the other hand, I see how using the newly retired amount -- as I suggested -- doesn't do better than double. Perhaps this:
let bonus_multiplier = 4; // same as number of updates per RTT 🤔
let proportion_of_rtt = (now - last_update) / rtt;
let excess = window_bytes_used - (max_active * proportion_of_rtt);
if excess > 0 {
self.max_active += bonus_multiplier * excess;
}
(Obviously, this is sloppy. Better code would use integer math and check for overflow and whatnot.)
For instance, if they blow through the entire window in 1/4 of an RTT, then this would add 4*3/4 of the the window, quadrupling it. There is no upper limit to how fast we can increase here if the peer sends faster (successfully). As our estimate becomes closer to approximately correct for the BDP, we'd correct less. If we're short by 10 bytes, then it would add just 40. As the RTT fluctuates, I expect that this would happen a couple of times before it settles just above the BDP.
For my next trick, I will also suggest that the per-stream limit look at the connection-level estimate (which should track BDP better) so that we start each stream at a decent fraction of that (which could be 1, but I'd suggest 1/4).
@martinthomson can you give this pull request a review? Not urgent, but I believe important, as this can have a significant impact on our up- and download throughput on high-bandwidth-delay connections. |
Friendly ping @martinthomson. Or maybe @larseggert, do you have some spare cycles to review this pull request? |
Signed-off-by: Lars Eggert <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Preliminary comments, with updated thoughts on the algorithm.
) { | ||
if !self.frame_needed() { | ||
return; | ||
} | ||
|
||
// Auto-tune max_active, i.e. the flow control window. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be in a new function. Then you can document it more cleanly and use early returns to make it neater.
@@ -363,13 +418,14 @@ impl ReceiverFlowControl<StreamId> { | |||
max_data: max_allowed, | |||
})); | |||
self.frame_sent(max_allowed); | |||
self.max_allowed_sent_at = Some(now); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The name of this is a bit strange. It's just the time at which you last sent a frame. Would last_update
make more sense?
@@ -255,6 +273,11 @@ where | |||
} | |||
} | |||
|
|||
const fn should_send_flowc_update(&self) -> bool { | |||
let window_bytes_unused = self.max_allowed.saturating_sub(self.retired); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it ever the case that max_allowed < retired
such that you need saturating_sub()
?
neqo-transport/src/fc.rs
Outdated
// Auto-tune max_active, i.e. the flow control window. | ||
// | ||
// If the sending rate ( window_bytes used / elapsed ) exceeds the rate | ||
// allowed by the maximum flow control window and the current rtt ( | ||
// max_active / rtt ), try to increase the maximum flow control window ( | ||
// max_active ). | ||
if let Some(max_allowed_sent_at) = self.max_allowed_sent_at { | ||
let elapsed = now.duration_since(max_allowed_sent_at); | ||
let window_bytes_used = self.max_active - (self.max_allowed - self.retired); | ||
|
||
// Same as `elapsed / rtt < window_bytes_used / max_active` | ||
// without floating point division. | ||
if elapsed.as_micros() * u128::from(self.max_active) | ||
< rtt.as_micros() * u128::from(window_bytes_used) | ||
{ | ||
let prev_max_active = self.max_active; | ||
// Try doubling the flow control window. | ||
// | ||
// Note that the flow control window should grow at least as | ||
// fast as the congestion control window, in order to not | ||
// unnecessarily limit throughput. | ||
self.max_active = min(2 * self.max_active, MAX_RECV_WINDOW_SIZE); | ||
qdebug!( | ||
"Increasing max stream receive window: previous max_active: {} MiB new max_active: {} MiB last update: {:?} rtt: {rtt:?} stream_id: {}", | ||
prev_max_active / 1024 / 1024, self.max_active / 1024 / 1024, now-self.max_allowed_sent_at.unwrap(), self.subject, | ||
); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason I didn't recommend doubling is that our congestion control algorithms will increase the window at a rate greater than double at times. Then we'll lag here if the peer uses one of those. They might be tripling or more when they are at the steep part of a Cubic ramp. We'll be giving them less than that.
The other problem I see with this is that it doubles when the other side is close to matching the BDP. Fluctuations in rate (or RTT estimate) will cause the condition to be tripped (retiring 1/4 of the window in ~1/4 of the RTT is expected). Whereas retiring 1/2 of the data in 1/4 of an RTT is cause to increase much more.
On the other hand, I see how using the newly retired amount -- as I suggested -- doesn't do better than double. Perhaps this:
let bonus_multiplier = 4; // same as number of updates per RTT 🤔
let proportion_of_rtt = (now - last_update) / rtt;
let excess = window_bytes_used - (max_active * proportion_of_rtt);
if excess > 0 {
self.max_active += bonus_multiplier * excess;
}
(Obviously, this is sloppy. Better code would use integer math and check for overflow and whatnot.)
For instance, if they blow through the entire window in 1/4 of an RTT, then this would add 4*3/4 of the the window, quadrupling it. There is no upper limit to how fast we can increase here if the peer sends faster (successfully). As our estimate becomes closer to approximately correct for the BDP, we'd correct less. If we're short by 10 bytes, then it would add just 40. As the RTT fluctuates, I expect that this would happen a couple of times before it settles just above the BDP.
For my next trick, I will also suggest that the per-stream limit look at the connection-level estimate (which should track BDP better) so that we start each stream at a decent fraction of that (which could be 1, but I'd suggest 1/4).
@@ -255,6 +273,11 @@ where | |||
} | |||
} | |||
|
|||
const fn should_send_flowc_update(&self) -> bool { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const fn should_send_flowc_update(&self) -> bool { | |
const fn should_send_update(&self) -> bool { |
The files is fc.rs
and the type is ReceiverFlowControl
@@ -494,10 +494,10 @@ impl TxBuffer { | |||
|
|||
/// Attempt to add some or all of the passed-in buffer to the `TxBuffer`. | |||
pub fn send(&mut self, buf: &[u8]) -> usize { | |||
let can_buffer = min(SEND_BUFFER_SIZE - self.buffered(), buf.len()); | |||
let can_buffer = min(MAX_SEND_BUFFER_SIZE - self.buffered(), buf.len()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm OK with that. As you say, it either gets discarded when the short-lived stream goes away, or it is available for the next write on a long-lived stream.
Was there a particular reason you increased this here? Was it necessary to exercise the increasing receive buffer size.
if elapsed.as_micros() * u128::from(self.max_active) | ||
< rtt.as_micros() * u128::from(window_bytes_used) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a way to avoid u128
math? It can be slow on some low-end ARM chips, and if we're executing this in the fast path we might want to avoid.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given that this happens at most 4 times per RTT, I'm OK with this being a tiny bit expensive. If this were on a hot path, that would be different.
That said, it might be OK to make this an approximate thing, with conversion to a double instead. The imprecision of floating point math shouldn't hurt in this case.
Benchmark resultsPerformance differences relative to b6e4cfc. decode 4096 bytes, mask ff: No change in performance detected.time: [10.868 µs 10.908 µs 10.954 µs] change: [-0.3752% +0.0964% +0.5582%] (p = 0.70 > 0.05) decode 1048576 bytes, mask ff: No change in performance detected.time: [3.1279 ms 3.1371 ms 3.1476 ms] change: [-0.3526% +0.0761% +0.4969%] (p = 0.74 > 0.05) decode 4096 bytes, mask 7f: Change within noise threshold.time: [17.606 µs 17.646 µs 17.692 µs] change: [-1.3032% -0.6574% -0.1182%] (p = 0.02 < 0.05) decode 1048576 bytes, mask 7f: No change in performance detected.time: [5.4070 ms 5.4199 ms 5.4343 ms] change: [-0.3472% +0.0150% +0.3539%] (p = 0.93 > 0.05) decode 4096 bytes, mask 3f: No change in performance detected.time: [6.6500 µs 6.6712 µs 6.6953 µs] change: [-0.5912% +0.0781% +0.7357%] (p = 0.82 > 0.05) decode 1048576 bytes, mask 3f: No change in performance detected.time: [1.7580 ms 1.7595 ms 1.7624 ms] change: [-0.0395% +0.0499% +0.2732%] (p = 0.69 > 0.05) coalesce_acked_from_zero 1+1 entries: No change in performance detected.time: [91.025 ns 91.322 ns 91.626 ns] change: [-0.6505% -0.1252% +0.3992%] (p = 0.65 > 0.05) coalesce_acked_from_zero 3+1 entries: Change within noise threshold.time: [109.36 ns 109.69 ns 110.05 ns] change: [+0.1337% +0.6709% +1.5148%] (p = 0.04 < 0.05) coalesce_acked_from_zero 10+1 entries: No change in performance detected.time: [108.78 ns 109.04 ns 109.40 ns] change: [-1.2619% -0.4920% +0.2018%] (p = 0.20 > 0.05) coalesce_acked_from_zero 1000+1 entries: No change in performance detected.time: [93.215 ns 93.360 ns 93.528 ns] change: [-0.8111% +0.1881% +1.1366%] (p = 0.73 > 0.05) RxStreamOrderer::inbound_frame(): No change in performance detected.time: [111.33 ms 111.38 ms 111.43 ms] change: [-0.0329% +0.0337% +0.1001%] (p = 0.32 > 0.05) SentPackets::take_ranges: No change in performance detected.time: [5.1663 µs 5.2420 µs 5.3122 µs] change: [-17.833% -5.9009% +2.6758%] (p = 0.49 > 0.05) transfer/pacing-false/varying-seeds: Change within noise threshold.time: [38.022 ms 38.085 ms 38.149 ms] change: [+2.3237% +2.5856% +2.8408%] (p = 0.00 < 0.05) transfer/pacing-true/varying-seeds: Change within noise threshold.time: [37.941 ms 38.001 ms 38.062 ms] change: [+2.2001% +2.4407% +2.6890%] (p = 0.00 < 0.05) transfer/pacing-false/same-seed: Change within noise threshold.time: [37.818 ms 37.879 ms 37.941 ms] change: [+2.1669% +2.4217% +2.6583%] (p = 0.00 < 0.05) transfer/pacing-true/same-seed: Change within noise threshold.time: [38.164 ms 38.213 ms 38.262 ms] change: [+2.8103% +3.0164% +3.2258%] (p = 0.00 < 0.05) 1-conn/1-100mb-resp/mtu-1504 (aka. Download)/client: No change in performance detected.time: [856.82 ms 867.67 ms 878.75 ms] thrpt: [113.80 MiB/s 115.25 MiB/s 116.71 MiB/s] change: time: [-2.1945% -0.5510% +1.0584%] (p = 0.52 > 0.05) thrpt: [-1.0474% +0.5541% +2.2437%] 1-conn/10_000-parallel-1b-resp/mtu-1504 (aka. RPS)/client: No change in performance detected.time: [320.32 ms 323.65 ms 326.93 ms] thrpt: [30.587 Kelem/s 30.898 Kelem/s 31.219 Kelem/s] change: time: [-0.6179% +0.9609% +2.6066%] (p = 0.24 > 0.05) thrpt: [-2.5403% -0.9518% +0.6217%] 1-conn/1-1b-resp/mtu-1504 (aka. HPS)/client: No change in performance detected.time: [25.372 ms 25.556 ms 25.747 ms] thrpt: [38.840 elem/s 39.130 elem/s 39.414 elem/s] change: time: [-1.1289% -0.1478% +0.8669%] (p = 0.76 > 0.05) thrpt: [-0.8595% +0.1480% +1.1418%] 1-conn/1-100mb-resp/mtu-1504 (aka. Upload)/client: Change within noise threshold.time: [1.8272 s 1.8459 s 1.8644 s] thrpt: [53.635 MiB/s 54.175 MiB/s 54.729 MiB/s] change: time: [-2.7996% -1.4901% -0.1100%] (p = 0.03 < 0.05) thrpt: [+0.1101% +1.5127% +2.8802%] Client/server transfer resultsPerformance differences relative to b6e4cfc. Transfer of 33554432 bytes over loopback, 30 runs. All unit-less numbers are in milliseconds.
|
Previously the stream send and receive window had a hard limit at 1MB. On high latency and/or high bandwidth connections (i.e. large bandwidth-delay product), 1 MB is not enough to exhaust the available bandwidth.
Sample scenario:
In other words, on a 50 ms connection a 1 MB window can at most achieve 160 Mbit/s.
This commit introduces an auto-tuning algorithm for the stream receive window, increasing the window towards the bandwidth-delay product of the connection.
Fixes #733.