diff --git a/esphome/core/scheduler.cpp b/esphome/core/scheduler.cpp index 44fc277ec82..51cbfb208ea 100644 --- a/esphome/core/scheduler.cpp +++ b/esphome/core/scheduler.cpp @@ -138,7 +138,8 @@ void HOT Scheduler::set_timer_common_(Component *component, SchedulerItem::Type // Still need to cancel existing timer if we have a name/id if (!skip_cancel) { LockGuard guard{this->lock_}; - this->cancel_item_locked_(component, name_type, static_name, hash_or_id, type); + this->cancel_item_locked_(component, name_type, static_name, hash_or_id, type, /* match_retry= */ false, + /* find_first= */ true); } return; } @@ -209,7 +210,8 @@ void HOT Scheduler::set_timer_common_(Component *component, SchedulerItem::Type // Common epilogue: atomic cancel-and-add (unless skip_cancel is true) if (!skip_cancel) { - this->cancel_item_locked_(component, name_type, static_name, hash_or_id, type); + this->cancel_item_locked_(component, name_type, static_name, hash_or_id, type, /* match_retry= */ false, + /* find_first= */ true); } target->push_back(item); if (target == &this->to_add_) { @@ -723,13 +725,20 @@ uint32_t HOT Scheduler::execute_item_(SchedulerItem *item, uint32_t now) { bool HOT Scheduler::cancel_item_(Component *component, NameType name_type, const char *static_name, uint32_t hash_or_id, SchedulerItem::Type type, bool match_retry) { LockGuard guard{this->lock_}; + // Public cancel path uses default find_first=false to cancel ALL matches because + // DelayAction parallel mode (skip_cancel=true) can create multiple items with the same key. return this->cancel_item_locked_(component, name_type, static_name, hash_or_id, type, match_retry); } -// Helper to cancel items - must be called with lock held +// Helper to cancel matching items - must be called with lock held. +// When find_first=true, stops after the first match and exits across containers +// (used by set_timer_common_ where cancel-before-add guarantees at most one match). +// When find_first=false, cancels ALL matches across all containers (needed for +// public cancel path where DelayAction parallel mode can create duplicates). // name_type determines matching: STATIC_STRING uses static_name, others use hash_or_id bool HOT Scheduler::cancel_item_locked_(Component *component, NameType name_type, const char *static_name, - uint32_t hash_or_id, SchedulerItem::Type type, bool match_retry) { + uint32_t hash_or_id, SchedulerItem::Type type, bool match_retry, + bool find_first) { // Early return if static string name is invalid if (name_type == NameType::STATIC_STRING && static_name == nullptr) { return false; @@ -741,7 +750,9 @@ bool HOT Scheduler::cancel_item_locked_(Component *component, NameType name_type // Mark items in defer queue as cancelled (they'll be skipped when processed) if (type == SchedulerItem::TIMEOUT) { total_cancelled += this->mark_matching_items_removed_locked_(this->defer_queue_, component, name_type, static_name, - hash_or_id, type, match_retry); + hash_or_id, type, match_retry, find_first); + if (find_first && total_cancelled > 0) + return true; } #endif /* not ESPHOME_THREAD_SINGLE */ @@ -752,14 +763,16 @@ bool HOT Scheduler::cancel_item_locked_(Component *component, NameType name_type // Only the main loop in call() should recycle items after execution completes. if (!this->items_.empty()) { size_t heap_cancelled = this->mark_matching_items_removed_locked_(this->items_, component, name_type, static_name, - hash_or_id, type, match_retry); + hash_or_id, type, match_retry, find_first); total_cancelled += heap_cancelled; this->to_remove_add_(heap_cancelled); + if (find_first && total_cancelled > 0) + return true; } // Cancel items in to_add_ total_cancelled += this->mark_matching_items_removed_locked_(this->to_add_, component, name_type, static_name, - hash_or_id, type, match_retry); + hash_or_id, type, match_retry, find_first); return total_cancelled > 0; } diff --git a/esphome/core/scheduler.h b/esphome/core/scheduler.h index 36c853ad176..1e44f41da84 100644 --- a/esphome/core/scheduler.h +++ b/esphome/core/scheduler.h @@ -320,10 +320,14 @@ class Scheduler { SchedulerItem *get_item_from_pool_locked_(); private: - // Helper to cancel items - must be called with lock held + // Helper to cancel matching items - must be called with lock held. + // When find_first=true, stops after the first match (used by set_timer_common_ where + // the cancel-before-add invariant guarantees at most one match). + // When find_first=false (default), cancels ALL matches (needed for DelayAction parallel + // mode where skip_cancel=true allows multiple items with the same key). // name_type determines matching: STATIC_STRING uses static_name, others use hash_or_id bool cancel_item_locked_(Component *component, NameType name_type, const char *static_name, uint32_t hash_or_id, - SchedulerItem::Type type, bool match_retry = false); + SchedulerItem::Type type, bool match_retry = false, bool find_first = false); // Common implementation for cancel operations - handles locking bool cancel_item_(Component *component, NameType name_type, const char *static_name, uint32_t hash_or_id, @@ -483,18 +487,25 @@ class Scheduler { #endif } - // Helper to mark matching items in a container as removed + // Helper to mark matching items in a container as removed. + // When find_first=true, stops after the first match (used by set_timer_common_ where + // the cancel-before-add invariant guarantees at most one match). + // When find_first=false, marks ALL matches (needed for public cancel path where + // DelayAction parallel mode with skip_cancel=true can create multiple items with the same key). // name_type determines matching: STATIC_STRING uses static_name, others use hash_or_id - // Returns the number of items marked for removal + // Returns the number of items marked for removal. // IMPORTANT: Must be called with scheduler lock held __attribute__((noinline)) size_t mark_matching_items_removed_locked_(std::vector &container, Component *component, NameType name_type, const char *static_name, uint32_t hash_or_id, - SchedulerItem::Type type, bool match_retry) { + SchedulerItem::Type type, bool match_retry, + bool find_first = false) { size_t count = 0; for (auto *item : container) { if (this->matches_item_locked_(item, component, name_type, static_name, hash_or_id, type, match_retry)) { this->set_item_removed_(item, true); + if (find_first) + return 1; count++; } } diff --git a/tests/benchmarks/core/bench_scheduler.cpp b/tests/benchmarks/core/bench_scheduler.cpp index 764f17ed735..9357734cc8e 100644 --- a/tests/benchmarks/core/bench_scheduler.cpp +++ b/tests/benchmarks/core/bench_scheduler.cpp @@ -99,11 +99,21 @@ BENCHMARK(Scheduler_SetTimeout); static void Scheduler_SetInterval(benchmark::State &state) { Scheduler scheduler; Component dummy_component; + // Number of distinct interval keys; controls how many unique timers exist + // simultaneously and the drain cadence for process_to_add(). + static constexpr int kKeyCount = 5; for (auto _ : state) { for (int i = 0; i < kInnerIterations; i++) { - scheduler.set_interval(&dummy_component, static_cast(i % 5), 1000, []() {}); + scheduler.set_interval(&dummy_component, static_cast(i % kKeyCount), 1000, []() {}); + // Drain to_add_ periodically to reflect production behavior where + // process_to_add() runs each main loop iteration. Without this, + // cancelled items accumulate in to_add_ causing O(n²) scan cost. + if ((i + 1) % kKeyCount == 0) { + scheduler.process_to_add(); + } } + // Final drain in case kInnerIterations is not a multiple of 5 scheduler.process_to_add(); benchmark::DoNotOptimize(scheduler); }