From 7331d45d93a8d469803fb183bacbd4934d39810d Mon Sep 17 00:00:00 2001 From: steank Date: Sat, 4 May 2024 06:23:44 -0700 Subject: [PATCH 1/3] Faster SocketSet::add * Track first empty slot index --- src/iface/socket_set.rs | 31 ++++++++++++++++++++++++++----- 1 file changed, 26 insertions(+), 5 deletions(-) diff --git a/src/iface/socket_set.rs b/src/iface/socket_set.rs index be55fef5d..85feddb11 100644 --- a/src/iface/socket_set.rs +++ b/src/iface/socket_set.rs @@ -43,6 +43,7 @@ impl fmt::Display for SocketHandle { #[derive(Debug)] pub struct SocketSet<'a> { sockets: ManagedSlice<'a, SocketStorage<'a>>, + first_empty_index: usize, } impl<'a> SocketSet<'a> { @@ -52,7 +53,7 @@ impl<'a> SocketSet<'a> { SocketsT: Into>>, { let sockets = sockets.into(); - SocketSet { sockets } + SocketSet { sockets, first_empty_index: 0 } } /// Add a socket to the set, and return its handle. @@ -73,10 +74,23 @@ impl<'a> SocketSet<'a> { let socket = socket.upcast(); - for (index, slot) in self.sockets.iter_mut().enumerate() { - if slot.inner.is_none() { - return put(index, slot, socket); + if self.first_empty_index < self.sockets.len() { + let handle = put(self.first_empty_index, &mut self.sockets[self.first_empty_index], socket); + + let mut set_index = false; + for i in (self.first_empty_index + 1)..self.sockets.len() { + if self.sockets[i].inner.is_none() { + self.first_empty_index = i; + set_index = true; + break; + } + } + + if !set_index { + self.first_empty_index = self.sockets.len(); } + + return handle; } match &mut self.sockets { @@ -85,6 +99,7 @@ impl<'a> SocketSet<'a> { ManagedSlice::Owned(sockets) => { sockets.push(SocketStorage { inner: None }); let index = sockets.len() - 1; + self.first_empty_index = sockets.len(); put(index, &mut sockets[index], socket) } } @@ -124,7 +139,13 @@ impl<'a> SocketSet<'a> { pub fn remove(&mut self, handle: SocketHandle) -> Socket<'a> { net_trace!("[{}]: removing", handle.0); match self.sockets[handle.0].inner.take() { - Some(item) => item.socket, + Some(item) => { + if handle.0 < self.first_empty_index { + self.first_empty_index = handle.0; + } + + item.socket + }, None => panic!("handle does not refer to a valid socket"), } } From 114c93d37620750b9e8d8c6c02f67ff0d4381138 Mon Sep 17 00:00:00 2001 From: steank Date: Thu, 23 May 2024 17:43:22 -0700 Subject: [PATCH 2/3] Formatting fixes, unit tests for modified SocketSet::add * Now passes `cargo fmt -- --check` * Simplify some logic * Add relevant unit tests --- src/iface/socket_set.rs | 104 ++++++++++++++++++++++++++++++++++++---- 1 file changed, 94 insertions(+), 10 deletions(-) diff --git a/src/iface/socket_set.rs b/src/iface/socket_set.rs index 85feddb11..f6f7734d0 100644 --- a/src/iface/socket_set.rs +++ b/src/iface/socket_set.rs @@ -29,6 +29,11 @@ pub(crate) struct Item<'a> { #[cfg_attr(feature = "defmt", derive(defmt::Format))] pub struct SocketHandle(usize); +#[cfg(test)] +pub(crate) fn new_handle(index: usize) -> SocketHandle { + SocketHandle(index) +} + impl fmt::Display for SocketHandle { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { write!(f, "#{}", self.0) @@ -53,7 +58,10 @@ impl<'a> SocketSet<'a> { SocketsT: Into>>, { let sockets = sockets.into(); - SocketSet { sockets, first_empty_index: 0 } + SocketSet { + sockets, + first_empty_index: 0, + } } /// Add a socket to the set, and return its handle. @@ -75,21 +83,20 @@ impl<'a> SocketSet<'a> { let socket = socket.upcast(); if self.first_empty_index < self.sockets.len() { - let handle = put(self.first_empty_index, &mut self.sockets[self.first_empty_index], socket); + let handle = put( + self.first_empty_index, + &mut self.sockets[self.first_empty_index], + socket, + ); - let mut set_index = false; for i in (self.first_empty_index + 1)..self.sockets.len() { if self.sockets[i].inner.is_none() { self.first_empty_index = i; - set_index = true; - break; + return handle; } } - if !set_index { - self.first_empty_index = self.sockets.len(); - } - + self.first_empty_index = self.sockets.len(); return handle; } @@ -145,7 +152,7 @@ impl<'a> SocketSet<'a> { } item.socket - }, + } None => panic!("handle does not refer to a valid socket"), } } @@ -170,3 +177,80 @@ impl<'a> SocketSet<'a> { self.sockets.iter_mut().filter_map(|x| x.inner.as_mut()) } } + +#[cfg(test)] +pub(crate) mod test { + use crate::iface::socket_set::new_handle; + use crate::iface::SocketSet; + use crate::socket::tcp; + use crate::socket::tcp::Socket; + use std::ptr; + + fn gen_owned_socket() -> Socket<'static> { + let rx = tcp::SocketBuffer::new(vec![0; 1]); + let tx = tcp::SocketBuffer::new(vec![0; 1]); + Socket::new(rx, tx) + } + + fn gen_owned_socket_set(size: usize) -> SocketSet<'static> { + let mut socket_set = SocketSet::new(Vec::with_capacity(size)); + for _ in 0..size { + socket_set.add(gen_owned_socket()); + } + + socket_set + } + + #[test] + fn test_add() { + let socket_set = gen_owned_socket_set(5); + assert_eq!(socket_set.first_empty_index, 5); + } + + #[test] + fn test_remove() { + let mut socket_set = gen_owned_socket_set(10); + + let removed_socket = socket_set.remove(new_handle(5)); + for socket in socket_set.iter() { + assert!(!ptr::eq(socket.1, &removed_socket)); + } + + assert_eq!(socket_set.first_empty_index, 5); + } + + #[test] + fn test_remove_add_integrity() { + let mut socket_set = gen_owned_socket_set(10); + + for remove_index in 0..10 { + let removed_socket = socket_set.remove(new_handle(remove_index)); + for socket in socket_set.iter() { + assert!(!ptr::eq(socket.1, &removed_socket)); + } + + let new_socket = gen_owned_socket(); + let handle = socket_set.add(new_socket); + assert_eq!(handle.0, remove_index); + } + + assert_eq!(socket_set.first_empty_index, 10); + } + + #[test] + fn test_full_reconstruct() { + let mut socket_set = gen_owned_socket_set(10); + + for index in 0..10 { + socket_set.remove(new_handle(index)); + } + + assert_eq!(socket_set.first_empty_index, 0); + + for _ in 0..10 { + socket_set.add(gen_owned_socket()); + } + + assert_eq!(socket_set.first_empty_index, 10); + } +} From 99784613e727bcfa31ecd6100c13328ecae3e393 Mon Sep 17 00:00:00 2001 From: steank Date: Thu, 23 May 2024 18:24:16 -0700 Subject: [PATCH 3/3] Add required #[cfg] to SocketSet test module --- src/iface/socket_set.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/iface/socket_set.rs b/src/iface/socket_set.rs index f6f7734d0..028e904fa 100644 --- a/src/iface/socket_set.rs +++ b/src/iface/socket_set.rs @@ -179,6 +179,7 @@ impl<'a> SocketSet<'a> { } #[cfg(test)] +#[cfg(all(feature = "socket-tcp", any(feature = "std", feature = "alloc")))] pub(crate) mod test { use crate::iface::socket_set::new_handle; use crate::iface::SocketSet;