Skip to content

Commit 4cd06bf

Browse files
authored
perf(http2): slow adaptive window pings as the BDP stabilizes (#2550)
This introduces a delay to sending a ping to calculate the BDP that becomes shorter as the BDP is changing, to improve throughput quickly, but then also becomes longer as the BDP stabilizes, to reduce the amount of pings sent. This improved the performance of the adaptive window end_to_end benchmark. It should also reduce the amount of pings the remote has to deal with, hopefully preventing hyper from triggering ENHANCE_YOUR_CALM errors.
1 parent 960a69a commit 4cd06bf

File tree

1 file changed

+60
-22
lines changed

1 file changed

+60
-22
lines changed

src/proto/h2/ping.rs

+60-22
Original file line numberDiff line numberDiff line change
@@ -51,9 +51,15 @@ pub(super) fn channel(ping_pong: PingPong, config: Config) -> (Recorder, Ponger)
5151
bdp: wnd,
5252
max_bandwidth: 0.0,
5353
rtt: 0.0,
54+
ping_delay: Duration::from_millis(100),
55+
stable_count: 0,
5456
});
5557

56-
let bytes = bdp.as_ref().map(|_| 0);
58+
let (bytes, next_bdp_at) = if bdp.is_some() {
59+
(Some(0), Some(Instant::now()))
60+
} else {
61+
(None, None)
62+
};
5763

5864
#[cfg(feature = "runtime")]
5965
let keep_alive = config.keep_alive_interval.map(|interval| KeepAlive {
@@ -75,6 +81,7 @@ pub(super) fn channel(ping_pong: PingPong, config: Config) -> (Recorder, Ponger)
7581
is_keep_alive_timed_out: false,
7682
ping_pong,
7783
ping_sent_at: None,
84+
next_bdp_at,
7885
}));
7986

8087
(
@@ -125,6 +132,9 @@ struct Shared {
125132
/// If `Some`, bdp is enabled, and this tracks how many bytes have been
126133
/// read during the current sample.
127134
bytes: Option<usize>,
135+
/// We delay a variable amount of time between BDP pings. This allows us
136+
/// to send less pings as the bandwidth stabilizes.
137+
next_bdp_at: Option<Instant>,
128138

129139
// keep-alive
130140
/// If `Some`, keep-alive is enabled, and the Instant is how long ago
@@ -143,6 +153,12 @@ struct Bdp {
143153
max_bandwidth: f64,
144154
/// Round trip time in seconds
145155
rtt: f64,
156+
/// Delay the next ping by this amount.
157+
///
158+
/// This will change depending on how stable the current bandwidth is.
159+
ping_delay: Duration,
160+
/// The count of ping round trips where BDP has stayed the same.
161+
stable_count: u32,
146162
}
147163

148164
#[cfg(feature = "runtime")]
@@ -207,6 +223,17 @@ impl Recorder {
207223
#[cfg(feature = "runtime")]
208224
locked.update_last_read_at();
209225

226+
// are we ready to send another bdp ping?
227+
// if not, we don't need to record bytes either
228+
229+
if let Some(ref next_bdp_at) = locked.next_bdp_at {
230+
if Instant::now() < *next_bdp_at {
231+
return;
232+
} else {
233+
locked.next_bdp_at = None;
234+
}
235+
}
236+
210237
if let Some(ref mut bytes) = locked.bytes {
211238
*bytes += len;
212239
} else {
@@ -265,6 +292,7 @@ impl Recorder {
265292

266293
impl Ponger {
267294
pub(super) fn poll(&mut self, cx: &mut task::Context<'_>) -> Poll<Ponged> {
295+
let now = Instant::now();
268296
let mut locked = self.shared.lock().unwrap();
269297
#[cfg(feature = "runtime")]
270298
let is_idle = self.is_idle();
@@ -282,13 +310,13 @@ impl Ponger {
282310
return Poll::Pending;
283311
}
284312

285-
let (bytes, rtt) = match locked.ping_pong.poll_pong(cx) {
313+
match locked.ping_pong.poll_pong(cx) {
286314
Poll::Ready(Ok(_pong)) => {
287-
let rtt = locked
315+
let start = locked
288316
.ping_sent_at
289-
.expect("pong received implies ping_sent_at")
290-
.elapsed();
317+
.expect("pong received implies ping_sent_at");
291318
locked.ping_sent_at = None;
319+
let rtt = now - start;
292320
trace!("recv pong");
293321

294322
#[cfg(feature = "runtime")]
@@ -299,19 +327,20 @@ impl Ponger {
299327
}
300328
}
301329

302-
if self.bdp.is_some() {
330+
if let Some(ref mut bdp) = self.bdp {
303331
let bytes = locked.bytes.expect("bdp enabled implies bytes");
304332
locked.bytes = Some(0); // reset
305333
trace!("received BDP ack; bytes = {}, rtt = {:?}", bytes, rtt);
306-
(bytes, rtt)
307-
} else {
308-
// no bdp, done!
309-
return Poll::Pending;
334+
335+
let update = bdp.calculate(bytes, rtt);
336+
locked.next_bdp_at = Some(now + bdp.ping_delay);
337+
if let Some(update) = update {
338+
return Poll::Ready(Ponged::SizeUpdate(update))
339+
}
310340
}
311341
}
312342
Poll::Ready(Err(e)) => {
313343
debug!("pong error: {}", e);
314-
return Poll::Pending;
315344
}
316345
Poll::Pending => {
317346
#[cfg(feature = "runtime")]
@@ -324,19 +353,11 @@ impl Ponger {
324353
}
325354
}
326355
}
327-
328-
return Poll::Pending;
329356
}
330-
};
331-
332-
drop(locked);
333-
334-
if let Some(bdp) = self.bdp.as_mut().and_then(|bdp| bdp.calculate(bytes, rtt)) {
335-
Poll::Ready(Ponged::SizeUpdate(bdp))
336-
} else {
337-
// XXX: this doesn't register a waker...?
338-
Poll::Pending
339357
}
358+
359+
// XXX: this doesn't register a waker...?
360+
Poll::Pending
340361
}
341362

342363
#[cfg(feature = "runtime")]
@@ -386,6 +407,7 @@ impl Bdp {
386407
fn calculate(&mut self, bytes: usize, rtt: Duration) -> Option<WindowSize> {
387408
// No need to do any math if we're at the limit.
388409
if self.bdp as usize == BDP_LIMIT {
410+
self.stabilize_delay();
389411
return None;
390412
}
391413

@@ -405,6 +427,7 @@ impl Bdp {
405427

406428
if bw < self.max_bandwidth {
407429
// not a faster bandwidth, so don't update
430+
self.stabilize_delay();
408431
return None;
409432
} else {
410433
self.max_bandwidth = bw;
@@ -415,11 +438,26 @@ impl Bdp {
415438
if bytes >= self.bdp as usize * 2 / 3 {
416439
self.bdp = (bytes * 2).min(BDP_LIMIT) as WindowSize;
417440
trace!("BDP increased to {}", self.bdp);
441+
442+
self.stable_count = 0;
443+
self.ping_delay /= 2;
418444
Some(self.bdp)
419445
} else {
446+
self.stabilize_delay();
420447
None
421448
}
422449
}
450+
451+
fn stabilize_delay(&mut self) {
452+
if self.ping_delay < Duration::from_secs(10) {
453+
self.stable_count += 1;
454+
455+
if self.stable_count >= 2 {
456+
self.ping_delay *= 4;
457+
self.stable_count = 0;
458+
}
459+
}
460+
}
423461
}
424462

425463
fn seconds(dur: Duration) -> f64 {

0 commit comments

Comments
 (0)