Skip to content

Commit 908ef8d

Browse files
astrowhitequark
authored andcommitted
Optimize RingBuffer::enqueue_many_with to an empty buffer.
This optimization makes sure that enqueueing to an empty ring buffer would never wrap around, in turn ensuring that enqueueing to an empty packet buffer would never use more than one metadata entry. Right now, pushing buffer-sized packets into a packet buffer requires at least two metadata entries, which is surprising and wasteful.
1 parent 9c1900f commit 908ef8d

File tree

2 files changed

+34
-20
lines changed

2 files changed

+34
-20
lines changed

src/socket/tcp.rs

Lines changed: 9 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -3830,33 +3830,25 @@ mod test {
38303830
#[test]
38313831
fn test_buffer_wraparound_tx() {
38323832
let mut s = socket_established();
3833-
s.tx_buffer = SocketBuffer::new(vec![0; 6]);
3834-
assert_eq!(s.send_slice(b"abc"), Ok(3));
3833+
s.tx_buffer = SocketBuffer::new(vec![b'.'; 9]);
3834+
assert_eq!(s.send_slice(b"xxxyyy"), Ok(6));
3835+
assert_eq!(s.tx_buffer.dequeue_many(3), &b"xxx"[..]);
3836+
assert_eq!(s.tx_buffer.len(), 3);
3837+
3838+
// "abcdef" not contiguous in tx buffer
3839+
assert_eq!(s.send_slice(b"abcdef"), Ok(6));
38353840
recv!(s, Ok(TcpRepr {
38363841
seq_number: LOCAL_SEQ + 1,
38373842
ack_number: Some(REMOTE_SEQ + 1),
3838-
payload: &b"abc"[..],
3843+
payload: &b"yyyabc"[..],
38393844
..RECV_TEMPL
38403845
}));
3841-
send!(s, TcpRepr {
3842-
seq_number: REMOTE_SEQ + 1,
3843-
ack_number: Some(LOCAL_SEQ + 1 + 3),
3844-
..SEND_TEMPL
3845-
});
3846-
assert_eq!(s.send_slice(b"defghi"), Ok(6));
38473846
recv!(s, Ok(TcpRepr {
3848-
seq_number: LOCAL_SEQ + 1 + 3,
3847+
seq_number: LOCAL_SEQ + 1 + 6,
38493848
ack_number: Some(REMOTE_SEQ + 1),
38503849
payload: &b"def"[..],
38513850
..RECV_TEMPL
38523851
}));
3853-
// "defghi" not contiguous in tx buffer
3854-
recv!(s, Ok(TcpRepr {
3855-
seq_number: LOCAL_SEQ + 1 + 3 + 3,
3856-
ack_number: Some(REMOTE_SEQ + 1),
3857-
payload: &b"ghi"[..],
3858-
..RECV_TEMPL
3859-
}));
38603852
}
38613853

38623854
// =========================================================================================//

src/storage/ring_buffer.rs

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,12 @@ impl<'a, T: 'a> RingBuffer<'a, T> {
170170
/// than the size of the slice passed into it.
171171
pub fn enqueue_many_with<'b, R, F>(&'b mut self, f: F) -> (usize, R)
172172
where F: FnOnce(&'b mut [T]) -> (usize, R) {
173+
if self.length == 0 {
174+
// Ring is currently empty. Reset `read_at` to optimize
175+
// for contiguous space.
176+
self.read_at = 0;
177+
}
178+
173179
let write_at = self.get_idx(self.length);
174180
let max_size = self.contiguous_window();
175181
let (size, result) = f(&mut self.storage[write_at..write_at + max_size]);
@@ -659,13 +665,13 @@ mod test {
659665
ring.dequeue_many(6).copy_from_slice(b"ABCDEF");
660666

661667
assert_eq!(ring.write_unallocated(0, b"ghi"), 3);
662-
assert_eq!(&ring.storage[..], b"ABCDEFghi...");
668+
assert_eq!(ring.get_unallocated(0, 3), b"ghi");
663669

664670
assert_eq!(ring.write_unallocated(3, b"jklmno"), 6);
665-
assert_eq!(&ring.storage[..], b"mnoDEFghijkl");
671+
assert_eq!(ring.get_unallocated(3, 3), b"jkl");
666672

667673
assert_eq!(ring.write_unallocated(9, b"pqrstu"), 3);
668-
assert_eq!(&ring.storage[..], b"mnopqrghijkl");
674+
assert_eq!(ring.get_unallocated(9, 3), b"pqr");
669675
}
670676

671677
#[test]
@@ -721,4 +727,20 @@ mod test {
721727
assert_eq!(no_capacity.enqueue_one(), Err(Error::Exhausted));
722728
assert_eq!(no_capacity.contiguous_window(), 0);
723729
}
730+
731+
/// Use the buffer a bit. Then empty it and put in an item of
732+
/// maximum size. By detecting a length of 0, the implementation
733+
/// can reset the current buffer position.
734+
#[test]
735+
fn test_buffer_write_wholly() {
736+
let mut ring = RingBuffer::new(vec![b'.'; 8]);
737+
ring.enqueue_many(2).copy_from_slice(b"xx");
738+
ring.enqueue_many(2).copy_from_slice(b"xx");
739+
assert_eq!(ring.len(), 4);
740+
ring.dequeue_many(4);
741+
assert_eq!(ring.len(), 0);
742+
743+
let large = ring.enqueue_many(8);
744+
assert_eq!(large.len(), 8);
745+
}
724746
}

0 commit comments

Comments
 (0)