mirror of
https://github.com/esphome/esphome.git
synced 2026-09-19 02:58:38 +00:00
[core] Remove pre-sleep socket scan from fast select path
The pre-sleep scan in Application::yield_with_select_() walks monitored_sockets_ on every loop iteration, issuing a volatile cross-thread read on each socket's lwip_sock::rcvevent to preserve select() semantics when the FreeRTOS task notification counter had been consumed but a socket still had unread data. That scenario only existed because of a Socket::ready() contract violation: callers could stop reading with rcvevent > 0, leaving data behind with no pending notification. That contract is now documented and enforced (#15590), and #15589 (the first failure that reverted the earlier removal attempt #14475) has been fixed. With the contract honoured, every rcvevent > 0 is paired with a pending xTaskNotifyGive from the lwip event_callback wrapper (see lwip_fast_select.c). ulTaskNotifyTake either returns immediately (counter non-zero) or wakes the moment the notify lands — the scan has nothing left to rescue. Evidence: https://github.com/esphome/esphome/pull/15638 — an instrumentation PR ran across 5 devices (ESP32 rev1/rev3.1/C3 on Ethernet and WiFi, plus LibreTiny RTL8720CF) through Home Assistant disconnect/reconnect cycles, multi-client API logger bursts, and BLE GATT connect storms. Across ~275,000 scans and 4 observed load-bearing candidates, every hit was in the 2–14µs range — the instruction-level window between the lwip callback writing rcvevent and calling xTaskNotifyGive a few instructions later. Zero hits exceeded 100µs. No hit came anywhere near loop_interval (16ms), which is the latency scale the scan was added to prevent. In addition to being unused, the scan is actively harmful on the hot path: N volatile 16-bit loads against cache-cold cross-thread lwip_sock structures on every main-loop iteration, just to reproduce a microsecond-scale ordering artifact the notification path is already handling authoritatively. This also removes the now-unused monitored_sockets_ vector and Application::{register,unregister}_socket() on the fast-select path. Socket implementations now call esphome_lwip_hook_socket() directly to install the netconn event callback wrapper.
This commit is contained in:
@@ -14,9 +14,13 @@ BSDSocketImpl::BSDSocketImpl(int fd, bool monitor_loop) {
|
||||
if (!monitor_loop || this->fd_ < 0)
|
||||
return;
|
||||
#ifdef USE_LWIP_FAST_SELECT
|
||||
// Cache lwip_sock pointer and register for monitoring (hooks callback internally)
|
||||
// Cache lwip_sock pointer (used by ready() for direct rcvevent reads) and hook the
|
||||
// netconn event callback so the main loop is notified via FreeRTOS task notifications.
|
||||
this->cached_sock_ = esphome_lwip_get_sock(this->fd_);
|
||||
this->loop_monitored_ = App.register_socket(this->cached_sock_);
|
||||
if (this->cached_sock_ != nullptr) {
|
||||
esphome_lwip_hook_socket(this->cached_sock_);
|
||||
this->loop_monitored_ = true;
|
||||
}
|
||||
#else
|
||||
this->loop_monitored_ = App.register_socket_fd(this->fd_);
|
||||
#endif
|
||||
@@ -30,12 +34,10 @@ BSDSocketImpl::~BSDSocketImpl() {
|
||||
|
||||
int BSDSocketImpl::close() {
|
||||
if (!this->closed_) {
|
||||
// Unregister before closing to avoid dangling pointer in monitored set
|
||||
#ifdef USE_LWIP_FAST_SELECT
|
||||
if (this->loop_monitored_) {
|
||||
App.unregister_socket(this->cached_sock_);
|
||||
this->cached_sock_ = nullptr;
|
||||
}
|
||||
// All LwIP sockets share the same static event_callback, so there is no per-socket
|
||||
// unhook needed — just drop the cached pointer before the socket is destroyed.
|
||||
this->cached_sock_ = nullptr;
|
||||
#else
|
||||
if (this->loop_monitored_) {
|
||||
App.unregister_socket_fd(this->fd_);
|
||||
|
||||
@@ -14,9 +14,13 @@ LwIPSocketImpl::LwIPSocketImpl(int fd, bool monitor_loop) {
|
||||
if (!monitor_loop || this->fd_ < 0)
|
||||
return;
|
||||
#ifdef USE_LWIP_FAST_SELECT
|
||||
// Cache lwip_sock pointer and register for monitoring (hooks callback internally)
|
||||
// Cache lwip_sock pointer (used by ready() for direct rcvevent reads) and hook the
|
||||
// netconn event callback so the main loop is notified via FreeRTOS task notifications.
|
||||
this->cached_sock_ = esphome_lwip_get_sock(this->fd_);
|
||||
this->loop_monitored_ = App.register_socket(this->cached_sock_);
|
||||
if (this->cached_sock_ != nullptr) {
|
||||
esphome_lwip_hook_socket(this->cached_sock_);
|
||||
this->loop_monitored_ = true;
|
||||
}
|
||||
#else
|
||||
this->loop_monitored_ = App.register_socket_fd(this->fd_);
|
||||
#endif
|
||||
@@ -30,12 +34,10 @@ LwIPSocketImpl::~LwIPSocketImpl() {
|
||||
|
||||
int LwIPSocketImpl::close() {
|
||||
if (!this->closed_) {
|
||||
// Unregister before closing to avoid dangling pointer in monitored set
|
||||
#ifdef USE_LWIP_FAST_SELECT
|
||||
if (this->loop_monitored_) {
|
||||
App.unregister_socket(this->cached_sock_);
|
||||
this->cached_sock_ = nullptr;
|
||||
}
|
||||
// All LwIP sockets share the same static event_callback, so there is no per-socket
|
||||
// unhook needed — just drop the cached pointer before the socket is destroyed.
|
||||
this->cached_sock_ = nullptr;
|
||||
#else
|
||||
if (this->loop_monitored_) {
|
||||
App.unregister_socket_fd(this->fd_);
|
||||
|
||||
@@ -449,32 +449,7 @@ void Application::enable_pending_loops_() {
|
||||
}
|
||||
}
|
||||
|
||||
#ifdef USE_LWIP_FAST_SELECT
|
||||
bool Application::register_socket(struct lwip_sock *sock) {
|
||||
// It modifies monitored_sockets_ without locking — must only be called from the main loop.
|
||||
if (sock == nullptr)
|
||||
return false;
|
||||
esphome_lwip_hook_socket(sock);
|
||||
this->monitored_sockets_.push_back(sock);
|
||||
return true;
|
||||
}
|
||||
|
||||
void Application::unregister_socket(struct lwip_sock *sock) {
|
||||
// It modifies monitored_sockets_ without locking — must only be called from the main loop.
|
||||
for (size_t i = 0; i < this->monitored_sockets_.size(); i++) {
|
||||
if (this->monitored_sockets_[i] != sock)
|
||||
continue;
|
||||
|
||||
// Swap with last element and pop - O(1) removal since order doesn't matter.
|
||||
// No need to unhook the netconn callback — all LwIP sockets share the same
|
||||
// static event_callback, and the socket will be closed by the caller.
|
||||
if (i < this->monitored_sockets_.size() - 1)
|
||||
this->monitored_sockets_[i] = this->monitored_sockets_.back();
|
||||
this->monitored_sockets_.pop_back();
|
||||
return;
|
||||
}
|
||||
}
|
||||
#elif defined(USE_HOST)
|
||||
#ifdef USE_HOST
|
||||
bool Application::register_socket_fd(int fd) {
|
||||
// WARNING: This function is NOT thread-safe and must only be called from the main loop
|
||||
// It modifies socket_fds_ and related variables without locking
|
||||
|
||||
@@ -534,12 +534,7 @@ class Application {
|
||||
|
||||
/// Register/unregister a socket to be monitored for read events.
|
||||
/// WARNING: These functions are NOT thread-safe. They must only be called from the main loop.
|
||||
#ifdef USE_LWIP_FAST_SELECT
|
||||
/// Fast select path: hooks netconn callback and registers for monitoring.
|
||||
/// @return true if registration was successful, false if sock is null
|
||||
bool register_socket(struct lwip_sock *sock);
|
||||
void unregister_socket(struct lwip_sock *sock);
|
||||
#elif defined(USE_HOST)
|
||||
#ifdef USE_HOST
|
||||
/// Fallback select() path: monitors file descriptors.
|
||||
/// NOTE: File descriptors >= FD_SETSIZE (typically 10 on ESP) will be rejected with an error.
|
||||
/// @return true if registration was successful, false if fd exceeds limits
|
||||
@@ -653,9 +648,7 @@ class Application {
|
||||
// and active_end_ is incremented
|
||||
// - This eliminates branch mispredictions from flag checking in the hot loop
|
||||
FixedVector<Component *> looping_components_{};
|
||||
#ifdef USE_LWIP_FAST_SELECT
|
||||
std::vector<struct lwip_sock *> monitored_sockets_; // Cached lwip_sock pointers for direct rcvevent read
|
||||
#elif defined(USE_HOST)
|
||||
#ifdef USE_HOST
|
||||
std::vector<int> socket_fds_; // Vector of all monitored socket file descriptors
|
||||
#endif
|
||||
#ifdef USE_HOST
|
||||
@@ -898,26 +891,16 @@ inline void ESPHOME_ALWAYS_INLINE Application::loop() {
|
||||
#ifndef USE_HOST
|
||||
inline void ESPHOME_ALWAYS_INLINE Application::yield_with_select_(uint32_t delay_ms) {
|
||||
#ifdef USE_LWIP_FAST_SELECT
|
||||
// Fast path (ESP32/LibreTiny): reads rcvevent directly from cached lwip_sock pointers.
|
||||
// Safe because this runs on the main loop which owns socket lifetime (create, read, close).
|
||||
// Fast path (ESP32/LibreTiny): FreeRTOS task notifications posted by the lwip
|
||||
// event_callback wrapper (see lwip_fast_select.c) are the single source of truth for
|
||||
// socket wake-ups. Every NETCONN_EVT_RCVPLUS posts an xTaskNotifyGive, so any notification
|
||||
// that lands between wakes keeps the counter non-zero (next ulTaskNotifyTake returns
|
||||
// immediately) or wakes a blocked Take directly. Also woken by wake_loop_threadsafe()
|
||||
// from background tasks, or timeout.
|
||||
if (delay_ms == 0) [[unlikely]] {
|
||||
yield();
|
||||
return;
|
||||
}
|
||||
|
||||
// Check if any socket already has pending data before sleeping.
|
||||
// If a socket still has unread data (rcvevent > 0) but the task notification was already
|
||||
// consumed, ulTaskNotifyTake would block until timeout — adding up to delay_ms latency.
|
||||
// This scan preserves select() semantics: return immediately when any fd is ready.
|
||||
for (struct lwip_sock *sock : this->monitored_sockets_) {
|
||||
if (esphome_lwip_socket_has_data(sock)) {
|
||||
yield();
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
// Sleep with instant wake via FreeRTOS task notification.
|
||||
// Woken by: callback wrapper (socket data), wake_loop_threadsafe() (background tasks), or timeout.
|
||||
#endif
|
||||
esphome::internal::wakeable_delay(delay_ms);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user