From 2ba600bf5498015d8f22dc9155afef65e3c5fe13 Mon Sep 17 00:00:00 2001 From: recatek Date: Tue, 18 Feb 2025 18:41:29 -0500 Subject: [PATCH] Eliminating an Instant::now() in BBR bandwidth estimation code This was the only case where `Instant::now()` is used in quinn-proto or quinn-udp for anything other than tests or rate-limiting logging. Making this change to keep these two crates more IO-agnostic. --- .../src/congestion/bbr/bw_estimation.rs | 38 +++++-------------- 1 file changed, 9 insertions(+), 29 deletions(-) diff --git a/quinn-proto/src/congestion/bbr/bw_estimation.rs b/quinn-proto/src/congestion/bbr/bw_estimation.rs index 8796f4572..84ea4e687 100644 --- a/quinn-proto/src/congestion/bbr/bw_estimation.rs +++ b/quinn-proto/src/congestion/bbr/bw_estimation.rs @@ -3,7 +3,7 @@ use std::fmt::{Debug, Display, Formatter}; use super::min_max::MinMax; use crate::{Duration, Instant}; -#[derive(Clone, Debug)] +#[derive(Clone, Debug, Default)] pub(crate) struct BandwidthEstimation { total_acked: u64, prev_total_acked: u64, @@ -11,7 +11,7 @@ pub(crate) struct BandwidthEstimation { prev_acked_time: Option, total_sent: u64, prev_total_sent: u64, - sent_time: Instant, + sent_time: Option, prev_sent_time: Option, max_filter: MinMax, acked_at_last_window: u64, @@ -21,8 +21,8 @@ impl BandwidthEstimation { pub(crate) fn on_sent(&mut self, now: Instant, bytes: u64) { self.prev_total_sent = self.total_sent; self.total_sent += bytes; - self.prev_sent_time = Some(self.sent_time); - self.sent_time = now; + self.prev_sent_time = self.sent_time; + self.sent_time = Some(now); } pub(crate) fn on_ack( @@ -43,14 +43,13 @@ impl BandwidthEstimation { None => return, }; - let send_rate = if self.sent_time > prev_sent_time { - Self::bw_from_delta( + let send_rate = match self.sent_time { + Some(sent_time) if sent_time > prev_sent_time => Self::bw_from_delta( self.total_sent - self.prev_total_sent, - self.sent_time - prev_sent_time, + sent_time - prev_sent_time, ) - .unwrap_or(0) - } else { - u64::MAX // will take the min of send and ack, so this is just a skip + .unwrap_or(0), + _ => u64::MAX, // will take the min of send and ack, so this is just a skip }; let ack_rate = match self.prev_acked_time { @@ -91,25 +90,6 @@ impl BandwidthEstimation { } } -impl Default for BandwidthEstimation { - fn default() -> Self { - Self { - total_acked: 0, - prev_total_acked: 0, - acked_time: None, - prev_acked_time: None, - total_sent: 0, - prev_total_sent: 0, - // The `sent_time` value set here is ignored; it is used in `on_ack()`, but will - // have been reset by `on_sent()` before that method is called. - sent_time: Instant::now(), - prev_sent_time: None, - max_filter: MinMax::default(), - acked_at_last_window: 0, - } - } -} - impl Display for BandwidthEstimation { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { write!(