From 8a06c4380db7cb13faa8230507b66863802747e5 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 15 Jun 2025 18:32:36 -0500 Subject: [PATCH 1/3] partition --- esphome/core/application.cpp | 59 +++++++++++++++++++++++++++++++++--- esphome/core/application.h | 25 +++++++++++++++ esphome/core/component.cpp | 8 ++--- esphome/core/component.h | 6 ---- 4 files changed, 83 insertions(+), 15 deletions(-) diff --git a/esphome/core/application.cpp b/esphome/core/application.cpp index 9dda32f0e6..f9d2cf72c6 100644 --- a/esphome/core/application.cpp +++ b/esphome/core/application.cpp @@ -97,11 +97,12 @@ void Application::loop() { // Feed WDT with time this->feed_wdt(last_op_end_time); - for (Component *component : this->looping_components_) { - // Skip components that are done or failed - if (component->should_skip_loop()) { - continue; - } + // Mark that we're in the loop for safe reentrant modifications + this->in_loop_ = true; + + for (this->current_loop_index_ = 0; this->current_loop_index_ < this->looping_components_active_end_; + this->current_loop_index_++) { + Component *component = this->looping_components_[this->current_loop_index_]; // Update the cached time before each component runs this->loop_component_start_time_ = last_op_end_time; @@ -117,6 +118,8 @@ void Application::loop() { this->app_state_ |= new_app_state; this->feed_wdt(last_op_end_time); } + + this->in_loop_ = false; this->app_state_ = new_app_state; // Use the last component's end time instead of calling millis() again @@ -244,6 +247,52 @@ void Application::calculate_looping_components_() { if (obj->has_overridden_loop()) this->looping_components_.push_back(obj); } + // Initially all components are active + this->looping_components_active_end_ = this->looping_components_.size(); +} + +void Application::disable_component_loop(Component *component) { + // Linear search to find component in active section + // Most configs have 10-30 looping components (30 is on the high end) + // O(n) is acceptable here as we optimize for memory, not complexity + for (uint16_t i = 0; i < this->looping_components_active_end_; i++) { + if (this->looping_components_[i] == component) { + // Move last active component to this position + this->looping_components_active_end_--; + if (i != this->looping_components_active_end_) { + this->looping_components_[i] = this->looping_components_[this->looping_components_active_end_]; + this->looping_components_[this->looping_components_active_end_] = component; + + // If we're currently iterating and just swapped the current position + if (this->in_loop_ && i == this->current_loop_index_) { + // Decrement so we'll process the swapped component next + this->current_loop_index_--; + } + } + return; + } + } +} + +void Application::enable_component_loop(Component *component) { + // Single pass through all components to find and move if needed + // With typical 10-30 components, O(n) is faster than maintaining a map + const uint16_t size = this->looping_components_.size(); + for (uint16_t i = 0; i < size; i++) { + if (this->looping_components_[i] == component) { + if (i < this->looping_components_active_end_) { + return; // Already active + } + // Found in inactive section - move to active + if (i != this->looping_components_active_end_) { + Component *temp = this->looping_components_[this->looping_components_active_end_]; + this->looping_components_[this->looping_components_active_end_] = component; + this->looping_components_[i] = temp; + } + this->looping_components_active_end_++; + return; + } + } } #ifdef USE_SOCKET_SELECT_SUPPORT diff --git a/esphome/core/application.h b/esphome/core/application.h index d9ef4fe036..8b2f78beaa 100644 --- a/esphome/core/application.h +++ b/esphome/core/application.h @@ -572,13 +572,38 @@ class Application { void calculate_looping_components_(); + void disable_component_loop(Component *component); + void enable_component_loop(Component *component); + void feed_wdt_arch_(); /// Perform a delay while also monitoring socket file descriptors for readiness void yield_with_select_(uint32_t delay_ms); std::vector components_{}; + + // Partitioned vector design for looping components + // ================================================= + // Components are partitioned into [active | inactive] sections: + // + // looping_components_: [A, B, C, D | E, F] + // ^ + // looping_components_active_end_ (4) + // + // - Components A,B,C,D are active and will be called in loop() + // - Components E,F are inactive (disabled/failed) and won't be called + // - No flag checking needed during iteration - just loop 0 to active_end_ + // - When a component is disabled, it's swapped with the last active component + // and active_end_ is decremented + // - When a component is enabled, it's swapped with the first inactive component + // and active_end_ is incremented + // - This eliminates branch mispredictions from flag checking in the hot loop std::vector looping_components_{}; + uint16_t looping_components_active_end_{0}; + + // For safe reentrant modifications during iteration + uint16_t current_loop_index_{0}; + bool in_loop_{false}; #ifdef USE_BINARY_SENSOR std::vector binary_sensors_{}; diff --git a/esphome/core/component.cpp b/esphome/core/component.cpp index 14deb9c1df..53e57cea6d 100644 --- a/esphome/core/component.cpp +++ b/esphome/core/component.cpp @@ -136,17 +136,21 @@ void Component::mark_failed() { this->component_state_ &= ~COMPONENT_STATE_MASK; this->component_state_ |= COMPONENT_STATE_FAILED; this->status_set_error(); + // Also remove from loop since failed components shouldn't loop + App.disable_component_loop(this); } void Component::disable_loop() { ESP_LOGD(TAG, "%s loop disabled", this->get_component_source()); this->component_state_ &= ~COMPONENT_STATE_MASK; this->component_state_ |= COMPONENT_STATE_LOOP_DONE; + App.disable_component_loop(this); } void Component::enable_loop() { if ((this->component_state_ & COMPONENT_STATE_MASK) == COMPONENT_STATE_LOOP_DONE) { ESP_LOGD(TAG, "%s loop enabled", this->get_component_source()); this->component_state_ &= ~COMPONENT_STATE_MASK; this->component_state_ |= COMPONENT_STATE_LOOP; + App.enable_component_loop(this); } } void Component::reset_to_construction_state() { @@ -185,10 +189,6 @@ bool Component::is_ready() const { return (this->component_state_ & COMPONENT_STATE_MASK) == COMPONENT_STATE_LOOP || (this->component_state_ & COMPONENT_STATE_MASK) == COMPONENT_STATE_SETUP; } -bool Component::should_skip_loop() const { - uint8_t state = this->component_state_ & COMPONENT_STATE_MASK; - return state == COMPONENT_STATE_FAILED || state == COMPONENT_STATE_LOOP_DONE; -} bool Component::can_proceed() { return true; } bool Component::status_has_warning() const { return this->component_state_ & STATUS_LED_WARNING; } bool Component::status_has_error() const { return this->component_state_ & STATUS_LED_ERROR; } diff --git a/esphome/core/component.h b/esphome/core/component.h index 8ce2e87049..f787520026 100644 --- a/esphome/core/component.h +++ b/esphome/core/component.h @@ -169,12 +169,6 @@ class Component { bool is_ready() const; - /** Check if this component should skip its loop execution. - * - * @return True if the component is in FAILED or LOOP_DONE state - */ - bool should_skip_loop() const; - virtual bool can_proceed(); bool status_has_warning() const; From cee7789ab64a90b978d0ac271f61fd4b9f04648f Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 15 Jun 2025 18:37:05 -0500 Subject: [PATCH 2/3] tweak --- esphome/core/application.h | 3 +++ esphome/core/component.h | 6 ++++++ 2 files changed, 9 insertions(+) diff --git a/esphome/core/application.h b/esphome/core/application.h index 8b2f78beaa..46330cb2ae 100644 --- a/esphome/core/application.h +++ b/esphome/core/application.h @@ -572,6 +572,9 @@ class Application { void calculate_looping_components_(); + // These methods are called by Component::disable_loop() and Component::enable_loop() + // Components should not call these directly - use this->disable_loop() or this->enable_loop() + // to ensure component state is properly updated along with the loop partition void disable_component_loop(Component *component); void enable_component_loop(Component *component); diff --git a/esphome/core/component.h b/esphome/core/component.h index f787520026..e2adb66c47 100644 --- a/esphome/core/component.h +++ b/esphome/core/component.h @@ -155,6 +155,9 @@ class Component { * * This is useful for components that only need to run for a certain period of time * or when inactive, saving CPU cycles. + * + * @note Components should call this->disable_loop() on themselves, not on other components. + * This ensures the component's state is properly updated along with the loop partition. */ void disable_loop(); @@ -162,6 +165,9 @@ class Component { * * This is useful for components that transition between active and inactive states * and need to re-enable their loop() method when becoming active again. + * + * @note Components should call this->enable_loop() on themselves, not on other components. + * This ensures the component's state is properly updated along with the loop partition. */ void enable_loop(); From f711706b1acf85559ae5723615b9b8a86862e91a Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 15 Jun 2025 18:40:08 -0500 Subject: [PATCH 3/3] Fix ESP32 Improv component to re-enable loop when service starts again --- esphome/components/esp32_improv/esp32_improv_component.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/esphome/components/esp32_improv/esp32_improv_component.cpp b/esphome/components/esp32_improv/esp32_improv_component.cpp index ff150a3d69..d41094fda1 100644 --- a/esphome/components/esp32_improv/esp32_improv_component.cpp +++ b/esphome/components/esp32_improv/esp32_improv_component.cpp @@ -256,6 +256,7 @@ void ESP32ImprovComponent::start() { ESP_LOGD(TAG, "Setting Improv to start"); this->should_start_ = true; + this->enable_loop(); } void ESP32ImprovComponent::stop() {