From 3912ede2cf5b9eb5884ec09641986e037a9263d1 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 14 Apr 2026 20:42:25 -1000 Subject: [PATCH 1/4] safer --- esphome/components/esp8266/core.cpp | 36 +++++++++++++++++++++-------- 1 file changed, 26 insertions(+), 10 deletions(-) diff --git a/esphome/components/esp8266/core.cpp b/esphome/components/esp8266/core.cpp index 7551ecc3a7..1cca79f104 100644 --- a/esphome/components/esp8266/core.cpp +++ b/esphome/components/esp8266/core.cpp @@ -34,9 +34,10 @@ void HOT yield() { ::yield(); } // that Arduino library code and ISR handlers (e.g. Wiegand, ZyAura) calling // ::millis() directly also get the fast version. Interrupts are briefly disabled // to protect the static state from concurrent ISR access. The critical section -// is bounded: the common path (delta < 10 ms) runs at most 10 subtract-and- -// compare iterations (~100 ns). Large gaps (WiFi scan, boot) fall back to a -// constant-time multiply-by-reciprocal (~2.5 μs, rare). +// is bounded on *both* paths: the common path (delta < 10 ms) runs at most 10 +// subtract-and-compare iterations (~100 ns). The rare path drops out of the +// critical section before doing the expensive /1000, so unrelated peripherals +// and the WiFi MAC are not blocked while the divide runs. // Threshold above which we use constant-time /1000 instead of the while loop. // 10 ms means the while loop runs at most 10 iterations (~100 ns) on the // common path, well within the WiFi stack's ~10 μs interrupt latency budget. @@ -56,20 +57,35 @@ uint32_t IRAM_ATTR HOT millis() { uint32_t delta = now_us - state.last_us; state.last_us = now_us; state.remainder += delta; - if (state.remainder >= MILLIS_RARE_PATH_THRESHOLD_US) { - // Rare path: large gap (WiFi scan, boot, long block). Constant-time - // conversion keeps the critical section bounded. - uint32_t ms = state.remainder / US_PER_MS; - state.cache += ms; - state.remainder -= ms * US_PER_MS; - } else { + if (state.remainder < MILLIS_RARE_PATH_THRESHOLD_US) { // Common path: small gap. Loop runs at most // MILLIS_RARE_PATH_THRESHOLD_US / US_PER_MS iterations. while (state.remainder >= US_PER_MS) { state.cache++; state.remainder -= US_PER_MS; } + uint32_t result = state.cache; + xt_wsr_ps(ps); + return result; } + // Rare path: large gap (WiFi scan, boot, long block). Take the accumulated + // μs out of the shared state, drop the critical section, then divide. This + // keeps interrupts unmasked during the ~2.5 μs __umulsidi3 — the rare path + // fires precisely when the system was already blocked, which is also when + // WiFi/peripheral ISRs most need to run on time. + // + // Concurrency on single-core ESP8266: only an ISR can preempt us. An ISR + // that calls millis() during the divide sees remainder=0 and a tiny delta, + // takes the common path, and returns the pre-commit cache. Monotonic — and + // any μs it adds to remainder are preserved by the `+=` on commit. + uint32_t pending_us = state.remainder; + state.remainder = 0; + xt_wsr_ps(ps); + uint32_t ms_to_add = pending_us / US_PER_MS; + uint32_t us_left = pending_us - ms_to_add * US_PER_MS; + ps = xt_rsil(15); + state.cache += ms_to_add; + state.remainder += us_left; uint32_t result = state.cache; xt_wsr_ps(ps); return result; From cf994c3b9f307fb3f4b8a13c49a56f87815ce0b3 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 14 Apr 2026 20:48:50 -1000 Subject: [PATCH 2/4] back to inlock divide .. was overkill and 20% regression --- esphome/components/esp8266/core.cpp | 36 ++++++++--------------------- 1 file changed, 10 insertions(+), 26 deletions(-) diff --git a/esphome/components/esp8266/core.cpp b/esphome/components/esp8266/core.cpp index 1cca79f104..7551ecc3a7 100644 --- a/esphome/components/esp8266/core.cpp +++ b/esphome/components/esp8266/core.cpp @@ -34,10 +34,9 @@ void HOT yield() { ::yield(); } // that Arduino library code and ISR handlers (e.g. Wiegand, ZyAura) calling // ::millis() directly also get the fast version. Interrupts are briefly disabled // to protect the static state from concurrent ISR access. The critical section -// is bounded on *both* paths: the common path (delta < 10 ms) runs at most 10 -// subtract-and-compare iterations (~100 ns). The rare path drops out of the -// critical section before doing the expensive /1000, so unrelated peripherals -// and the WiFi MAC are not blocked while the divide runs. +// is bounded: the common path (delta < 10 ms) runs at most 10 subtract-and- +// compare iterations (~100 ns). Large gaps (WiFi scan, boot) fall back to a +// constant-time multiply-by-reciprocal (~2.5 μs, rare). // Threshold above which we use constant-time /1000 instead of the while loop. // 10 ms means the while loop runs at most 10 iterations (~100 ns) on the // common path, well within the WiFi stack's ~10 μs interrupt latency budget. @@ -57,35 +56,20 @@ uint32_t IRAM_ATTR HOT millis() { uint32_t delta = now_us - state.last_us; state.last_us = now_us; state.remainder += delta; - if (state.remainder < MILLIS_RARE_PATH_THRESHOLD_US) { + if (state.remainder >= MILLIS_RARE_PATH_THRESHOLD_US) { + // Rare path: large gap (WiFi scan, boot, long block). Constant-time + // conversion keeps the critical section bounded. + uint32_t ms = state.remainder / US_PER_MS; + state.cache += ms; + state.remainder -= ms * US_PER_MS; + } else { // Common path: small gap. Loop runs at most // MILLIS_RARE_PATH_THRESHOLD_US / US_PER_MS iterations. while (state.remainder >= US_PER_MS) { state.cache++; state.remainder -= US_PER_MS; } - uint32_t result = state.cache; - xt_wsr_ps(ps); - return result; } - // Rare path: large gap (WiFi scan, boot, long block). Take the accumulated - // μs out of the shared state, drop the critical section, then divide. This - // keeps interrupts unmasked during the ~2.5 μs __umulsidi3 — the rare path - // fires precisely when the system was already blocked, which is also when - // WiFi/peripheral ISRs most need to run on time. - // - // Concurrency on single-core ESP8266: only an ISR can preempt us. An ISR - // that calls millis() during the divide sees remainder=0 and a tiny delta, - // takes the common path, and returns the pre-commit cache. Monotonic — and - // any μs it adds to remainder are preserved by the `+=` on commit. - uint32_t pending_us = state.remainder; - state.remainder = 0; - xt_wsr_ps(ps); - uint32_t ms_to_add = pending_us / US_PER_MS; - uint32_t us_left = pending_us - ms_to_add * US_PER_MS; - ps = xt_rsil(15); - state.cache += ms_to_add; - state.remainder += us_left; uint32_t result = state.cache; xt_wsr_ps(ps); return result; From 1a23d01511b0832b2c922554d232b6aa5c84cdf5 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 22 Apr 2026 07:52:51 +0200 Subject: [PATCH 3/4] Address review: tighten millis() / delay() comments, fix off-by-one wording --- esphome/components/esp8266/core.cpp | 57 +++++++++++------------------ 1 file changed, 21 insertions(+), 36 deletions(-) diff --git a/esphome/components/esp8266/core.cpp b/esphome/components/esp8266/core.cpp index 7551ecc3a7..b25e9f7ef8 100644 --- a/esphome/components/esp8266/core.cpp +++ b/esphome/components/esp8266/core.cpp @@ -16,30 +16,19 @@ extern "C" { namespace esphome { void HOT yield() { ::yield(); } -// Arduino ESP8266's millis() uses 4× 64-bit multiplies with magic constants to -// convert system_get_time() → ms while tracking overflow (~3.3 μs per call on -// the LX106 which has no hardware multiply-high instruction). We replace it with -// a simple accumulator that tracks a running millis counter from μs deltas using -// pure 32-bit ops on the common path (subtract, add, compare-and-subtract). -// Large gaps (>10 ms) fall back to a constant-time /1000 conversion. +// Fast accumulator replacement for Arduino's millis() (~3.3 μs via 4× 64-bit +// multiplies on the LX106). Tracks a running ms counter from 32-bit +// system_get_time() deltas using pure 32-bit ops. Installed as __wrap_millis +// (via -Wl,--wrap=millis) so Arduino libs and IRAM_ATTR ISR handlers (e.g. +// Wiegand, ZyAura) also get the fast version. xt_rsil(15) guards the static +// state against ISR re-entry; the critical section is bounded (≤9 while-loop +// iterations, ~100 ns on the common path, or a constant-time /1000 ~2.5 μs on +// the rare path — well under WiFi's ~10 μs ISR latency budget). // -// Overflow safety: system_get_time() is a uint32_t that wraps every ~71.6 minutes. -// Unsigned subtraction (now - last) handles one wrap correctly. ESPHome calls -// millis() thousands of times per second (1+N per loop iteration at 60+ Hz), so -// missing a full 71-minute wrap period is not a realistic concern. At boot, -// state.last_us starts at 0 and system_get_time() counts from 0, so the first call's -// delta equals the real elapsed time — no special initialization needed. -// -// This function is also installed as __wrap_millis (via -Wl,--wrap=millis) so -// that Arduino library code and ISR handlers (e.g. Wiegand, ZyAura) calling -// ::millis() directly also get the fast version. Interrupts are briefly disabled -// to protect the static state from concurrent ISR access. The critical section -// is bounded: the common path (delta < 10 ms) runs at most 10 subtract-and- -// compare iterations (~100 ns). Large gaps (WiFi scan, boot) fall back to a -// constant-time multiply-by-reciprocal (~2.5 μs, rare). -// Threshold above which we use constant-time /1000 instead of the while loop. -// 10 ms means the while loop runs at most 10 iterations (~100 ns) on the -// common path, well within the WiFi stack's ~10 μs interrupt latency budget. +// Overflow: system_get_time() wraps every ~71.6 min; unsigned now_us - last_us +// handles one wrap. Both the ESPHome main loop (1+N millis() calls per +// iteration at 60+ Hz) and esphome::delay() (which polls millis() in its wait +// loop) keep state.last_us fresh well inside the 71-minute window. static constexpr uint32_t MILLIS_RARE_PATH_THRESHOLD_US = 10000; static constexpr uint32_t US_PER_MS = 1000; @@ -63,8 +52,7 @@ uint32_t IRAM_ATTR HOT millis() { state.cache += ms; state.remainder -= ms * US_PER_MS; } else { - // Common path: small gap. Loop runs at most - // MILLIS_RARE_PATH_THRESHOLD_US / US_PER_MS iterations. + // Common path: small gap. while (state.remainder >= US_PER_MS) { state.cache++; state.remainder -= US_PER_MS; @@ -75,17 +63,14 @@ uint32_t IRAM_ATTR HOT millis() { return result; } uint64_t millis_64() { return Millis64Impl::compute(millis()); } -// Avoid calling ::delay() which pulls in __delay from core_esp8266_wiring.cpp. -// __delay has an intra-object call to the original millis() that --wrap=millis -// can't intercept, preventing the linker from garbage-collecting the expensive -// original millis body (~80 bytes IRAM). -// -// Semantic difference from Arduino's delay(): Arduino sets up a one-shot -// os_timer and calls esp_suspend() to suspend the continuation once for the -// full duration. Our loop polls millis() + optimistic_yield(1000) which still -// calls esp_schedule()/esp_suspend_within_cont() via yield(), so SDK tasks -// and WiFi run correctly. Less power-efficient for long delays but -// functionally equivalent. +// Poll-based delay that avoids ::delay() — Arduino's __delay has an intra-object +// call to the original millis() that --wrap can't intercept, so calling ::delay() +// would keep the slow Arduino millis body alive in IRAM. optimistic_yield still +// enters esp_schedule()/esp_suspend_within_cont() via yield(), so SDK tasks and +// WiFi run correctly. Theoretically less power-efficient than Arduino's +// os_timer-based delay() for long waits, but nearly all ESPHome delays are short +// (sensor/I²C/SPI settling in the 1–100 ms range) where the difference is +// negligible. void HOT delay(uint32_t ms) { if (ms == 0) { optimistic_yield(1000); From 12ad6637232d667cc652265abbc7af2baf711956 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 22 Apr 2026 07:54:11 +0200 Subject: [PATCH 4/4] Address review: NMI note + static_assert hard-encoding remainder+delta overflow invariant --- esphome/components/esp8266/core.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/esphome/components/esp8266/core.cpp b/esphome/components/esp8266/core.cpp index b25e9f7ef8..62907ac6aa 100644 --- a/esphome/components/esp8266/core.cpp +++ b/esphome/components/esp8266/core.cpp @@ -23,7 +23,8 @@ void HOT yield() { ::yield(); } // Wiegand, ZyAura) also get the fast version. xt_rsil(15) guards the static // state against ISR re-entry; the critical section is bounded (≤9 while-loop // iterations, ~100 ns on the common path, or a constant-time /1000 ~2.5 μs on -// the rare path — well under WiFi's ~10 μs ISR latency budget). +// the rare path — well under WiFi's ~10 μs ISR latency budget). NMIs (level +// >15) are not masked, but the ESP8266 SDK's NMI handlers don't call millis(). // // Overflow: system_get_time() wraps every ~71.6 min; unsigned now_us - last_us // handles one wrap. Both the ESPHome main loop (1+N millis() calls per @@ -31,6 +32,10 @@ void HOT yield() { ::yield(); } // loop) keep state.last_us fresh well inside the 71-minute window. static constexpr uint32_t MILLIS_RARE_PATH_THRESHOLD_US = 10000; static constexpr uint32_t US_PER_MS = 1000; +// Guarantees state.remainder + delta cannot wrap uint32_t: the threshold caps +// remainder on entry, and delta ≤ UINT32_MAX (one system_get_time() wrap). +static_assert(MILLIS_RARE_PATH_THRESHOLD_US < UINT32_MAX / 2, + "threshold must leave headroom so remainder + delta cannot overflow uint32_t"); uint32_t IRAM_ATTR HOT millis() { // Struct packs the three statics so the compiler loads one base address