perf(transport): have add_datagram take Vec<u8> (#2120)

`add_datagram` takes a Quic datagram and adds it to the queue of datagrams to be
sent out. Previously it would take a reference (i.e. `&[u8]`) and would allocate
it into a new `Vec<u8>` before enqueuing. At the call-site the original
allocation (referenced by the `&[u8]`) would go out-of-scope and thus be
de-allocated. This is a wasted allocation for each Quic datagram to be send.

This commit has the call-site pass the owned `Vec<u8>` down right away.

Co-authored-by: Lars Eggert <[email protected]>

neqo-http3/src/features/extended_connect/webtransport_session.rs

@@ -415,7 +415,7 @@ impl WebTransportSession {
             let mut dgram_data = Encoder::default();
             dgram_data.encode_varint(self.session_id.as_u64() / 4);
             dgram_data.encode(buf);
-            conn.send_datagram(dgram_data.as_ref(), id)?;
+            conn.send_datagram(dgram_data.into(), id)?;
         } else {
             debug_assert!(false);
             return Err(Error::Unavailable);

neqo-transport/src/connection/mod.rs

@@ -3460,7 +3460,7 @@ impl Connection {
     /// to check the estimated max datagram size and to use smaller datagrams.
     /// `max_datagram_size` is just a current estimate and will change over
     /// time depending on the encoded size of the packet number, ack frames, etc.
-    pub fn send_datagram(&mut self, buf: &[u8], id: impl Into<DatagramTracking>) -> Res<()> {
+    pub fn send_datagram(&mut self, buf: Vec<u8>, id: impl Into<DatagramTracking>) -> Res<()> {
         self.quic_datagrams
             .add_datagram(buf, id.into(), &mut self.stats.borrow_mut())
     }

neqo-transport/src/connection/tests/datagram.rs

@@ -58,12 +58,12 @@ fn datagram_disabled_both() {
     assert_eq!(client.max_datagram_size(), Err(Error::NotAvailable));
     assert_eq!(server.max_datagram_size(), Err(Error::NotAvailable));
     assert_eq!(
-        client.send_datagram(DATA_SMALLER_THAN_MTU, None),
+        client.send_datagram(DATA_SMALLER_THAN_MTU.to_vec(), None),
         Err(Error::TooMuchData)
     );
     assert_eq!(server.stats().frame_tx.datagram, 0);
     assert_eq!(
-        server.send_datagram(DATA_SMALLER_THAN_MTU, None),
+        server.send_datagram(DATA_SMALLER_THAN_MTU.to_vec(), None),
         Err(Error::TooMuchData)
     );
     assert_eq!(server.stats().frame_tx.datagram, 0);
@@ -82,11 +82,14 @@ fn datagram_enabled_on_client() {
         Ok(DATAGRAM_LEN_SMALLER_THAN_MTU)
     );
     assert_eq!(
-        client.send_datagram(DATA_SMALLER_THAN_MTU, Some(1)),
+        client.send_datagram(DATA_SMALLER_THAN_MTU.to_vec(), Some(1)),
         Err(Error::TooMuchData)
     );
     let dgram_sent = server.stats().frame_tx.datagram;
-    assert_eq!(server.send_datagram(DATA_SMALLER_THAN_MTU, Some(1)), Ok(()));
+    assert_eq!(
+        server.send_datagram(DATA_SMALLER_THAN_MTU.to_vec(), Some(1)),
+        Ok(())
+    );
     let out = server.process_output(now()).dgram().unwrap();
     assert_eq!(server.stats().frame_tx.datagram, dgram_sent + 1);
 
@@ -110,11 +113,14 @@ fn datagram_enabled_on_server() {
     );
     assert_eq!(server.max_datagram_size(), Err(Error::NotAvailable));
     assert_eq!(
-        server.send_datagram(DATA_SMALLER_THAN_MTU, Some(1)),
+        server.send_datagram(DATA_SMALLER_THAN_MTU.to_vec(), Some(1)),
         Err(Error::TooMuchData)
     );
     let dgram_sent = client.stats().frame_tx.datagram;
-    assert_eq!(client.send_datagram(DATA_SMALLER_THAN_MTU, Some(1)), Ok(()));
+    assert_eq!(
+        client.send_datagram(DATA_SMALLER_THAN_MTU.to_vec(), Some(1)),
+        Ok(())
+    );
     let out = client.process_output(now()).dgram().unwrap();
     assert_eq!(client.stats().frame_tx.datagram, dgram_sent + 1);
 
@@ -156,7 +162,10 @@ fn limit_data_size() {
 
     // Datagram can be queued because they are smaller than allowed by the peer,
     // but they cannot be sent.
-    assert_eq!(server.send_datagram(DATA_BIGGER_THAN_MTU, Some(1)), Ok(()));
+    assert_eq!(
+        server.send_datagram(DATA_BIGGER_THAN_MTU.to_vec(), Some(1)),
+        Ok(())
+    );
 
     let dgram_dropped_s = server.stats().datagram_tx.dropped_too_big;
     let dgram_sent_s = server.stats().frame_tx.datagram;
@@ -172,7 +181,10 @@ fn limit_data_size() {
     ));
 
     // The same test for the client side.
-    assert_eq!(client.send_datagram(DATA_BIGGER_THAN_MTU, Some(1)), Ok(()));
+    assert_eq!(
+        client.send_datagram(DATA_BIGGER_THAN_MTU.to_vec(), Some(1)),
+        Ok(())
+    );
     let dgram_sent_c = client.stats().frame_tx.datagram;
     assert!(client.process_output(now()).dgram().is_none());
     assert_eq!(client.stats().frame_tx.datagram, dgram_sent_c);
@@ -188,8 +200,14 @@ fn after_dgram_dropped_continue_writing_frames() {
 
     // Datagram can be queued because they are smaller than allowed by the peer,
     // but they cannot be sent.
-    assert_eq!(client.send_datagram(DATA_BIGGER_THAN_MTU, Some(1)), Ok(()));
-    assert_eq!(client.send_datagram(DATA_SMALLER_THAN_MTU, Some(2)), Ok(()));
+    assert_eq!(
+        client.send_datagram(DATA_BIGGER_THAN_MTU.to_vec(), Some(1)),
+        Ok(())
+    );
+    assert_eq!(
+        client.send_datagram(DATA_SMALLER_THAN_MTU.to_vec(), Some(2)),
+        Ok(())
+    );
 
     let datagram_dropped = |e| {
         matches!(
@@ -214,7 +232,10 @@ fn datagram_acked() {
     let (mut client, mut server) = connect_datagram();
 
     let dgram_sent = client.stats().frame_tx.datagram;
-    assert_eq!(client.send_datagram(DATA_SMALLER_THAN_MTU, Some(1)), Ok(()));
+    assert_eq!(
+        client.send_datagram(DATA_SMALLER_THAN_MTU.to_vec(), Some(1)),
+        Ok(())
+    );
     let out = client.process_output(now()).dgram();
     assert_eq!(client.stats().frame_tx.datagram, dgram_sent + 1);
 
@@ -267,7 +288,7 @@ fn datagram_after_stream_data() {
 
     // Write a datagram first.
     let dgram_sent = client.stats().frame_tx.datagram;
-    assert_eq!(client.send_datagram(DATA_MTU, Some(1)), Ok(()));
+    assert_eq!(client.send_datagram(DATA_MTU.to_vec(), Some(1)), Ok(()));
 
     // Create a stream with normal priority and send some data.
     let stream_id = client.stream_create(StreamType::BiDi).unwrap();
@@ -309,7 +330,7 @@ fn datagram_before_stream_data() {
 
     // Write a datagram.
     let dgram_sent = client.stats().frame_tx.datagram;
-    assert_eq!(client.send_datagram(DATA_MTU, Some(1)), Ok(()));
+    assert_eq!(client.send_datagram(DATA_MTU.to_vec(), Some(1)), Ok(()));
 
     if let ConnectionEvent::Datagram(data) =
         &send_packet_and_get_server_event(&mut client, &mut server)
@@ -331,7 +352,10 @@ fn datagram_lost() {
     let (mut client, _) = connect_datagram();
 
     let dgram_sent = client.stats().frame_tx.datagram;
-    assert_eq!(client.send_datagram(DATA_SMALLER_THAN_MTU, Some(1)), Ok(()));
+    assert_eq!(
+        client.send_datagram(DATA_SMALLER_THAN_MTU.to_vec(), Some(1)),
+        Ok(())
+    );
     let _out = client.process_output(now()).dgram(); // This packet will be lost.
     assert_eq!(client.stats().frame_tx.datagram, dgram_sent + 1);
 
@@ -358,7 +382,10 @@ fn datagram_sent_once() {
     let (mut client, _) = connect_datagram();
 
     let dgram_sent = client.stats().frame_tx.datagram;
-    assert_eq!(client.send_datagram(DATA_SMALLER_THAN_MTU, Some(1)), Ok(()));
+    assert_eq!(
+        client.send_datagram(DATA_SMALLER_THAN_MTU.to_vec(), Some(1)),
+        Ok(())
+    );
     let _out = client.process_output(now()).dgram();
     assert_eq!(client.stats().frame_tx.datagram, dgram_sent + 1);
 
@@ -402,16 +429,19 @@ fn outgoing_datagram_queue_full() {
     let (mut client, mut server) = connect_datagram();
 
     let dgram_sent = client.stats().frame_tx.datagram;
-    assert_eq!(client.send_datagram(DATA_SMALLER_THAN_MTU, Some(1)), Ok(()));
     assert_eq!(
-        client.send_datagram(DATA_SMALLER_THAN_MTU_2, Some(2)),
+        client.send_datagram(DATA_SMALLER_THAN_MTU.to_vec(), Some(1)),
+        Ok(())
+    );
+    assert_eq!(
+        client.send_datagram(DATA_SMALLER_THAN_MTU_2.to_vec(), Some(2)),
         Ok(())
     );
 
     // The outgoing datagram queue limit is 2, therefore the datagram with id 1
     // will be dropped after adding one more datagram.
     let dgram_dropped = client.stats().datagram_tx.dropped_queue_full;
-    assert_eq!(client.send_datagram(DATA_MTU, Some(3)), Ok(()));
+    assert_eq!(client.send_datagram(DATA_MTU.to_vec(), Some(3)), Ok(()));
     assert!(matches!(
         client.next_event().unwrap(),
         ConnectionEvent::OutgoingDatagramOutcome { id, outcome } if id == 1 && outcome == OutgoingDatagramOutcome::DroppedQueueFull
@@ -441,7 +471,7 @@ fn outgoing_datagram_queue_full() {
     ));
 }
 
-fn send_datagram(sender: &mut Connection, receiver: &mut Connection, data: &[u8]) {
+fn send_datagram(sender: &mut Connection, receiver: &mut Connection, data: Vec<u8>) {
     let dgram_sent = sender.stats().frame_tx.datagram;
     assert_eq!(sender.send_datagram(data, Some(1)), Ok(()));
     let out = sender.process_output(now()).dgram().unwrap();
@@ -469,9 +499,9 @@ fn multiple_datagram_events() {
     let mut server = default_server();
     connect_force_idle(&mut client, &mut server);
 
-    send_datagram(&mut server, &mut client, FIRST_DATAGRAM);
-    send_datagram(&mut server, &mut client, SECOND_DATAGRAM);
-    send_datagram(&mut server, &mut client, THIRD_DATAGRAM);
+    send_datagram(&mut server, &mut client, FIRST_DATAGRAM.to_vec());
+    send_datagram(&mut server, &mut client, SECOND_DATAGRAM.to_vec());
+    send_datagram(&mut server, &mut client, THIRD_DATAGRAM.to_vec());
 
     let mut datagrams = client.events().filter_map(|evt| {
         if let ConnectionEvent::Datagram(d) = evt {
@@ -486,7 +516,7 @@ fn multiple_datagram_events() {
     assert!(datagrams.next().is_none());
 
     // New events can be queued.
-    send_datagram(&mut server, &mut client, FOURTH_DATAGRAM);
+    send_datagram(&mut server, &mut client, FOURTH_DATAGRAM.to_vec());
     let mut datagrams = client.events().filter_map(|evt| {
         if let ConnectionEvent::Datagram(d) = evt {
             Some(d)
@@ -515,9 +545,9 @@ fn too_many_datagram_events() {
     let mut server = default_server();
     connect_force_idle(&mut client, &mut server);
 
-    send_datagram(&mut server, &mut client, FIRST_DATAGRAM);
-    send_datagram(&mut server, &mut client, SECOND_DATAGRAM);
-    send_datagram(&mut server, &mut client, THIRD_DATAGRAM);
+    send_datagram(&mut server, &mut client, FIRST_DATAGRAM.to_vec());
+    send_datagram(&mut server, &mut client, SECOND_DATAGRAM.to_vec());
+    send_datagram(&mut server, &mut client, THIRD_DATAGRAM.to_vec());
 
     // Datagram with FIRST_DATAGRAM data will be dropped.
     assert!(matches!(
@@ -536,7 +566,7 @@ fn too_many_datagram_events() {
     assert_eq!(client.stats().incoming_datagram_dropped, 1);
 
     // New events can be queued.
-    send_datagram(&mut server, &mut client, FOURTH_DATAGRAM);
+    send_datagram(&mut server, &mut client, FOURTH_DATAGRAM.to_vec());
     assert!(matches!(
         client.next_event().unwrap(),
         ConnectionEvent::Datagram(data) if data == FOURTH_DATAGRAM
@@ -552,11 +582,11 @@ fn multiple_quic_datagrams_in_one_packet() {
     let dgram_sent = client.stats().frame_tx.datagram;
     // Enqueue 2 datagrams that can fit in a single packet.
     assert_eq!(
-        client.send_datagram(DATA_SMALLER_THAN_MTU_2, Some(1)),
+        client.send_datagram(DATA_SMALLER_THAN_MTU_2.to_vec(), Some(1)),
         Ok(())
     );
     assert_eq!(
-        client.send_datagram(DATA_SMALLER_THAN_MTU_2, Some(2)),
+        client.send_datagram(DATA_SMALLER_THAN_MTU_2.to_vec(), Some(2)),
         Ok(())
     );
 
@@ -608,21 +638,21 @@ fn datagram_fill() {
 
     let buf = vec![9; space];
     // This will completely fill available space.
-    send_datagram(&mut client, &mut server, &buf);
+    send_datagram(&mut client, &mut server, buf.clone());
     // This will leave 1 byte free, but more frames won't be added in this space.
-    send_datagram(&mut client, &mut server, &buf[..buf.len() - 1]);
+    send_datagram(&mut client, &mut server, buf[..buf.len() - 1].to_vec());
     // This will leave 2 bytes free, which is enough space for a length field,
     // but not enough space for another frame after that.
-    send_datagram(&mut client, &mut server, &buf[..buf.len() - 2]);
+    send_datagram(&mut client, &mut server, buf[..buf.len() - 2].to_vec());
     // Three bytes free will be space enough for a length frame, but not enough
     // space left over for another frame (we need 2 bytes).
-    send_datagram(&mut client, &mut server, &buf[..buf.len() - 3]);
+    send_datagram(&mut client, &mut server, buf[..buf.len() - 3].to_vec());
 
     // Four bytes free is enough space for another frame.
     let called = Rc::new(RefCell::new(false));
     client.test_frame_writer = Some(Box::new(TrackingFrameWriter {
         called: Rc::clone(&called),
     }));
-    send_datagram(&mut client, &mut server, &buf[..buf.len() - 4]);
+    send_datagram(&mut client, &mut server, buf[..buf.len() - 4].to_vec());
     assert!(*called.borrow());
 }

neqo-transport/src/quic_datagrams.rs

@@ -150,11 +150,11 @@ impl QuicDatagrams {
     /// not fit into the packet.
     pub fn add_datagram(
         &mut self,
-        buf: &[u8],
+        data: Vec<u8>,
         tracking: DatagramTracking,
         stats: &mut Stats,
     ) -> Res<()> {
-        if u64::try_from(buf.len())? > self.remote_datagram_size {
+        if u64::try_from(data.len())? > self.remote_datagram_size {
             return Err(Error::TooMuchData);
         }
         if self.datagrams.len() == self.max_queued_outgoing_datagrams {
@@ -167,10 +167,7 @@ impl QuicDatagrams {
             );
             stats.datagram_tx.dropped_queue_full += 1;
         }
-        self.datagrams.push_back(QuicDatagram {
-            data: buf.to_vec(),
-            tracking,
-        });
+        self.datagrams.push_back(QuicDatagram { data, tracking });
         Ok(())
     }