From 89b9bddf1b23888f46f713714d562c149744d1b7 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Fri, 18 Jul 2025 22:55:21 -1000 Subject: [PATCH 1/9] [CI] Fix clang-tidy not running when platformio.ini changes (#9678) --- script/determine-jobs.py | 10 +++++++++ tests/script/test_determine_jobs.py | 33 +++++++++++++++++++++++------ 2 files changed, 36 insertions(+), 7 deletions(-) diff --git a/script/determine-jobs.py b/script/determine-jobs.py index fc5c397c65..e26bc29c2f 100755 --- a/script/determine-jobs.py +++ b/script/determine-jobs.py @@ -137,6 +137,10 @@ def should_run_clang_tidy(branch: str | None = None) -> bool: - This ensures all C++ code is checked, including tests, examples, etc. - Examples: esphome/core/component.cpp, tests/custom/my_component.h + 3. The .clang-tidy.hash file itself changed + - This indicates the configuration has been updated and clang-tidy should run + - Ensures that PRs updating the clang-tidy configuration are properly validated + If the hash check fails for any reason, clang-tidy runs as a safety measure to ensure code quality is maintained. @@ -160,6 +164,12 @@ def should_run_clang_tidy(branch: str | None = None) -> bool: # If hash check fails, run clang-tidy to be safe return True + # Check if .clang-tidy.hash file itself was changed + # This handles the case where the hash was properly updated in the PR + files = changed_files(branch) + if ".clang-tidy.hash" in files: + return True + return _any_changed_file_endswith(branch, CPP_FILE_EXTENSIONS) diff --git a/tests/script/test_determine_jobs.py b/tests/script/test_determine_jobs.py index 4aaaadd80a..84be7344c3 100644 --- a/tests/script/test_determine_jobs.py +++ b/tests/script/test_determine_jobs.py @@ -6,7 +6,7 @@ import json import os import subprocess import sys -from unittest.mock import Mock, patch +from unittest.mock import Mock, call, patch import pytest @@ -262,6 +262,8 @@ def test_should_run_integration_tests_component_dependency() -> None: (0, [], True), # Hash changed - need full scan (1, ["esphome/core.cpp"], True), # C++ file changed (1, ["README.md"], False), # No C++ files changed + (1, [".clang-tidy.hash"], True), # Hash file itself changed + (1, ["platformio.ini", ".clang-tidy.hash"], True), # Config + hash changed ], ) def test_should_run_clang_tidy( @@ -277,11 +279,26 @@ def test_should_run_clang_tidy( result = determine_jobs.should_run_clang_tidy() assert result == expected_result - # Test with hash check failing (exception) - if check_returncode != 0: - with patch("subprocess.run", side_effect=Exception("Failed")): - result = determine_jobs.should_run_clang_tidy() - assert result is True # Fail safe - run clang-tidy + +def test_should_run_clang_tidy_hash_check_exception() -> None: + """Test should_run_clang_tidy when hash check fails with exception.""" + # When hash check fails, clang-tidy should run as a safety measure + with ( + patch.object(determine_jobs, "changed_files", return_value=["README.md"]), + patch("subprocess.run", side_effect=Exception("Hash check failed")), + ): + result = determine_jobs.should_run_clang_tidy() + assert result is True # Fail safe - run clang-tidy + + # Even with C++ files, exception should trigger clang-tidy + with ( + patch.object( + determine_jobs, "changed_files", return_value=["esphome/core.cpp"] + ), + patch("subprocess.run", side_effect=Exception("Hash check failed")), + ): + result = determine_jobs.should_run_clang_tidy() + assert result is True def test_should_run_clang_tidy_with_branch() -> None: @@ -291,7 +308,9 @@ def test_should_run_clang_tidy_with_branch() -> None: with patch("subprocess.run") as mock_run: mock_run.return_value = Mock(returncode=1) # Hash unchanged determine_jobs.should_run_clang_tidy("release") - mock_changed.assert_called_once_with("release") + # Changed files is called twice now - once for hash check, once for .clang-tidy.hash check + assert mock_changed.call_count == 2 + mock_changed.assert_has_calls([call("release"), call("release")]) @pytest.mark.parametrize( From 211739bba005d494354d9a6fcb061f26b7991059 Mon Sep 17 00:00:00 2001 From: RubenKelevra Date: Fri, 18 Jul 2025 08:12:00 +0200 Subject: [PATCH 2/9] core/scheduler: Make millis_64_ rollover monotonic on SMP MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The current implementation uses only memory_order_relaxed on all atomic accesses. That protects each variable individually but not the semantic link between the low word (last_millis_) and the high-word epoch counter (millis_major_). On a multi-core target a reader could observe a freshly stored low word before seeing the matching increment of the epoch, causing a ~49-day negative jump. Key fixes - Release/acquire pairing - writer: compare_exchange_weak(..., memory_order_release, …) - reader: first load of last_millis_ now uses memory_order_acquire - ensures any core that sees the new low word also sees the updated high word - Epoch-coherency retry loop - re-loads millis_major_ after the update and retries if it changed, guaranteeing monotonicity even when another core rolls over concurrently - millis_major_ promoted to std::atomic on SMP platforms - removes the formal data race at negligible cost - new macros for better readability - ESPHOME_SINGLE_CORE – currently ESP8266/RP2040 only - ESPHOME_ATOMIC_SCHEDULER – all others except LibreTiny - Logging and comments - loads atomics safely in debug output - updated inline docs to match the memory ordering Behavior on single-core or non-atomic platforms is unchanged; multi-core targets now get a provably monotonic 64-bit millisecond clock with minimal overhead. --- esphome/core/defines.h | 10 +++ esphome/core/scheduler.cpp | 166 ++++++++++++++++++++++--------------- esphome/core/scheduler.h | 40 ++++++--- 3 files changed, 136 insertions(+), 80 deletions(-) diff --git a/esphome/core/defines.h b/esphome/core/defines.h index 7ddb3436cd..4e3145444d 100644 --- a/esphome/core/defines.h +++ b/esphome/core/defines.h @@ -229,6 +229,16 @@ #define USE_SOCKET_SELECT_SUPPORT #endif +// Helper macro for platforms that lack atomic scheduler support +#if defined(USE_ESP8266) || defined(USE_RP2040) +#define ESPHOME_SINGLE_CORE +#endif + +// Helper macro for platforms with atomic scheduler support +#if !defined(ESPHOME_SINGLE_CORE) && !defined(USE_LIBRETINY) +#define ESPHOME_ATOMIC_SCHEDULER +#endif + // Disabled feature flags // #define USE_BSEC // Requires a library with proprietary license // #define USE_BSEC2 // Requires a library with proprietary license diff --git a/esphome/core/scheduler.cpp b/esphome/core/scheduler.cpp index 7a0c08e1f0..21c45c2f97 100644 --- a/esphome/core/scheduler.cpp +++ b/esphome/core/scheduler.cpp @@ -54,7 +54,7 @@ static void validate_static_string(const char *name) { ESP_LOGW(TAG, "WARNING: Scheduler name '%s' at %p might be on heap (static ref at %p)", name, name, static_str); } } -#endif +#endif /* ESPHOME_DEBUG_SCHEDULER */ // A note on locking: the `lock_` lock protects the `items_` and `to_add_` containers. It must be taken when writing to // them (i.e. when adding/removing items, but not when changing items). As items are only deleted from the loop task, @@ -82,9 +82,9 @@ void HOT Scheduler::set_timer_common_(Component *component, SchedulerItem::Type item->callback = std::move(func); item->remove = false; -#if !defined(USE_ESP8266) && !defined(USE_RP2040) +#ifndef ESPHOME_SINGLE_CORE // Special handling for defer() (delay = 0, type = TIMEOUT) - // ESP8266 and RP2040 are excluded because they don't need thread-safe defer handling + // Single-core platforms don't need thread-safe defer handling if (delay == 0 && type == SchedulerItem::TIMEOUT) { // Put in defer queue for guaranteed FIFO execution LockGuard guard{this->lock_}; @@ -92,7 +92,7 @@ void HOT Scheduler::set_timer_common_(Component *component, SchedulerItem::Type this->defer_queue_.push_back(std::move(item)); return; } -#endif +#endif /* not ESPHOME_SINGLE_CORE */ // Get fresh timestamp for new timer/interval - ensures accurate scheduling const auto now = this->millis_64_(millis()); // Fresh millis() call @@ -123,7 +123,7 @@ void HOT Scheduler::set_timer_common_(Component *component, SchedulerItem::Type ESP_LOGD(TAG, "set_%s(name='%s/%s', %s=%" PRIu32 ", offset=%" PRIu32 ")", type_str, item->get_source(), name_cstr ? name_cstr : "(null)", type_str, delay, static_cast(item->next_execution_ - now)); } -#endif +#endif /* ESPHOME_DEBUG_SCHEDULER */ LockGuard guard{this->lock_}; // If name is provided, do atomic cancel-and-add @@ -231,7 +231,7 @@ optional HOT Scheduler::next_schedule_in(uint32_t now) { return item->next_execution_ - now_64; } void HOT Scheduler::call(uint32_t now) { -#if !defined(USE_ESP8266) && !defined(USE_RP2040) +#ifndef ESPHOME_SINGLE_CORE // Process defer queue first to guarantee FIFO execution order for deferred items. // Previously, defer() used the heap which gave undefined order for equal timestamps, // causing race conditions on multi-core systems (ESP32, BK7200). @@ -239,8 +239,7 @@ void HOT Scheduler::call(uint32_t now) { // - Deferred items (delay=0) go directly to defer_queue_ in set_timer_common_ // - Items execute in exact order they were deferred (FIFO guarantee) // - No deferred items exist in to_add_, so processing order doesn't affect correctness - // ESP8266 and RP2040 don't use this queue - they fall back to the heap-based approach - // (ESP8266: single-core, RP2040: empty mutex implementation). + // Single-core platforms don't use this queue and fall back to the heap-based approach. // // Note: Items cancelled via cancel_item_locked_() are marked with remove=true but still // processed here. They are removed from the queue normally via pop_front() but skipped @@ -262,7 +261,7 @@ void HOT Scheduler::call(uint32_t now) { this->execute_item_(item.get(), now); } } -#endif +#endif /* not ESPHOME_SINGLE_CORE */ // Convert the fresh timestamp from main loop to 64-bit for scheduler operations const auto now_64 = this->millis_64_(now); // 'now' from parameter - fresh from Application::loop() @@ -274,13 +273,15 @@ void HOT Scheduler::call(uint32_t now) { if (now_64 - last_print > 2000) { last_print = now_64; std::vector> old_items; -#if !defined(USE_ESP8266) && !defined(USE_RP2040) && !defined(USE_LIBRETINY) - ESP_LOGD(TAG, "Items: count=%zu, now=%" PRIu64 " (%u, %" PRIu32 ")", this->items_.size(), now_64, - this->millis_major_, this->last_millis_.load(std::memory_order_relaxed)); -#else - ESP_LOGD(TAG, "Items: count=%zu, now=%" PRIu64 " (%u, %" PRIu32 ")", this->items_.size(), now_64, +#ifdef ESPHOME_ATOMIC_SCHEDULER + const auto last_dbg = this->last_millis_.load(std::memory_order_relaxed); + const auto major_dbg = this->millis_major_.load(std::memory_order_relaxed); + ESP_LOGD(TAG, "Items: count=%zu, now=%" PRIu64 " (%" PRIu16 ", %" PRIu32 ")", this->items_.size(), now_64, + major_dbg, last_dbg); +#else /* not ESPHOME_ATOMIC_SCHEDULER */ + ESP_LOGD(TAG, "Items: count=%zu, now=%" PRIu64 " (%" PRIu16 ", %" PRIu32 ")", this->items_.size(), now_64, this->millis_major_, this->last_millis_); -#endif +#endif /* else ESPHOME_ATOMIC_SCHEDULER */ while (!this->empty_()) { std::unique_ptr item; { @@ -305,7 +306,7 @@ void HOT Scheduler::call(uint32_t now) { std::make_heap(this->items_.begin(), this->items_.end(), SchedulerItem::cmp); } } -#endif // ESPHOME_DEBUG_SCHEDULER +#endif /* ESPHOME_DEBUG_SCHEDULER */ // If we have too many items to remove if (this->to_remove_ > MAX_LOGICALLY_DELETED_ITEMS) { @@ -352,7 +353,7 @@ void HOT Scheduler::call(uint32_t now) { ESP_LOGV(TAG, "Running %s '%s/%s' with interval=%" PRIu32 " next_execution=%" PRIu64 " (now=%" PRIu64 ")", item->get_type_str(), item->get_source(), item_name ? item_name : "(null)", item->interval, item->next_execution_, now_64); -#endif +#endif /* ESPHOME_DEBUG_SCHEDULER */ // Warning: During callback(), a lot of stuff can happen, including: // - timeouts/intervals get added, potentially invalidating vector pointers @@ -460,7 +461,7 @@ bool HOT Scheduler::cancel_item_locked_(Component *component, const char *name_c size_t total_cancelled = 0; // Check all containers for matching items -#if !defined(USE_ESP8266) && !defined(USE_RP2040) +#ifndef ESPHOME_SINGLE_CORE // Only check defer queue for timeouts (intervals never go there) if (type == SchedulerItem::TIMEOUT) { for (auto &item : this->defer_queue_) { @@ -470,7 +471,7 @@ bool HOT Scheduler::cancel_item_locked_(Component *component, const char *name_c } } } -#endif +#endif /* not ESPHOME_SINGLE_CORE */ // Cancel items in the main heap for (auto &item : this->items_) { @@ -496,7 +497,7 @@ bool HOT Scheduler::cancel_item_locked_(Component *component, const char *name_c uint64_t Scheduler::millis_64_(uint32_t now) { // THREAD SAFETY NOTE: // This function can be called from multiple threads simultaneously on ESP32/LibreTiny. - // On single-threaded platforms (ESP8266, RP2040), atomics are not needed. + // On single-core platforms, atomics are not needed. // // IMPORTANT: Always pass fresh millis() values to this function. The implementation // handles out-of-order timestamps between threads, but minimizing time differences @@ -508,99 +509,128 @@ uint64_t Scheduler::millis_64_(uint32_t now) { // This prevents race conditions at the rollover boundary without requiring // 64-bit atomics or locking on every call. +#ifdef ESPHOME_ATOMIC_SCHEDULER + for (;;) { + uint16_t major = this->millis_major_.load(std::memory_order_acquire); +#else /* not ESPHOME_ATOMIC_SCHEDULER */ + uint16_t major = this->millis_major_; +#endif /* else ESPHOME_ATOMIC_SCHEDULER */ + #ifdef USE_LIBRETINY - // LibreTiny: Multi-threaded but lacks atomic operation support - // TODO: If LibreTiny ever adds atomic support, remove this entire block and - // let it fall through to the atomic-based implementation below - // We need to use a lock when near the rollover boundary to prevent races - uint32_t last = this->last_millis_; + // LibreTiny: Multi-threaded but lacks atomic operation support + // TODO: If LibreTiny ever adds atomic support, remove this entire block and + // let it fall through to the atomic-based implementation below + // We need to use a lock when near the rollover boundary to prevent races + uint32_t last = this->last_millis_; - // Define a safe window around the rollover point (10 seconds) - // This covers any reasonable scheduler delays or thread preemption - static const uint32_t ROLLOVER_WINDOW = 10000; // 10 seconds in milliseconds + // Define a safe window around the rollover point (10 seconds) + // This covers any reasonable scheduler delays or thread preemption + static const uint32_t ROLLOVER_WINDOW = 10000; // 10 seconds in milliseconds - // Check if we're near the rollover boundary (close to std::numeric_limits::max() or just past 0) - bool near_rollover = (last > (std::numeric_limits::max() - ROLLOVER_WINDOW)) || (now < ROLLOVER_WINDOW); + // Check if we're near the rollover boundary (close to std::numeric_limits::max() or just past 0) + bool near_rollover = (last > (std::numeric_limits::max() - ROLLOVER_WINDOW)) || (now < ROLLOVER_WINDOW); - if (near_rollover || (now < last && (last - now) > HALF_MAX_UINT32)) { - // Near rollover or detected a rollover - need lock for safety - LockGuard guard{this->lock_}; - // Re-read with lock held - last = this->last_millis_; + if (near_rollover || (now < last && (last - now) > HALF_MAX_UINT32)) { + // Near rollover or detected a rollover - need lock for safety + LockGuard guard{this->lock_}; + // Re-read with lock held + last = this->last_millis_; - if (now < last && (last - now) > HALF_MAX_UINT32) { - // True rollover detected (happens every ~49.7 days) - this->millis_major_++; + if (now < last && (last - now) > HALF_MAX_UINT32) { + // True rollover detected (happens every ~49.7 days) + this->millis_major_++; + major++; #ifdef ESPHOME_DEBUG_SCHEDULER - ESP_LOGD(TAG, "Detected true 32-bit rollover at %" PRIu32 "ms (was %" PRIu32 ")", now, last); -#endif + ESP_LOGD(TAG, "Detected true 32-bit rollover at %" PRIu32 "ms (was %" PRIu32 ")", now, last); +#endif /* ESPHOME_DEBUG_SCHEDULER */ + } + // Update last_millis_ while holding lock + this->last_millis_ = now; + } else if (now > last) { + // Normal case: Not near rollover and time moved forward + // Update without lock. While this may cause minor races (microseconds of + // backwards time movement), they're acceptable because: + // 1. The scheduler operates at millisecond resolution, not microsecond + // 2. We've already prevented the critical rollover race condition + // 3. Any backwards movement is orders of magnitude smaller than scheduler delays + this->last_millis_ = now; } - // Update last_millis_ while holding lock - this->last_millis_ = now; - } else if (now > last) { - // Normal case: Not near rollover and time moved forward - // Update without lock. While this may cause minor races (microseconds of - // backwards time movement), they're acceptable because: - // 1. The scheduler operates at millisecond resolution, not microsecond - // 2. We've already prevented the critical rollover race condition - // 3. Any backwards movement is orders of magnitude smaller than scheduler delays - this->last_millis_ = now; - } - // If now <= last and we're not near rollover, don't update - // This minimizes backwards time movement + // If now <= last and we're not near rollover, don't update + // This minimizes backwards time movement -#elif !defined(USE_ESP8266) && !defined(USE_RP2040) - // Multi-threaded platforms with atomic support (ESP32) - uint32_t last = this->last_millis_.load(std::memory_order_relaxed); +#elif defined(ESPHOME_ATOMIC_SCHEDULER) + /* + * Multi-threaded platforms with atomic support (ESP32) + * Acquire so that if we later decide **not** to take the lock we still + * observe a `millis_major_` value coherent with the loaded `last_millis_`. + * The acquire load ensures any later read of `millis_major_` sees its + * corresponding increment. + */ + uint32_t last = this->last_millis_.load(std::memory_order_acquire); // If we might be near a rollover (large backwards jump), take the lock for the entire operation // This ensures rollover detection and last_millis_ update are atomic together if (now < last && (last - now) > HALF_MAX_UINT32) { // Potential rollover - need lock for atomic rollover detection + update LockGuard guard{this->lock_}; - // Re-read with lock held + // Re-read with lock held; mutex already provides ordering last = this->last_millis_.load(std::memory_order_relaxed); if (now < last && (last - now) > HALF_MAX_UINT32) { // True rollover detected (happens every ~49.7 days) - this->millis_major_++; + this->millis_major_.fetch_add(1, std::memory_order_relaxed); + major++; #ifdef ESPHOME_DEBUG_SCHEDULER ESP_LOGD(TAG, "Detected true 32-bit rollover at %" PRIu32 "ms (was %" PRIu32 ")", now, last); -#endif +#endif /* ESPHOME_DEBUG_SCHEDULER */ } - // Update last_millis_ while holding lock to prevent races - this->last_millis_.store(now, std::memory_order_relaxed); + /* + * Update last_millis_ while holding the lock to prevent races + * Publish the new low-word *after* bumping `millis_major_` (done above) + * so readers never see a mismatched pair. + */ + this->last_millis_.store(now, std::memory_order_release); } else { // Normal case: Try lock-free update, but only allow forward movement within same epoch // This prevents accidentally moving backwards across a rollover boundary while (now > last && (now - last) < HALF_MAX_UINT32) { - if (this->last_millis_.compare_exchange_weak(last, now, std::memory_order_relaxed)) { + if (this->last_millis_.compare_exchange_weak(last, now, + std::memory_order_release, // success + std::memory_order_relaxed)) { // failure break; } + // CAS failure means no data was published; relaxed is fine // last is automatically updated by compare_exchange_weak if it fails } } - -#else - // Single-threaded platforms (ESP8266, RP2040): No atomics needed +#else /* not USE_LIBRETINY; not ESPHOME_ATOMIC_SCHEDULER */ + // Single-core platforms: No atomics needed uint32_t last = this->last_millis_; // Check for rollover if (now < last && (last - now) > HALF_MAX_UINT32) { this->millis_major_++; + major++; #ifdef ESPHOME_DEBUG_SCHEDULER ESP_LOGD(TAG, "Detected true 32-bit rollover at %" PRIu32 "ms (was %" PRIu32 ")", now, last); -#endif +#endif /* ESPHOME_DEBUG_SCHEDULER */ } // Only update if time moved forward if (now > last) { this->last_millis_ = now; } -#endif +#endif /* else (USE_LIBRETINY / ESPHOME_ATOMIC_SCHEDULER) */ +#ifdef ESPHOME_ATOMIC_SCHEDULER + uint16_t major_end = this->millis_major_.load(std::memory_order_relaxed); + if (major_end == major) + return now + (static_cast(major) << 32); + } +#else /* not ESPHOME_ATOMIC_SCHEDULER */ // Combine major (high 32 bits) and now (low 32 bits) into 64-bit time - return now + (static_cast(this->millis_major_) << 32); + return now + (static_cast(major) << 32); +#endif /* ESPHOME_ATOMIC_SCHEDULER */ } bool HOT Scheduler::SchedulerItem::cmp(const std::unique_ptr &a, diff --git a/esphome/core/scheduler.h b/esphome/core/scheduler.h index 64df2f2bb0..540db3c16c 100644 --- a/esphome/core/scheduler.h +++ b/esphome/core/scheduler.h @@ -1,10 +1,11 @@ #pragma once +#include "esphome/core/defines.h" #include #include #include #include -#if !defined(USE_ESP8266) && !defined(USE_RP2040) && !defined(USE_LIBRETINY) +#ifdef ESPHOME_ATOMIC_SCHEDULER #include #endif @@ -204,22 +205,37 @@ class Scheduler { Mutex lock_; std::vector> items_; std::vector> to_add_; -#if !defined(USE_ESP8266) && !defined(USE_RP2040) - // ESP8266 and RP2040 don't need the defer queue because: - // ESP8266: Single-core with no preemptive multitasking - // RP2040: Currently has empty mutex implementation in ESPHome - // Both platforms save 40 bytes of RAM by excluding this +#ifndef ESPHOME_SINGLE_CORE + // Single-core platforms don't need the defer queue and save 40 bytes of RAM std::deque> defer_queue_; // FIFO queue for defer() calls -#endif -#if !defined(USE_ESP8266) && !defined(USE_RP2040) && !defined(USE_LIBRETINY) - // Multi-threaded platforms with atomic support: last_millis_ needs atomic for lock-free updates +#endif /* ESPHOME_SINGLE_CORE */ +#ifdef ESPHOME_ATOMIC_SCHEDULER + /* + * Multi-threaded platforms with atomic support: last_millis_ needs atomic for lock-free updates + * + * MEMORY-ORDERING NOTE + * -------------------- + * `last_millis_` and `millis_major_` form a single 64-bit timestamp split in half. + * Writers publish `last_millis_` with memory_order_release and readers use + * memory_order_acquire. This ensures that once a reader sees the new low word, + * it also observes the corresponding increment of `millis_major_`. + */ std::atomic last_millis_{0}; -#else +#else /* not ESPHOME_ATOMIC_SCHEDULER */ // Platforms without atomic support or single-threaded platforms uint32_t last_millis_{0}; -#endif - // millis_major_ is protected by lock when incrementing +#endif /* else ESPHOME_ATOMIC_SCHEDULER */ + /* + * Upper 16 bits of the 64-bit millis counter. Incremented only while holding + * `lock_`; read concurrently. Atomic (relaxed) avoids a formal data race. + * Ordering relative to `last_millis_` is provided by its release store and the + * corresponding acquire loads. + */ +#ifdef ESPHOME_ATOMIC_SCHEDULER + std::atomic millis_major_{0}; +#else /* not ESPHOME_ATOMIC_SCHEDULER */ uint16_t millis_major_{0}; +#endif /* else ESPHOME_ATOMIC_SCHEDULER */ uint32_t to_remove_{0}; }; From fde80bc53055de3e48af8c6f21c20249929369a5 Mon Sep 17 00:00:00 2001 From: RubenKelevra Date: Sat, 19 Jul 2025 21:44:35 +0200 Subject: [PATCH 3/9] core/scheduler: split millis_64_ into different platform functions --- esphome/core/defines.h | 11 +- esphome/core/scheduler.cpp | 267 ++++++++++++++++++++++--------------- esphome/core/scheduler.h | 14 +- 3 files changed, 178 insertions(+), 114 deletions(-) diff --git a/esphome/core/defines.h b/esphome/core/defines.h index 4e3145444d..d13c838ea7 100644 --- a/esphome/core/defines.h +++ b/esphome/core/defines.h @@ -229,14 +229,19 @@ #define USE_SOCKET_SELECT_SUPPORT #endif -// Helper macro for platforms that lack atomic scheduler support +// Helper macro for single core platforms that lack atomic scheduler support #if defined(USE_ESP8266) || defined(USE_RP2040) #define ESPHOME_SINGLE_CORE #endif -// Helper macro for platforms with atomic scheduler support +// Helper macro for multi core platforms that lack atomic scheduler support +#if !defined(ESPHOME_SINGLE_CORE) && defined(USE_LIBRETINY) +#define ESPHOME_MULTI_CORE_NO_ATOMICS +#endif + +// Helper macro for multi core platforms with atomic scheduler support #if !defined(ESPHOME_SINGLE_CORE) && !defined(USE_LIBRETINY) -#define ESPHOME_ATOMIC_SCHEDULER +#define ESPHOME_MULTI_CORE_ATOMICS #endif // Disabled feature flags diff --git a/esphome/core/scheduler.cpp b/esphome/core/scheduler.cpp index 21c45c2f97..ee4ec3818c 100644 --- a/esphome/core/scheduler.cpp +++ b/esphome/core/scheduler.cpp @@ -273,15 +273,15 @@ void HOT Scheduler::call(uint32_t now) { if (now_64 - last_print > 2000) { last_print = now_64; std::vector> old_items; -#ifdef ESPHOME_ATOMIC_SCHEDULER +#ifdef ESPHOME_MULTI_CORE_ATOMICS const auto last_dbg = this->last_millis_.load(std::memory_order_relaxed); const auto major_dbg = this->millis_major_.load(std::memory_order_relaxed); ESP_LOGD(TAG, "Items: count=%zu, now=%" PRIu64 " (%" PRIu16 ", %" PRIu32 ")", this->items_.size(), now_64, major_dbg, last_dbg); -#else /* not ESPHOME_ATOMIC_SCHEDULER */ +#else /* not ESPHOME_MULTI_CORE_ATOMICS */ ESP_LOGD(TAG, "Items: count=%zu, now=%" PRIu64 " (%" PRIu16 ", %" PRIu32 ")", this->items_.size(), now_64, this->millis_major_, this->last_millis_); -#endif /* else ESPHOME_ATOMIC_SCHEDULER */ +#endif /* else ESPHOME_MULTI_CORE_ATOMICS */ while (!this->empty_()) { std::unique_ptr item; { @@ -494,10 +494,18 @@ bool HOT Scheduler::cancel_item_locked_(Component *component, const char *name_c return total_cancelled > 0; } +#ifdef ESPHOME_SINGLE_CORE + uint64_t Scheduler::millis_64_(uint32_t now) { // THREAD SAFETY NOTE: - // This function can be called from multiple threads simultaneously on ESP32/LibreTiny. - // On single-core platforms, atomics are not needed. + // This function has three implemenations, based on the precompiler flags + // - ESPHOME_SINGLE_CORE + // - ESPHOME_MULTI_CORE_NO_ATOMICS + // - ESPHOME_MULTI_CORE_ATOMICS + // + // Make sure all changes are synchronous if you edit this function. + // + // This is the single core implementation. // // IMPORTANT: Always pass fresh millis() values to this function. The implementation // handles out-of-order timestamps between threads, but minimizing time differences @@ -509,101 +517,8 @@ uint64_t Scheduler::millis_64_(uint32_t now) { // This prevents race conditions at the rollover boundary without requiring // 64-bit atomics or locking on every call. -#ifdef ESPHOME_ATOMIC_SCHEDULER - for (;;) { - uint16_t major = this->millis_major_.load(std::memory_order_acquire); -#else /* not ESPHOME_ATOMIC_SCHEDULER */ uint16_t major = this->millis_major_; -#endif /* else ESPHOME_ATOMIC_SCHEDULER */ -#ifdef USE_LIBRETINY - // LibreTiny: Multi-threaded but lacks atomic operation support - // TODO: If LibreTiny ever adds atomic support, remove this entire block and - // let it fall through to the atomic-based implementation below - // We need to use a lock when near the rollover boundary to prevent races - uint32_t last = this->last_millis_; - - // Define a safe window around the rollover point (10 seconds) - // This covers any reasonable scheduler delays or thread preemption - static const uint32_t ROLLOVER_WINDOW = 10000; // 10 seconds in milliseconds - - // Check if we're near the rollover boundary (close to std::numeric_limits::max() or just past 0) - bool near_rollover = (last > (std::numeric_limits::max() - ROLLOVER_WINDOW)) || (now < ROLLOVER_WINDOW); - - if (near_rollover || (now < last && (last - now) > HALF_MAX_UINT32)) { - // Near rollover or detected a rollover - need lock for safety - LockGuard guard{this->lock_}; - // Re-read with lock held - last = this->last_millis_; - - if (now < last && (last - now) > HALF_MAX_UINT32) { - // True rollover detected (happens every ~49.7 days) - this->millis_major_++; - major++; -#ifdef ESPHOME_DEBUG_SCHEDULER - ESP_LOGD(TAG, "Detected true 32-bit rollover at %" PRIu32 "ms (was %" PRIu32 ")", now, last); -#endif /* ESPHOME_DEBUG_SCHEDULER */ - } - // Update last_millis_ while holding lock - this->last_millis_ = now; - } else if (now > last) { - // Normal case: Not near rollover and time moved forward - // Update without lock. While this may cause minor races (microseconds of - // backwards time movement), they're acceptable because: - // 1. The scheduler operates at millisecond resolution, not microsecond - // 2. We've already prevented the critical rollover race condition - // 3. Any backwards movement is orders of magnitude smaller than scheduler delays - this->last_millis_ = now; - } - // If now <= last and we're not near rollover, don't update - // This minimizes backwards time movement - -#elif defined(ESPHOME_ATOMIC_SCHEDULER) - /* - * Multi-threaded platforms with atomic support (ESP32) - * Acquire so that if we later decide **not** to take the lock we still - * observe a `millis_major_` value coherent with the loaded `last_millis_`. - * The acquire load ensures any later read of `millis_major_` sees its - * corresponding increment. - */ - uint32_t last = this->last_millis_.load(std::memory_order_acquire); - - // If we might be near a rollover (large backwards jump), take the lock for the entire operation - // This ensures rollover detection and last_millis_ update are atomic together - if (now < last && (last - now) > HALF_MAX_UINT32) { - // Potential rollover - need lock for atomic rollover detection + update - LockGuard guard{this->lock_}; - // Re-read with lock held; mutex already provides ordering - last = this->last_millis_.load(std::memory_order_relaxed); - - if (now < last && (last - now) > HALF_MAX_UINT32) { - // True rollover detected (happens every ~49.7 days) - this->millis_major_.fetch_add(1, std::memory_order_relaxed); - major++; -#ifdef ESPHOME_DEBUG_SCHEDULER - ESP_LOGD(TAG, "Detected true 32-bit rollover at %" PRIu32 "ms (was %" PRIu32 ")", now, last); -#endif /* ESPHOME_DEBUG_SCHEDULER */ - } - /* - * Update last_millis_ while holding the lock to prevent races - * Publish the new low-word *after* bumping `millis_major_` (done above) - * so readers never see a mismatched pair. - */ - this->last_millis_.store(now, std::memory_order_release); - } else { - // Normal case: Try lock-free update, but only allow forward movement within same epoch - // This prevents accidentally moving backwards across a rollover boundary - while (now > last && (now - last) < HALF_MAX_UINT32) { - if (this->last_millis_.compare_exchange_weak(last, now, - std::memory_order_release, // success - std::memory_order_relaxed)) { // failure - break; - } - // CAS failure means no data was published; relaxed is fine - // last is automatically updated by compare_exchange_weak if it fails - } - } -#else /* not USE_LIBRETINY; not ESPHOME_ATOMIC_SCHEDULER */ // Single-core platforms: No atomics needed uint32_t last = this->last_millis_; @@ -620,19 +535,163 @@ uint64_t Scheduler::millis_64_(uint32_t now) { if (now > last) { this->last_millis_ = now; } -#endif /* else (USE_LIBRETINY / ESPHOME_ATOMIC_SCHEDULER) */ -#ifdef ESPHOME_ATOMIC_SCHEDULER + // Combine major (high 32 bits) and now (low 32 bits) into 64-bit time + return now + (static_cast(major) << 32); +} + +#endif + +#ifdef ESPHOME_MULTI_CORE_NO_ATOMICS + +uint64_t Scheduler::millis_64_(uint32_t now) { + // THREAD SAFETY NOTE: + // This function has three implemenations, based on the precompiler flags + // - ESPHOME_SINGLE_CORE + // - ESPHOME_MULTI_CORE_NO_ATOMICS + // - ESPHOME_MULTI_CORE_ATOMICS + // + // Make sure all changes are synchronous if you edit this function. + // + // This is the multi core no atomics implementation. + // + // IMPORTANT: Always pass fresh millis() values to this function. The implementation + // handles out-of-order timestamps between threads, but minimizing time differences + // helps maintain accuracy. + // + // The implementation handles the 32-bit rollover (every 49.7 days) by: + // 1. Using a lock when detecting rollover to ensure atomic update + // 2. Restricting normal updates to forward movement within the same epoch + // This prevents race conditions at the rollover boundary without requiring + // 64-bit atomics or locking on every call. + + uint16_t major = this->millis_major_; + + // LibreTiny: Multi-threaded but lacks atomic operation support + // TODO: If LibreTiny ever adds atomic support, remove this entire block and + // let it fall through to the atomic-based implementation below + // We need to use a lock when near the rollover boundary to prevent races + uint32_t last = this->last_millis_; + + // Define a safe window around the rollover point (10 seconds) + // This covers any reasonable scheduler delays or thread preemption + static const uint32_t ROLLOVER_WINDOW = 10000; // 10 seconds in milliseconds + + // Check if we're near the rollover boundary (close to std::numeric_limits::max() or just past 0) + bool near_rollover = (last > (std::numeric_limits::max() - ROLLOVER_WINDOW)) || (now < ROLLOVER_WINDOW); + + if (near_rollover || (now < last && (last - now) > HALF_MAX_UINT32)) { + // Near rollover or detected a rollover - need lock for safety + LockGuard guard{this->lock_}; + // Re-read with lock held + last = this->last_millis_; + + if (now < last && (last - now) > HALF_MAX_UINT32) { + // True rollover detected (happens every ~49.7 days) + this->millis_major_++; + major++; +#ifdef ESPHOME_DEBUG_SCHEDULER + ESP_LOGD(TAG, "Detected true 32-bit rollover at %" PRIu32 "ms (was %" PRIu32 ")", now, last); +#endif /* ESPHOME_DEBUG_SCHEDULER */ + } + // Update last_millis_ while holding lock + this->last_millis_ = now; + } else if (now > last) { + // Normal case: Not near rollover and time moved forward + // Update without lock. While this may cause minor races (microseconds of + // backwards time movement), they're acceptable because: + // 1. The scheduler operates at millisecond resolution, not microsecond + // 2. We've already prevented the critical rollover race condition + // 3. Any backwards movement is orders of magnitude smaller than scheduler delays + this->last_millis_ = now; + } + // If now <= last and we're not near rollover, don't update + // This minimizes backwards time movement + + // Combine major (high 32 bits) and now (low 32 bits) into 64-bit time + return now + (static_cast(major) << 32); +} + +#endif + +#ifdef ESPHOME_MULTI_CORE_ATOMICS + +uint64_t Scheduler::millis_64_(uint32_t now) { + // THREAD SAFETY NOTE: + // This function has three implemenations, based on the precompiler flags + // - ESPHOME_SINGLE_CORE + // - ESPHOME_MULTI_CORE_NO_ATOMICS + // - ESPHOME_MULTI_CORE_ATOMICS + // + // Make sure all changes are synchronous if you edit this function. + // + // This is the multi core with atomics implementation. + // + // IMPORTANT: Always pass fresh millis() values to this function. The implementation + // handles out-of-order timestamps between threads, but minimizing time differences + // helps maintain accuracy. + // + // The implementation handles the 32-bit rollover (every 49.7 days) by: + // 1. Using a lock when detecting rollover to ensure atomic update + // 2. Restricting normal updates to forward movement within the same epoch + // This prevents race conditions at the rollover boundary without requiring + // 64-bit atomics or locking on every call. + + for (;;) { + uint16_t major = this->millis_major_.load(std::memory_order_acquire); + + /* + * Multi-threaded platforms with atomic support (ESP32) + * Acquire so that if we later decide **not** to take the lock we still + * observe a `millis_major_` value coherent with the loaded `last_millis_`. + * The acquire load ensures any later read of `millis_major_` sees its + * corresponding increment. + */ + uint32_t last = this->last_millis_.load(std::memory_order_acquire); + + // If we might be near a rollover (large backwards jump), take the lock for the entire operation + // This ensures rollover detection and last_millis_ update are atomic together + if (now < last && (last - now) > HALF_MAX_UINT32) { + // Potential rollover - need lock for atomic rollover detection + update + LockGuard guard{this->lock_}; + // Re-read with lock held; mutex already provides ordering + last = this->last_millis_.load(std::memory_order_relaxed); + + if (now < last && (last - now) > HALF_MAX_UINT32) { + // True rollover detected (happens every ~49.7 days) + this->millis_major_.fetch_add(1, std::memory_order_relaxed); + major++; +#ifdef ESPHOME_DEBUG_SCHEDULER + ESP_LOGD(TAG, "Detected true 32-bit rollover at %" PRIu32 "ms (was %" PRIu32 ")", now, last); +#endif /* ESPHOME_DEBUG_SCHEDULER */ + } + /* + * Update last_millis_ while holding the lock to prevent races + * Publish the new low-word *after* bumping `millis_major_` (done above) + * so readers never see a mismatched pair. + */ + this->last_millis_.store(now, std::memory_order_release); + } else { + // Normal case: Try lock-free update, but only allow forward movement within same epoch + // This prevents accidentally moving backwards across a rollover boundary + while (now > last && (now - last) < HALF_MAX_UINT32) { + if (this->last_millis_.compare_exchange_weak(last, now, + std::memory_order_release, // success + std::memory_order_relaxed)) { // failure + break; + } + // CAS failure means no data was published; relaxed is fine + // last is automatically updated by compare_exchange_weak if it fails + } + } uint16_t major_end = this->millis_major_.load(std::memory_order_relaxed); if (major_end == major) return now + (static_cast(major) << 32); } -#else /* not ESPHOME_ATOMIC_SCHEDULER */ - // Combine major (high 32 bits) and now (low 32 bits) into 64-bit time - return now + (static_cast(major) << 32); -#endif /* ESPHOME_ATOMIC_SCHEDULER */ } +#endif + bool HOT Scheduler::SchedulerItem::cmp(const std::unique_ptr &a, const std::unique_ptr &b) { return a->next_execution_ > b->next_execution_; diff --git a/esphome/core/scheduler.h b/esphome/core/scheduler.h index 540db3c16c..0e3bca22a6 100644 --- a/esphome/core/scheduler.h +++ b/esphome/core/scheduler.h @@ -5,7 +5,7 @@ #include #include #include -#ifdef ESPHOME_ATOMIC_SCHEDULER +#ifdef ESPHOME_MULTI_CORE_ATOMICS #include #endif @@ -209,7 +209,7 @@ class Scheduler { // Single-core platforms don't need the defer queue and save 40 bytes of RAM std::deque> defer_queue_; // FIFO queue for defer() calls #endif /* ESPHOME_SINGLE_CORE */ -#ifdef ESPHOME_ATOMIC_SCHEDULER +#ifdef ESPHOME_MULTI_CORE_ATOMICS /* * Multi-threaded platforms with atomic support: last_millis_ needs atomic for lock-free updates * @@ -221,21 +221,21 @@ class Scheduler { * it also observes the corresponding increment of `millis_major_`. */ std::atomic last_millis_{0}; -#else /* not ESPHOME_ATOMIC_SCHEDULER */ +#else /* not ESPHOME_MULTI_CORE_ATOMICS */ // Platforms without atomic support or single-threaded platforms uint32_t last_millis_{0}; -#endif /* else ESPHOME_ATOMIC_SCHEDULER */ +#endif /* else ESPHOME_MULTI_CORE_ATOMICS */ /* * Upper 16 bits of the 64-bit millis counter. Incremented only while holding * `lock_`; read concurrently. Atomic (relaxed) avoids a formal data race. * Ordering relative to `last_millis_` is provided by its release store and the * corresponding acquire loads. */ -#ifdef ESPHOME_ATOMIC_SCHEDULER +#ifdef ESPHOME_MULTI_CORE_ATOMICS std::atomic millis_major_{0}; -#else /* not ESPHOME_ATOMIC_SCHEDULER */ +#else /* not ESPHOME_MULTI_CORE_ATOMICS */ uint16_t millis_major_{0}; -#endif /* else ESPHOME_ATOMIC_SCHEDULER */ +#endif /* else ESPHOME_MULTI_CORE_ATOMICS */ uint32_t to_remove_{0}; }; From a5f5af9596d83dbd3518fd4720ddc93d1a24b8c8 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 19 Jul 2025 10:36:49 -1000 Subject: [PATCH 4/9] make more readable --- esphome/core/scheduler.cpp | 237 ++++++++++++++++--------------------- esphome/core/scheduler.h | 4 +- 2 files changed, 103 insertions(+), 138 deletions(-) diff --git a/esphome/core/scheduler.cpp b/esphome/core/scheduler.cpp index ee4ec3818c..317426b51e 100644 --- a/esphome/core/scheduler.cpp +++ b/esphome/core/scheduler.cpp @@ -494,23 +494,23 @@ bool HOT Scheduler::cancel_item_locked_(Component *component, const char *name_c return total_cancelled > 0; } -#ifdef ESPHOME_SINGLE_CORE - uint64_t Scheduler::millis_64_(uint32_t now) { // THREAD SAFETY NOTE: - // This function has three implemenations, based on the precompiler flags - // - ESPHOME_SINGLE_CORE - // - ESPHOME_MULTI_CORE_NO_ATOMICS - // - ESPHOME_MULTI_CORE_ATOMICS + // This function has three implementations, based on the precompiler flags + // - ESPHOME_SINGLE_CORE - Runs on single-core platforms (ESP8266, RP2040, etc.) + // - ESPHOME_MULTI_CORE_NO_ATOMICS - Runs on multi-core platforms without atomics (LibreTiny) + // - ESPHOME_MULTI_CORE_ATOMICS - Runs on multi-core platforms with atomics (ESP32, HOST, etc.) // - // Make sure all changes are synchronous if you edit this function. - // - // This is the single core implementation. + // Make sure all changes are synchronized if you edit this function. // // IMPORTANT: Always pass fresh millis() values to this function. The implementation // handles out-of-order timestamps between threads, but minimizing time differences // helps maintain accuracy. // + +#ifdef ESPHOME_SINGLE_CORE + // This is the single core implementation. + // // The implementation handles the 32-bit rollover (every 49.7 days) by: // 1. Using a lock when detecting rollover to ensure atomic update // 2. Restricting normal updates to forward movement within the same epoch @@ -539,158 +539,121 @@ uint64_t Scheduler::millis_64_(uint32_t now) { // Combine major (high 32 bits) and now (low 32 bits) into 64-bit time return now + (static_cast(major) << 32); } - -#endif +#endif // ESPHOME_SINGLE_CORE #ifdef ESPHOME_MULTI_CORE_NO_ATOMICS +// This is the multi core no atomics implementation. +// +// The implementation handles the 32-bit rollover (every 49.7 days) by: +// 1. Using a lock when detecting rollover to ensure atomic update +// 2. Restricting normal updates to forward movement within the same epoch +// This prevents race conditions at the rollover boundary without requiring +// 64-bit atomics or locking on every call. -uint64_t Scheduler::millis_64_(uint32_t now) { - // THREAD SAFETY NOTE: - // This function has three implemenations, based on the precompiler flags - // - ESPHOME_SINGLE_CORE - // - ESPHOME_MULTI_CORE_NO_ATOMICS - // - ESPHOME_MULTI_CORE_ATOMICS - // - // Make sure all changes are synchronous if you edit this function. - // - // This is the multi core no atomics implementation. - // - // IMPORTANT: Always pass fresh millis() values to this function. The implementation - // handles out-of-order timestamps between threads, but minimizing time differences - // helps maintain accuracy. - // - // The implementation handles the 32-bit rollover (every 49.7 days) by: - // 1. Using a lock when detecting rollover to ensure atomic update - // 2. Restricting normal updates to forward movement within the same epoch - // This prevents race conditions at the rollover boundary without requiring - // 64-bit atomics or locking on every call. +uint16_t major = this->millis_major_; +uint32_t last = this->last_millis_; - uint16_t major = this->millis_major_; +// Define a safe window around the rollover point (10 seconds) +// This covers any reasonable scheduler delays or thread preemption +static const uint32_t ROLLOVER_WINDOW = 10000; // 10 seconds in milliseconds - // LibreTiny: Multi-threaded but lacks atomic operation support - // TODO: If LibreTiny ever adds atomic support, remove this entire block and - // let it fall through to the atomic-based implementation below - // We need to use a lock when near the rollover boundary to prevent races - uint32_t last = this->last_millis_; +// Check if we're near the rollover boundary (close to std::numeric_limits::max() or just past 0) +bool near_rollover = (last > (std::numeric_limits::max() - ROLLOVER_WINDOW)) || (now < ROLLOVER_WINDOW); - // Define a safe window around the rollover point (10 seconds) - // This covers any reasonable scheduler delays or thread preemption - static const uint32_t ROLLOVER_WINDOW = 10000; // 10 seconds in milliseconds +if (near_rollover || (now < last && (last - now) > HALF_MAX_UINT32)) { + // Near rollover or detected a rollover - need lock for safety + LockGuard guard{this->lock_}; + // Re-read with lock held + last = this->last_millis_; - // Check if we're near the rollover boundary (close to std::numeric_limits::max() or just past 0) - bool near_rollover = (last > (std::numeric_limits::max() - ROLLOVER_WINDOW)) || (now < ROLLOVER_WINDOW); + if (now < last && (last - now) > HALF_MAX_UINT32) { + // True rollover detected (happens every ~49.7 days) + this->millis_major_++; + major++; +#ifdef ESPHOME_DEBUG_SCHEDULER + ESP_LOGD(TAG, "Detected true 32-bit rollover at %" PRIu32 "ms (was %" PRIu32 ")", now, last); +#endif /* ESPHOME_DEBUG_SCHEDULER */ + } + // Update last_millis_ while holding lock + this->last_millis_ = now; +} else if (now > last) { + // Normal case: Not near rollover and time moved forward + // Update without lock. While this may cause minor races (microseconds of + // backwards time movement), they're acceptable because: + // 1. The scheduler operates at millisecond resolution, not microsecond + // 2. We've already prevented the critical rollover race condition + // 3. Any backwards movement is orders of magnitude smaller than scheduler delays + this->last_millis_ = now; +} +// If now <= last and we're not near rollover, don't update +// This minimizes backwards time movement - if (near_rollover || (now < last && (last - now) > HALF_MAX_UINT32)) { - // Near rollover or detected a rollover - need lock for safety +// Combine major (high 32 bits) and now (low 32 bits) into 64-bit time +return now + (static_cast(major) << 32); +#endif // ESPHOME_MULTI_CORE_NO_ATOMICS + +#ifdef ESPHOME_MULTI_CORE_ATOMICS +// This is the multi core with atomics implementation. +// +// The implementation handles the 32-bit rollover (every 49.7 days) by: +// 1. Using a lock when detecting rollover to ensure atomic update +// 2. Restricting normal updates to forward movement within the same epoch +// This prevents race conditions at the rollover boundary without requiring +// 64-bit atomics or locking on every call. + +for (;;) { + uint16_t major = this->millis_major_.load(std::memory_order_acquire); + + /* + * Acquire so that if we later decide **not** to take the lock we still + * observe a `millis_major_` value coherent with the loaded `last_millis_`. + * The acquire load ensures any later read of `millis_major_` sees its + * corresponding increment. + */ + uint32_t last = this->last_millis_.load(std::memory_order_acquire); + + // If we might be near a rollover (large backwards jump), take the lock for the entire operation + // This ensures rollover detection and last_millis_ update are atomic together + if (now < last && (last - now) > HALF_MAX_UINT32) { + // Potential rollover - need lock for atomic rollover detection + update LockGuard guard{this->lock_}; - // Re-read with lock held - last = this->last_millis_; + // Re-read with lock held; mutex already provides ordering + last = this->last_millis_.load(std::memory_order_relaxed); if (now < last && (last - now) > HALF_MAX_UINT32) { // True rollover detected (happens every ~49.7 days) - this->millis_major_++; + this->millis_major_.fetch_add(1, std::memory_order_relaxed); major++; #ifdef ESPHOME_DEBUG_SCHEDULER ESP_LOGD(TAG, "Detected true 32-bit rollover at %" PRIu32 "ms (was %" PRIu32 ")", now, last); #endif /* ESPHOME_DEBUG_SCHEDULER */ } - // Update last_millis_ while holding lock - this->last_millis_ = now; - } else if (now > last) { - // Normal case: Not near rollover and time moved forward - // Update without lock. While this may cause minor races (microseconds of - // backwards time movement), they're acceptable because: - // 1. The scheduler operates at millisecond resolution, not microsecond - // 2. We've already prevented the critical rollover race condition - // 3. Any backwards movement is orders of magnitude smaller than scheduler delays - this->last_millis_ = now; - } - // If now <= last and we're not near rollover, don't update - // This minimizes backwards time movement - - // Combine major (high 32 bits) and now (low 32 bits) into 64-bit time - return now + (static_cast(major) << 32); -} - -#endif - -#ifdef ESPHOME_MULTI_CORE_ATOMICS - -uint64_t Scheduler::millis_64_(uint32_t now) { - // THREAD SAFETY NOTE: - // This function has three implemenations, based on the precompiler flags - // - ESPHOME_SINGLE_CORE - // - ESPHOME_MULTI_CORE_NO_ATOMICS - // - ESPHOME_MULTI_CORE_ATOMICS - // - // Make sure all changes are synchronous if you edit this function. - // - // This is the multi core with atomics implementation. - // - // IMPORTANT: Always pass fresh millis() values to this function. The implementation - // handles out-of-order timestamps between threads, but minimizing time differences - // helps maintain accuracy. - // - // The implementation handles the 32-bit rollover (every 49.7 days) by: - // 1. Using a lock when detecting rollover to ensure atomic update - // 2. Restricting normal updates to forward movement within the same epoch - // This prevents race conditions at the rollover boundary without requiring - // 64-bit atomics or locking on every call. - - for (;;) { - uint16_t major = this->millis_major_.load(std::memory_order_acquire); - /* - * Multi-threaded platforms with atomic support (ESP32) - * Acquire so that if we later decide **not** to take the lock we still - * observe a `millis_major_` value coherent with the loaded `last_millis_`. - * The acquire load ensures any later read of `millis_major_` sees its - * corresponding increment. + * Update last_millis_ while holding the lock to prevent races + * Publish the new low-word *after* bumping `millis_major_` (done above) + * so readers never see a mismatched pair. */ - uint32_t last = this->last_millis_.load(std::memory_order_acquire); - - // If we might be near a rollover (large backwards jump), take the lock for the entire operation - // This ensures rollover detection and last_millis_ update are atomic together - if (now < last && (last - now) > HALF_MAX_UINT32) { - // Potential rollover - need lock for atomic rollover detection + update - LockGuard guard{this->lock_}; - // Re-read with lock held; mutex already provides ordering - last = this->last_millis_.load(std::memory_order_relaxed); - - if (now < last && (last - now) > HALF_MAX_UINT32) { - // True rollover detected (happens every ~49.7 days) - this->millis_major_.fetch_add(1, std::memory_order_relaxed); - major++; -#ifdef ESPHOME_DEBUG_SCHEDULER - ESP_LOGD(TAG, "Detected true 32-bit rollover at %" PRIu32 "ms (was %" PRIu32 ")", now, last); -#endif /* ESPHOME_DEBUG_SCHEDULER */ - } - /* - * Update last_millis_ while holding the lock to prevent races - * Publish the new low-word *after* bumping `millis_major_` (done above) - * so readers never see a mismatched pair. - */ - this->last_millis_.store(now, std::memory_order_release); - } else { - // Normal case: Try lock-free update, but only allow forward movement within same epoch - // This prevents accidentally moving backwards across a rollover boundary - while (now > last && (now - last) < HALF_MAX_UINT32) { - if (this->last_millis_.compare_exchange_weak(last, now, - std::memory_order_release, // success - std::memory_order_relaxed)) { // failure - break; - } - // CAS failure means no data was published; relaxed is fine - // last is automatically updated by compare_exchange_weak if it fails + this->last_millis_.store(now, std::memory_order_release); + } else { + // Normal case: Try lock-free update, but only allow forward movement within same epoch + // This prevents accidentally moving backwards across a rollover boundary + while (now > last && (now - last) < HALF_MAX_UINT32) { + if (this->last_millis_.compare_exchange_weak(last, now, + std::memory_order_release, // success + std::memory_order_relaxed)) { // failure + break; } + // CAS failure means no data was published; relaxed is fine + // last is automatically updated by compare_exchange_weak if it fails } - uint16_t major_end = this->millis_major_.load(std::memory_order_relaxed); - if (major_end == major) - return now + (static_cast(major) << 32); } + uint16_t major_end = this->millis_major_.load(std::memory_order_relaxed); + if (major_end == major) + return now + (static_cast(major) << 32); } +#endif // ESPHOME_MULTI_CORE_ATOMICS -#endif +} bool HOT Scheduler::SchedulerItem::cmp(const std::unique_ptr &a, const std::unique_ptr &b) { diff --git a/esphome/core/scheduler.h b/esphome/core/scheduler.h index 0e3bca22a6..c9c2008718 100644 --- a/esphome/core/scheduler.h +++ b/esphome/core/scheduler.h @@ -209,6 +209,8 @@ class Scheduler { // Single-core platforms don't need the defer queue and save 40 bytes of RAM std::deque> defer_queue_; // FIFO queue for defer() calls #endif /* ESPHOME_SINGLE_CORE */ + uint32_t to_remove_{0}; + #ifdef ESPHOME_MULTI_CORE_ATOMICS /* * Multi-threaded platforms with atomic support: last_millis_ needs atomic for lock-free updates @@ -225,6 +227,7 @@ class Scheduler { // Platforms without atomic support or single-threaded platforms uint32_t last_millis_{0}; #endif /* else ESPHOME_MULTI_CORE_ATOMICS */ + /* * Upper 16 bits of the 64-bit millis counter. Incremented only while holding * `lock_`; read concurrently. Atomic (relaxed) avoids a formal data race. @@ -236,7 +239,6 @@ class Scheduler { #else /* not ESPHOME_MULTI_CORE_ATOMICS */ uint16_t millis_major_{0}; #endif /* else ESPHOME_MULTI_CORE_ATOMICS */ - uint32_t to_remove_{0}; }; } // namespace esphome From 58696961bd3869b0eec903025b3919afde0eb164 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 19 Jul 2025 10:38:28 -1000 Subject: [PATCH 5/9] make more readable --- esphome/core/scheduler.cpp | 201 ++++++++++++++++++------------------- 1 file changed, 97 insertions(+), 104 deletions(-) diff --git a/esphome/core/scheduler.cpp b/esphome/core/scheduler.cpp index 317426b51e..6834b72aab 100644 --- a/esphome/core/scheduler.cpp +++ b/esphome/core/scheduler.cpp @@ -511,15 +511,10 @@ uint64_t Scheduler::millis_64_(uint32_t now) { #ifdef ESPHOME_SINGLE_CORE // This is the single core implementation. // - // The implementation handles the 32-bit rollover (every 49.7 days) by: - // 1. Using a lock when detecting rollover to ensure atomic update - // 2. Restricting normal updates to forward movement within the same epoch - // This prevents race conditions at the rollover boundary without requiring - // 64-bit atomics or locking on every call. + // Single-core platforms have no concurrency, so this is a simple implementation + // that just tracks 32-bit rollover (every 49.7 days) without any locking or atomics. uint16_t major = this->millis_major_; - - // Single-core platforms: No atomics needed uint32_t last = this->last_millis_; // Check for rollover @@ -538,121 +533,119 @@ uint64_t Scheduler::millis_64_(uint32_t now) { // Combine major (high 32 bits) and now (low 32 bits) into 64-bit time return now + (static_cast(major) << 32); -} #endif // ESPHOME_SINGLE_CORE #ifdef ESPHOME_MULTI_CORE_NO_ATOMICS -// This is the multi core no atomics implementation. -// -// The implementation handles the 32-bit rollover (every 49.7 days) by: -// 1. Using a lock when detecting rollover to ensure atomic update -// 2. Restricting normal updates to forward movement within the same epoch -// This prevents race conditions at the rollover boundary without requiring -// 64-bit atomics or locking on every call. + // This is the multi core no atomics implementation. + // + // The implementation handles the 32-bit rollover (every 49.7 days) by: + // 1. Using a lock when detecting rollover to ensure atomic update + // 2. Restricting normal updates to forward movement within the same epoch + // This prevents race conditions at the rollover boundary without requiring + // 64-bit atomics or locking on every call. -uint16_t major = this->millis_major_; -uint32_t last = this->last_millis_; + uint16_t major = this->millis_major_; + uint32_t last = this->last_millis_; -// Define a safe window around the rollover point (10 seconds) -// This covers any reasonable scheduler delays or thread preemption -static const uint32_t ROLLOVER_WINDOW = 10000; // 10 seconds in milliseconds + // Define a safe window around the rollover point (10 seconds) + // This covers any reasonable scheduler delays or thread preemption + static const uint32_t ROLLOVER_WINDOW = 10000; // 10 seconds in milliseconds -// Check if we're near the rollover boundary (close to std::numeric_limits::max() or just past 0) -bool near_rollover = (last > (std::numeric_limits::max() - ROLLOVER_WINDOW)) || (now < ROLLOVER_WINDOW); + // Check if we're near the rollover boundary (close to std::numeric_limits::max() or just past 0) + bool near_rollover = (last > (std::numeric_limits::max() - ROLLOVER_WINDOW)) || (now < ROLLOVER_WINDOW); -if (near_rollover || (now < last && (last - now) > HALF_MAX_UINT32)) { - // Near rollover or detected a rollover - need lock for safety - LockGuard guard{this->lock_}; - // Re-read with lock held - last = this->last_millis_; - - if (now < last && (last - now) > HALF_MAX_UINT32) { - // True rollover detected (happens every ~49.7 days) - this->millis_major_++; - major++; -#ifdef ESPHOME_DEBUG_SCHEDULER - ESP_LOGD(TAG, "Detected true 32-bit rollover at %" PRIu32 "ms (was %" PRIu32 ")", now, last); -#endif /* ESPHOME_DEBUG_SCHEDULER */ - } - // Update last_millis_ while holding lock - this->last_millis_ = now; -} else if (now > last) { - // Normal case: Not near rollover and time moved forward - // Update without lock. While this may cause minor races (microseconds of - // backwards time movement), they're acceptable because: - // 1. The scheduler operates at millisecond resolution, not microsecond - // 2. We've already prevented the critical rollover race condition - // 3. Any backwards movement is orders of magnitude smaller than scheduler delays - this->last_millis_ = now; -} -// If now <= last and we're not near rollover, don't update -// This minimizes backwards time movement - -// Combine major (high 32 bits) and now (low 32 bits) into 64-bit time -return now + (static_cast(major) << 32); -#endif // ESPHOME_MULTI_CORE_NO_ATOMICS - -#ifdef ESPHOME_MULTI_CORE_ATOMICS -// This is the multi core with atomics implementation. -// -// The implementation handles the 32-bit rollover (every 49.7 days) by: -// 1. Using a lock when detecting rollover to ensure atomic update -// 2. Restricting normal updates to forward movement within the same epoch -// This prevents race conditions at the rollover boundary without requiring -// 64-bit atomics or locking on every call. - -for (;;) { - uint16_t major = this->millis_major_.load(std::memory_order_acquire); - - /* - * Acquire so that if we later decide **not** to take the lock we still - * observe a `millis_major_` value coherent with the loaded `last_millis_`. - * The acquire load ensures any later read of `millis_major_` sees its - * corresponding increment. - */ - uint32_t last = this->last_millis_.load(std::memory_order_acquire); - - // If we might be near a rollover (large backwards jump), take the lock for the entire operation - // This ensures rollover detection and last_millis_ update are atomic together - if (now < last && (last - now) > HALF_MAX_UINT32) { - // Potential rollover - need lock for atomic rollover detection + update + if (near_rollover || (now < last && (last - now) > HALF_MAX_UINT32)) { + // Near rollover or detected a rollover - need lock for safety LockGuard guard{this->lock_}; - // Re-read with lock held; mutex already provides ordering - last = this->last_millis_.load(std::memory_order_relaxed); + // Re-read with lock held + last = this->last_millis_; if (now < last && (last - now) > HALF_MAX_UINT32) { // True rollover detected (happens every ~49.7 days) - this->millis_major_.fetch_add(1, std::memory_order_relaxed); + this->millis_major_++; major++; #ifdef ESPHOME_DEBUG_SCHEDULER ESP_LOGD(TAG, "Detected true 32-bit rollover at %" PRIu32 "ms (was %" PRIu32 ")", now, last); #endif /* ESPHOME_DEBUG_SCHEDULER */ } - /* - * Update last_millis_ while holding the lock to prevent races - * Publish the new low-word *after* bumping `millis_major_` (done above) - * so readers never see a mismatched pair. - */ - this->last_millis_.store(now, std::memory_order_release); - } else { - // Normal case: Try lock-free update, but only allow forward movement within same epoch - // This prevents accidentally moving backwards across a rollover boundary - while (now > last && (now - last) < HALF_MAX_UINT32) { - if (this->last_millis_.compare_exchange_weak(last, now, - std::memory_order_release, // success - std::memory_order_relaxed)) { // failure - break; - } - // CAS failure means no data was published; relaxed is fine - // last is automatically updated by compare_exchange_weak if it fails - } + // Update last_millis_ while holding lock + this->last_millis_ = now; + } else if (now > last) { + // Normal case: Not near rollover and time moved forward + // Update without lock. While this may cause minor races (microseconds of + // backwards time movement), they're acceptable because: + // 1. The scheduler operates at millisecond resolution, not microsecond + // 2. We've already prevented the critical rollover race condition + // 3. Any backwards movement is orders of magnitude smaller than scheduler delays + this->last_millis_ = now; } - uint16_t major_end = this->millis_major_.load(std::memory_order_relaxed); - if (major_end == major) - return now + (static_cast(major) << 32); -} -#endif // ESPHOME_MULTI_CORE_ATOMICS + // If now <= last and we're not near rollover, don't update + // This minimizes backwards time movement + // Combine major (high 32 bits) and now (low 32 bits) into 64-bit time + return now + (static_cast(major) << 32); +#endif // ESPHOME_MULTI_CORE_NO_ATOMICS + +#ifdef ESPHOME_MULTI_CORE_ATOMICS + // This is the multi core with atomics implementation. + // + // The implementation handles the 32-bit rollover (every 49.7 days) by: + // 1. Using a lock when detecting rollover to ensure atomic update + // 2. Restricting normal updates to forward movement within the same epoch + // This prevents race conditions at the rollover boundary without requiring + // 64-bit atomics or locking on every call. + + for (;;) { + uint16_t major = this->millis_major_.load(std::memory_order_acquire); + + /* + * Acquire so that if we later decide **not** to take the lock we still + * observe a `millis_major_` value coherent with the loaded `last_millis_`. + * The acquire load ensures any later read of `millis_major_` sees its + * corresponding increment. + */ + uint32_t last = this->last_millis_.load(std::memory_order_acquire); + + // If we might be near a rollover (large backwards jump), take the lock for the entire operation + // This ensures rollover detection and last_millis_ update are atomic together + if (now < last && (last - now) > HALF_MAX_UINT32) { + // Potential rollover - need lock for atomic rollover detection + update + LockGuard guard{this->lock_}; + // Re-read with lock held; mutex already provides ordering + last = this->last_millis_.load(std::memory_order_relaxed); + + if (now < last && (last - now) > HALF_MAX_UINT32) { + // True rollover detected (happens every ~49.7 days) + this->millis_major_.fetch_add(1, std::memory_order_relaxed); + major++; +#ifdef ESPHOME_DEBUG_SCHEDULER + ESP_LOGD(TAG, "Detected true 32-bit rollover at %" PRIu32 "ms (was %" PRIu32 ")", now, last); +#endif /* ESPHOME_DEBUG_SCHEDULER */ + } + /* + * Update last_millis_ while holding the lock to prevent races + * Publish the new low-word *after* bumping `millis_major_` (done above) + * so readers never see a mismatched pair. + */ + this->last_millis_.store(now, std::memory_order_release); + } else { + // Normal case: Try lock-free update, but only allow forward movement within same epoch + // This prevents accidentally moving backwards across a rollover boundary + while (now > last && (now - last) < HALF_MAX_UINT32) { + if (this->last_millis_.compare_exchange_weak(last, now, + std::memory_order_release, // success + std::memory_order_relaxed)) { // failure + break; + } + // CAS failure means no data was published; relaxed is fine + // last is automatically updated by compare_exchange_weak if it fails + } + } + uint16_t major_end = this->millis_major_.load(std::memory_order_relaxed); + if (major_end == major) + return now + (static_cast(major) << 32); + } +#endif // ESPHOME_MULTI_CORE_ATOMICS } bool HOT Scheduler::SchedulerItem::cmp(const std::unique_ptr &a, From 5ed589fc97be0f0077b91e0a40e94d7defe644c0 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 19 Jul 2025 10:39:27 -1000 Subject: [PATCH 6/9] make more readable --- esphome/core/scheduler.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/esphome/core/scheduler.cpp b/esphome/core/scheduler.cpp index 6834b72aab..536a2b1025 100644 --- a/esphome/core/scheduler.cpp +++ b/esphome/core/scheduler.cpp @@ -538,11 +538,11 @@ uint64_t Scheduler::millis_64_(uint32_t now) { #ifdef ESPHOME_MULTI_CORE_NO_ATOMICS // This is the multi core no atomics implementation. // - // The implementation handles the 32-bit rollover (every 49.7 days) by: - // 1. Using a lock when detecting rollover to ensure atomic update - // 2. Restricting normal updates to forward movement within the same epoch - // This prevents race conditions at the rollover boundary without requiring - // 64-bit atomics or locking on every call. + // Without atomics, this implementation uses locks more aggressively: + // 1. Always locks when near the rollover boundary (within 10 seconds) + // 2. Always locks when detecting a large backwards jump + // 3. Updates without lock in normal forward progression (accepting minor races) + // This is less efficient but necessary without atomic operations. uint16_t major = this->millis_major_; uint32_t last = this->last_millis_; From acbcc5f9b8505317424eb21d847ba0db0cd551c2 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 19 Jul 2025 10:40:21 -1000 Subject: [PATCH 7/9] make more readable --- esphome/core/scheduler.cpp | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/esphome/core/scheduler.cpp b/esphome/core/scheduler.cpp index 536a2b1025..ca267cbe27 100644 --- a/esphome/core/scheduler.cpp +++ b/esphome/core/scheduler.cpp @@ -589,11 +589,12 @@ uint64_t Scheduler::millis_64_(uint32_t now) { #ifdef ESPHOME_MULTI_CORE_ATOMICS // This is the multi core with atomics implementation. // - // The implementation handles the 32-bit rollover (every 49.7 days) by: - // 1. Using a lock when detecting rollover to ensure atomic update - // 2. Restricting normal updates to forward movement within the same epoch - // This prevents race conditions at the rollover boundary without requiring - // 64-bit atomics or locking on every call. + // Uses atomic operations with acquire/release semantics to ensure coherent + // reads of millis_major_ and last_millis_ across cores. Features: + // 1. Epoch-coherency retry loop to handle concurrent updates + // 2. Lock only taken for actual rollover detection and update + // 3. Lock-free CAS updates for normal forward time progression + // 4. Memory ordering ensures cores see consistent time values for (;;) { uint16_t major = this->millis_major_.load(std::memory_order_acquire); From 152e3ee587ef1f83d076d3b0d087eb956943a484 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 19 Jul 2025 10:43:57 -1000 Subject: [PATCH 8/9] make more readable --- esphome/core/scheduler.cpp | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/esphome/core/scheduler.cpp b/esphome/core/scheduler.cpp index ca267cbe27..40034da898 100644 --- a/esphome/core/scheduler.cpp +++ b/esphome/core/scheduler.cpp @@ -538,12 +538,11 @@ uint64_t Scheduler::millis_64_(uint32_t now) { #ifdef ESPHOME_MULTI_CORE_NO_ATOMICS // This is the multi core no atomics implementation. // - // Without atomics, this implementation uses locks more aggressively: - // 1. Always locks when near the rollover boundary (within 10 seconds) - // 2. Always locks when detecting a large backwards jump - // 3. Updates without lock in normal forward progression (accepting minor races) - // This is less efficient but necessary without atomic operations. - + // The implementation handles the 32-bit rollover (every 49.7 days) by: + // 1. Using a lock when detecting rollover to ensure atomic update + // 2. Restricting normal updates to forward movement within the same epoch + // This prevents race conditions at the rollover boundary without requiring + // 64-bit atomics or locking on every call. uint16_t major = this->millis_major_; uint32_t last = this->last_millis_; @@ -589,12 +588,11 @@ uint64_t Scheduler::millis_64_(uint32_t now) { #ifdef ESPHOME_MULTI_CORE_ATOMICS // This is the multi core with atomics implementation. // - // Uses atomic operations with acquire/release semantics to ensure coherent - // reads of millis_major_ and last_millis_ across cores. Features: - // 1. Epoch-coherency retry loop to handle concurrent updates - // 2. Lock only taken for actual rollover detection and update - // 3. Lock-free CAS updates for normal forward time progression - // 4. Memory ordering ensures cores see consistent time values + // The implementation handles the 32-bit rollover (every 49.7 days) by: + // 1. Using a lock when detecting rollover to ensure atomic update + // 2. Restricting normal updates to forward movement within the same epoch + // This prevents race conditions at the rollover boundary without requiring + // 64-bit atomics or locking on every call. for (;;) { uint16_t major = this->millis_major_.load(std::memory_order_acquire); From 9119ac1c322b19372e2187c104af7d3dfbe1e161 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 19 Jul 2025 10:50:40 -1000 Subject: [PATCH 9/9] fix stale comments --- esphome/core/scheduler.cpp | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/esphome/core/scheduler.cpp b/esphome/core/scheduler.cpp index 40034da898..7665e04368 100644 --- a/esphome/core/scheduler.cpp +++ b/esphome/core/scheduler.cpp @@ -538,11 +538,11 @@ uint64_t Scheduler::millis_64_(uint32_t now) { #ifdef ESPHOME_MULTI_CORE_NO_ATOMICS // This is the multi core no atomics implementation. // - // The implementation handles the 32-bit rollover (every 49.7 days) by: - // 1. Using a lock when detecting rollover to ensure atomic update - // 2. Restricting normal updates to forward movement within the same epoch - // This prevents race conditions at the rollover boundary without requiring - // 64-bit atomics or locking on every call. + // Without atomics, this implementation uses locks more aggressively: + // 1. Always locks when near the rollover boundary (within 10 seconds) + // 2. Always locks when detecting a large backwards jump + // 3. Updates without lock in normal forward progression (accepting minor races) + // This is less efficient but necessary without atomic operations. uint16_t major = this->millis_major_; uint32_t last = this->last_millis_; @@ -588,11 +588,12 @@ uint64_t Scheduler::millis_64_(uint32_t now) { #ifdef ESPHOME_MULTI_CORE_ATOMICS // This is the multi core with atomics implementation. // - // The implementation handles the 32-bit rollover (every 49.7 days) by: - // 1. Using a lock when detecting rollover to ensure atomic update - // 2. Restricting normal updates to forward movement within the same epoch - // This prevents race conditions at the rollover boundary without requiring - // 64-bit atomics or locking on every call. + // Uses atomic operations with acquire/release semantics to ensure coherent + // reads of millis_major_ and last_millis_ across cores. Features: + // 1. Epoch-coherency retry loop to handle concurrent updates + // 2. Lock only taken for actual rollover detection and update + // 3. Lock-free CAS updates for normal forward time progression + // 4. Memory ordering ensures cores see consistent time values for (;;) { uint16_t major = this->millis_major_.load(std::memory_order_acquire);