From 77c437a252c8accd4e52b228752c03019b50b60b Mon Sep 17 00:00:00 2001 From: Karl Rikte Date: Sat, 13 Jan 2024 00:19:35 +0100 Subject: [PATCH 1/5] Improve response times When the network interface can't make any progress, it's probably a good idea to yeild the task so that the wifi task may run and proceed with actually sending/receiving data. This seems to greatly improve response times, especially with low tick_rate_hz values. At 100Hz, ping time goes down from 25ms to <2ms. A simple http server test saw a response time reduction from 45ms to 6ms. At 1000Hz tick rate the gains are diminishing but there are some small gains. I ran my tests on esp32s3. This might also increase throughput. I have not tested that. It also may have some unforseen adverse effects. Ideally, we would have a more fine grained return value from interface.poll() than a bool so we have a better idea if yeilding is appropriate or not. --- esp-wifi/src/wifi_interface.rs | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/esp-wifi/src/wifi_interface.rs b/esp-wifi/src/wifi_interface.rs index 7247e084..a975122b 100644 --- a/esp-wifi/src/wifi_interface.rs +++ b/esp-wifi/src/wifi_interface.rs @@ -452,6 +452,7 @@ impl<'a, MODE: WifiDeviceMode> WifiStack<'a, MODE> { sockets, ) }) { + crate::timer::yield_task(); break; } } @@ -725,7 +726,9 @@ impl<'s, 'n: 's, MODE: WifiDeviceMode> Read for Socket<'s, 'n, MODE> { use smoltcp::socket::tcp::RecvError; loop { - interface.poll(timestamp(), device, sockets); + if interface.poll(timestamp(), device, sockets) == false { + crate::timer::yield_task(); + } let socket = sockets.get_mut::(self.socket_handle); match socket.recv_slice(buf) { @@ -745,11 +748,14 @@ impl<'s, 'n: 's, MODE: WifiDeviceMode> Write for Socket<'s, 'n, MODE> { loop { let (may_send, is_open, can_send) = self.network.with_mut(|interface, device, sockets| { - interface.poll( + if interface.poll( Instant::from_millis((self.network.current_millis_fn)() as i64), device, sockets, - ); + ) == false + { + crate::timer::yield_task(); + } let socket = sockets.get_mut::(self.socket_handle); @@ -796,6 +802,7 @@ impl<'s, 'n: 's, MODE: WifiDeviceMode> Write for Socket<'s, 'n, MODE> { }); if let false = res { + crate::timer::yield_task(); break; } } From e00d3188d002719484cd90dcf2452a759c594b1a Mon Sep 17 00:00:00 2001 From: bjoernQ Date: Mon, 5 Feb 2024 17:29:58 +0100 Subject: [PATCH 2/5] Revert "Improve response times" This reverts commit 77c437a252c8accd4e52b228752c03019b50b60b. --- esp-wifi/src/wifi_interface.rs | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/esp-wifi/src/wifi_interface.rs b/esp-wifi/src/wifi_interface.rs index a975122b..7247e084 100644 --- a/esp-wifi/src/wifi_interface.rs +++ b/esp-wifi/src/wifi_interface.rs @@ -452,7 +452,6 @@ impl<'a, MODE: WifiDeviceMode> WifiStack<'a, MODE> { sockets, ) }) { - crate::timer::yield_task(); break; } } @@ -726,9 +725,7 @@ impl<'s, 'n: 's, MODE: WifiDeviceMode> Read for Socket<'s, 'n, MODE> { use smoltcp::socket::tcp::RecvError; loop { - if interface.poll(timestamp(), device, sockets) == false { - crate::timer::yield_task(); - } + interface.poll(timestamp(), device, sockets); let socket = sockets.get_mut::(self.socket_handle); match socket.recv_slice(buf) { @@ -748,14 +745,11 @@ impl<'s, 'n: 's, MODE: WifiDeviceMode> Write for Socket<'s, 'n, MODE> { loop { let (may_send, is_open, can_send) = self.network.with_mut(|interface, device, sockets| { - if interface.poll( + interface.poll( Instant::from_millis((self.network.current_millis_fn)() as i64), device, sockets, - ) == false - { - crate::timer::yield_task(); - } + ); let socket = sockets.get_mut::(self.socket_handle); @@ -802,7 +796,6 @@ impl<'s, 'n: 's, MODE: WifiDeviceMode> Write for Socket<'s, 'n, MODE> { }); if let false = res { - crate::timer::yield_task(); break; } } From 1084673f06eea64d5564156497bc01eee42c836c Mon Sep 17 00:00:00 2001 From: bjoernQ Date: Mon, 5 Feb 2024 18:22:56 +0100 Subject: [PATCH 3/5] Fix embassy_bench --- esp-wifi/examples/embassy_bench.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/esp-wifi/examples/embassy_bench.rs b/esp-wifi/examples/embassy_bench.rs index 13d1a78f..befc6a5c 100644 --- a/esp-wifi/examples/embassy_bench.rs +++ b/esp-wifi/examples/embassy_bench.rs @@ -13,8 +13,8 @@ use examples_util::hal; use embassy_time::{with_timeout, Duration, Timer}; use esp_backtrace as _; use esp_println::println; -use esp_wifi::wifi::{ClientConfiguration, Configuration}; -use esp_wifi::wifi::{WifiApDevice, WifiController, WifiDevice, WifiEvent, WifiState}; +use esp_wifi::wifi::{ClientConfiguration, Configuration, WifiStaDevice}; +use esp_wifi::wifi::{WifiController, WifiDevice, WifiEvent, WifiState}; use esp_wifi::{initialize, EspWifiInitFor}; use hal::clock::ClockControl; use hal::Rng; @@ -60,7 +60,7 @@ async fn main(spawner: Spawner) -> ! { let wifi = peripherals.WIFI; let (wifi_interface, controller) = - esp_wifi::wifi::new_with_mode(&init, wifi, WifiApDevice).unwrap(); + esp_wifi::wifi::new_with_mode(&init, wifi, WifiStaDevice).unwrap(); let timer_group0 = TimerGroup::new(peripherals.TIMG0, &clocks); embassy::init(&clocks, timer_group0); @@ -146,7 +146,7 @@ async fn connection(mut controller: WifiController<'static>) { } #[embassy_executor::task] -async fn net_task(stack: &'static Stack>) { +async fn net_task(stack: &'static Stack>) { stack.run().await } From db1065b4b3933972788269ec8bb608da6ae86a65 Mon Sep 17 00:00:00 2001 From: bjoernQ Date: Mon, 5 Feb 2024 18:26:52 +0100 Subject: [PATCH 4/5] Yield from tx_token/rx_token if no progress can be made otherwise --- esp-wifi/src/wifi/mod.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/esp-wifi/src/wifi/mod.rs b/esp-wifi/src/wifi/mod.rs index 71b9d759..fc17cd9a 100644 --- a/esp-wifi/src/wifi/mod.rs +++ b/esp-wifi/src/wifi/mod.rs @@ -1494,6 +1494,10 @@ mod sealed { } fn tx_token(self) -> Option> { + if !self.can_send() { + crate::timer::yield_task(); + } + if self.can_send() { Some(WifiTxToken { mode: self }) } else { @@ -1503,6 +1507,12 @@ mod sealed { fn rx_token(self) -> Option<(WifiRxToken, WifiTxToken)> { let is_empty = critical_section::with(|cs| self.data_queue_rx(cs).is_empty()); + if is_empty || !self.can_send() { + crate::timer::yield_task(); + } + + let is_empty = + is_empty && critical_section::with(|cs| self.data_queue_rx(cs).is_empty()); if !is_empty { self.tx_token().map(|tx| (WifiRxToken { mode: self }, tx)) From 8fc04ada24fd19ac8bf969ecc3ecf7efdcd97c0a Mon Sep 17 00:00:00 2001 From: bjoernQ Date: Tue, 6 Feb 2024 09:28:05 +0100 Subject: [PATCH 5/5] CHANGELOG.md entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b95a85ef..52e58500 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed - Users don't need embedded-svc to control wifi anymore. The wifi trait is optionally implemented now. (#429) +- Better network performance by forced yielding of the task when buffers are full / empty. (#430) ### Removed