chore: Avoid Box<dyn CongestionControl> (#3451)

* chore: Avoid `Box<dyn CongestionControl>`

For discussion - is this worth it?

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

* Use shared macro

* Fix

Author: larseggert

neqo-common/src/lib.rs

@@ -107,6 +107,31 @@ pub enum MessageType {
     Response,
 }
 
+/// Dispatch a method call on an enum to its variants' inner values.
+///
+/// The variant list is given once in a local wrapper; method bodies stay clean:
+///
+/// ```ignore
+/// // Once per enum, in the impl module:
+/// macro_rules! dispatch {
+///     ($self:ident . $method:ident $args:tt) => {
+///         neqo_common::dispatch!([Variant1, Variant2, Variant3] $self . $method $args)
+///     };
+/// }
+///
+/// impl SomeTrait for MyEnum {
+///     fn method(&self) -> T { dispatch!(self.method()) }
+/// }
+/// ```
+#[macro_export]
+macro_rules! dispatch {
+    ([$($variant:ident),+ $(,)?] $self:ident . $method:ident $args:tt) => {
+        match $self {
+            $( Self::$variant(v) => v.$method $args, )+
+        }
+    };
+}
+
 #[cfg(test)]
 #[cfg_attr(coverage_nightly, coverage(off))]
 mod tests {

neqo-transport/src/cc/classic_cc.rs

@@ -229,6 +229,11 @@ impl<S, T> ClassicCongestionController<S, 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<S, T> CongestionController for ClassicCongestionController<S, T>
@@ -261,11 +266,6 @@ where
         self.max_datagram_size() * 2
     }
 
-    #[cfg(test)]
-    fn cwnd_initial(&self) -> usize {
-        cwnd_initial(self.pmtud.plpmtu())
-    }
-
     fn pmtud(&self) -> &Pmtud {
         &self.pmtud
     }

neqo-transport/src/cc/mod.rs

@@ -48,10 +48,6 @@ pub trait CongestionController: Display + Debug {
     #[must_use]
     fn cwnd_min(&self) -> usize;
 
-    #[cfg(test)]
-    #[must_use]
-    fn cwnd_initial(&self) -> usize;
-
     #[must_use]
     fn pmtud(&self) -> &Pmtud;
 
@@ -115,6 +111,116 @@ pub enum SlowStart {
     HyStart,
 }
 
+/// A concrete congestion controller, dispatching across all combinations of
+/// algorithm and slow-start strategy.
+///
+/// This enum avoids the heap allocation and vtable indirection of `Box<dyn CongestionController>`
+/// on the per-packet hot path.
+#[derive(Debug, strum::Display)]
+pub enum CongestionControlImplementation {
+    #[strum(to_string = "{0}")]
+    ClassicNewReno(ClassicCongestionController<ClassicSlowStart, NewReno>),
+    #[strum(to_string = "{0}")]
+    HyStartNewReno(ClassicCongestionController<HyStart, NewReno>),
+    #[strum(to_string = "{0}")]
+    ClassicCubic(ClassicCongestionController<ClassicSlowStart, Cubic>),
+    #[strum(to_string = "{0}")]
+    HyStartCubic(ClassicCongestionController<HyStart, Cubic>),
+}
+
+macro_rules! dispatch {
+    ($self:ident . $method:ident $args:tt) => {
+        neqo_common::dispatch!(
+            [ClassicNewReno, HyStartNewReno, ClassicCubic, HyStartCubic]
+            $self . $method $args
+        )
+    };
+}
+
+impl CongestionController for CongestionControlImplementation {
+    fn set_qlog(&mut self, qlog: Qlog) {
+        dispatch!(self.set_qlog(qlog));
+    }
+
+    fn cwnd(&self) -> usize {
+        dispatch!(self.cwnd())
+    }
+
+    fn bytes_in_flight(&self) -> usize {
+        dispatch!(self.bytes_in_flight())
+    }
+
+    fn cwnd_avail(&self) -> usize {
+        dispatch!(self.cwnd_avail())
+    }
+
+    fn cwnd_min(&self) -> usize {
+        dispatch!(self.cwnd_min())
+    }
+
+    fn pmtud(&self) -> &Pmtud {
+        dispatch!(self.pmtud())
+    }
+
+    fn pmtud_mut(&mut self) -> &mut Pmtud {
+        dispatch!(self.pmtud_mut())
+    }
+
+    fn on_packets_acked(
+        &mut self,
+        acked_pkts: &[sent::Packet],
+        rtt_est: &RttEstimate,
+        now: Instant,
+        cc_stats: &mut CongestionControlStats,
+    ) {
+        dispatch!(self.on_packets_acked(acked_pkts, rtt_est, now, cc_stats));
+    }
+
+    fn on_packets_lost(
+        &mut self,
+        first_rtt_sample_time: Option<Instant>,
+        prev_largest_acked_sent: Option<Instant>,
+        pto: Duration,
+        lost_packets: &[sent::Packet],
+        now: Instant,
+        cc_stats: &mut CongestionControlStats,
+    ) -> bool {
+        dispatch!(self.on_packets_lost(
+            first_rtt_sample_time,
+            prev_largest_acked_sent,
+            pto,
+            lost_packets,
+            now,
+            cc_stats,
+        ))
+    }
+
+    fn on_ecn_ce_received(
+        &mut self,
+        largest_acked_pkt: &sent::Packet,
+        now: Instant,
+        cc_stats: &mut CongestionControlStats,
+    ) -> bool {
+        dispatch!(self.on_ecn_ce_received(largest_acked_pkt, now, cc_stats))
+    }
+
+    fn recovery_packet(&self) -> bool {
+        dispatch!(self.recovery_packet())
+    }
+
+    fn discard(&mut self, pkt: &sent::Packet, now: Instant) {
+        dispatch!(self.discard(pkt, now));
+    }
+
+    fn on_packet_sent(&mut self, pkt: &sent::Packet, now: Instant) {
+        dispatch!(self.on_packet_sent(pkt, now));
+    }
+
+    fn discard_in_flight(&mut self, now: Instant) {
+        dispatch!(self.discard_in_flight(now));
+    }
+}
+
 #[cfg(test)]
 #[cfg_attr(coverage_nightly, coverage(off))]
 mod tests;

neqo-transport/src/sender.rs

@@ -13,8 +13,8 @@ use neqo_common::{qdebug, qlog::Qlog};
 use crate::{
     ConnectionParameters, SlowStart, Stats,
     cc::{
-        ClassicCongestionController, ClassicSlowStart, CongestionControl, CongestionController,
-        Cubic, HyStart, NewReno,
+        ClassicCongestionController, ClassicSlowStart, CongestionControl,
+        CongestionControlImplementation, CongestionController as _, Cubic, HyStart, NewReno,
     },
     pace::Pacer,
     pmtud::Pmtud,
@@ -29,7 +29,7 @@ pub const PACING_BURST_SIZE: usize = 2;
 
 #[derive(Debug)]
 pub struct PacketSender {
-    cc: Box<dyn CongestionController>,
+    cc: CongestionControlImplementation,
     pacer: Pacer,
     qlog: Qlog,
 }
@@ -45,34 +45,38 @@ impl PacketSender {
                 conn_params.get_slow_start(),
             ) {
                 (CongestionControl::NewReno, SlowStart::Classic) => {
-                    Box::new(ClassicCongestionController::new(
-                        ClassicSlowStart::default(),
-                        NewReno::default(),
-                        pmtud,
-                        spurious_recovery,
-                    ))
+                    CongestionControlImplementation::ClassicNewReno(
+                        ClassicCongestionController::new(
+                            ClassicSlowStart::default(),
+                            NewReno::default(),
+                            pmtud,
+                            spurious_recovery,
+                        ),
+                    )
                 }
                 (CongestionControl::NewReno, SlowStart::HyStart) => {
-                    Box::new(ClassicCongestionController::new(
-                        HyStart::new(
-                            conn_params.pacing_enabled(),
-                            conn_params.get_hystart_css_baseline(),
+                    CongestionControlImplementation::HyStartNewReno(
+                        ClassicCongestionController::new(
+                            HyStart::new(
+                                conn_params.pacing_enabled(),
+                                conn_params.get_hystart_css_baseline(),
+                            ),
+                            NewReno::default(),
+                            pmtud,
+                            spurious_recovery,
                         ),
-                        NewReno::default(),
-                        pmtud,
-                        spurious_recovery,
-                    ))
+                    )
                 }
                 (CongestionControl::Cubic, SlowStart::Classic) => {
-                    Box::new(ClassicCongestionController::new(
+                    CongestionControlImplementation::ClassicCubic(ClassicCongestionController::new(
                         ClassicSlowStart::default(),
                         Cubic::default(),
                         pmtud,
                         spurious_recovery,
                     ))
                 }
                 (CongestionControl::Cubic, SlowStart::HyStart) => {
-                    Box::new(ClassicCongestionController::new(
+                    CongestionControlImplementation::HyStartCubic(ClassicCongestionController::new(
                         HyStart::new(
                             conn_params.pacing_enabled(),
                             conn_params.get_hystart_css_baseline(),