chore: Avoid Box<dyn CongestionControl>

For discussion - is this worth it?

(The extra dep could be avoided if we implement a dispatch macro ourselves.)

Author: larseggert

Cargo.lock

@@ -30,6 +30,7 @@ license = "MIT OR Apache-2.0"
 rust-version = "1.90.0"
 
 [workspace.dependencies]
+enum_dispatch = { version = "0.3", default-features = false }
 enum-map = { version = "2.7", default-features = false }
 enumset = { version = "1.1", default-features = false }
 hex = { version = "0.4", default-features = false }

Cargo.toml

@@ -18,6 +18,7 @@ workspace = true
 
 [dependencies]
 # Checked against https://searchfox.org/mozilla-central/source/Cargo.lock 2024-11-11
+enum_dispatch = { workspace = true }
 enum-map = { workspace = true }
 enumset = { workspace = true }
 indexmap = { version = "2", default-features = false } # See https://github.com/mozilla/neqo/issues/1858

neqo-transport/Cargo.toml

@@ -190,6 +190,11 @@ impl<T> ClassicCongestionControl<T> {
     pub const fn max_datagram_size(&self) -> usize {
         self.pmtud.plpmtu()
     }
+
+    #[cfg(test)]
+    pub const fn cwnd_initial(&self) -> usize {
+        cwnd_initial(self.pmtud.plpmtu())
+    }
 }
 
 impl<T: WindowAdjustment> CongestionControl for ClassicCongestionControl<T> {
@@ -218,11 +223,6 @@ impl<T: WindowAdjustment> CongestionControl for ClassicCongestionControl<T> {
         self.max_datagram_size() * 2
     }
 
-    #[cfg(test)]
-    fn cwnd_initial(&self) -> usize {
-        cwnd_initial(self.pmtud.plpmtu())
-    }
-
     fn pmtud(&self) -> &Pmtud {
         &self.pmtud
     }
@@ -841,7 +841,7 @@ mod tests {
     use crate::{
         Pmtud,
         cc::{
-            CWND_INITIAL_PKTS, CongestionControl, CongestionControlAlgorithm, CongestionEvent,
+            CWND_INITIAL_PKTS, CongestionControl as _, CongestionController, CongestionEvent,
             classic_cc::Phase,
             cubic::Cubic,
             new_reno::NewReno,
@@ -884,21 +884,8 @@ mod tests {
         )
     }
 
-    fn congestion_control(cc: CongestionControlAlgorithm) -> Box<dyn CongestionControl> {
-        match cc {
-            CongestionControlAlgorithm::NewReno => Box::new(ClassicCongestionControl::new(
-                NewReno::default(),
-                Pmtud::new(IP_ADDR, MTU),
-            )),
-            CongestionControlAlgorithm::Cubic => Box::new(ClassicCongestionControl::new(
-                Cubic::default(),
-                Pmtud::new(IP_ADDR, MTU),
-            )),
-        }
-    }
-
     fn persistent_congestion_by_algorithm(
-        mut cc: Box<dyn CongestionControl>,
+        mut cc: CongestionController,
         reduced_cwnd: usize,
         lost_packets: &[sent::Packet],
         persistent_expected: bool,
@@ -922,14 +909,19 @@ mod tests {
     }
 
     fn persistent_congestion(lost_packets: &[sent::Packet], persistent_expected: bool) {
-        let cc = congestion_control(CongestionControlAlgorithm::NewReno);
+        let cc = ClassicCongestionControl::new(NewReno::default(), Pmtud::new(IP_ADDR, MTU));
         let cwnd_initial = cc.cwnd_initial();
-        persistent_congestion_by_algorithm(cc, cwnd_initial / 2, lost_packets, persistent_expected);
+        persistent_congestion_by_algorithm(
+            CongestionController::NewReno(cc),
+            cwnd_initial / 2,
+            lost_packets,
+            persistent_expected,
+        );
 
-        let cc = congestion_control(CongestionControlAlgorithm::Cubic);
+        let cc = ClassicCongestionControl::new(Cubic::default(), Pmtud::new(IP_ADDR, MTU));
         let cwnd_initial = cc.cwnd_initial();
         persistent_congestion_by_algorithm(
-            cc,
+            CongestionController::Cubic(cc),
             cwnd_initial * Cubic::BETA_USIZE_DIVIDEND / Cubic::BETA_USIZE_DIVISOR,
             lost_packets,
             persistent_expected,

neqo-transport/src/cc/classic_cc.rs

@@ -11,6 +11,7 @@ use std::{
     time::{Duration, Instant},
 };
 
+use enum_dispatch::enum_dispatch;
 use enum_map::Enum;
 use neqo_common::qlog::Qlog;
 
@@ -31,6 +32,7 @@ pub enum CongestionEvent {
     Spurious,
 }
 
+#[enum_dispatch]
 pub trait CongestionControl: Display + Debug {
     fn set_qlog(&mut self, qlog: Qlog);
 
@@ -46,10 +48,6 @@ pub trait CongestionControl: Display + Debug {
     #[must_use]
     fn cwnd_min(&self) -> usize;
 
-    #[cfg(test)]
-    #[must_use]
-    fn cwnd_initial(&self) -> usize;
-
     #[must_use]
     fn pmtud(&self) -> &Pmtud;
 
@@ -103,6 +101,19 @@ pub enum CongestionControlAlgorithm {
     Cubic,
 }
 
+/// A concrete congestion controller, dispatching to either `NewReno` or `Cubic`.
+///
+/// This enum avoids the heap allocation and vtable indirection of `Box<dyn CongestionControl>`
+/// on the per-packet hot path.
+#[enum_dispatch(CongestionControl)]
+#[derive(Debug, strum::Display)]
+pub enum CongestionController {
+    #[strum(to_string = "{0}")]
+    NewReno(ClassicCongestionControl<NewReno>),
+    #[strum(to_string = "{0}")]
+    Cubic(ClassicCongestionControl<Cubic>),
+}
+
 #[cfg(test)]
 #[cfg_attr(coverage_nightly, coverage(off))]
 mod tests;

neqo-transport/src/cc/mod.rs

@@ -12,7 +12,10 @@ use neqo_common::{qdebug, qlog::Qlog};
 
 use crate::{
     ConnectionParameters, Stats,
-    cc::{ClassicCongestionControl, CongestionControl, CongestionControlAlgorithm, Cubic, NewReno},
+    cc::{
+        ClassicCongestionControl, CongestionControl as _, CongestionControlAlgorithm,
+        CongestionController, Cubic, NewReno,
+    },
     pace::Pacer,
     pmtud::Pmtud,
     recovery::sent,
@@ -25,7 +28,7 @@ pub const PACING_BURST_SIZE: usize = 2;
 
 #[derive(Debug)]
 pub struct PacketSender {
-    cc: Box<dyn CongestionControl>,
+    cc: CongestionController,
     pacer: Pacer,
 }
 
@@ -35,12 +38,12 @@ impl PacketSender {
         let mtu = pmtud.plpmtu();
         Self {
             cc: match conn_params.get_cc_algorithm() {
-                CongestionControlAlgorithm::NewReno => {
-                    Box::new(ClassicCongestionControl::new(NewReno::default(), pmtud))
-                }
-                CongestionControlAlgorithm::Cubic => {
-                    Box::new(ClassicCongestionControl::new(Cubic::default(), pmtud))
-                }
+                CongestionControlAlgorithm::NewReno => CongestionController::NewReno(
+                    ClassicCongestionControl::new(NewReno::default(), pmtud),
+                ),
+                CongestionControlAlgorithm::Cubic => CongestionController::Cubic(
+                    ClassicCongestionControl::new(Cubic::default(), pmtud),
+                ),
             },
             pacer: Pacer::new(
                 conn_params.pacing_enabled(),