From da9c44d28dc5e247867e5a998d1a3c3e31e37698 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 11 Apr 2026 23:21:15 -1000 Subject: [PATCH] [esp8266] Bound the critical section in millis() accumulator MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Split the μs→ms conversion into two paths to keep the interrupt- disabled critical section bounded: - Common path (delta < 10 ms): while loop runs at most 10 iterations (~100 ns). This covers the normal hot-path case where millis() is called thousands of times per second. - Rare path (delta >= 10 ms): constant-time multiply-by-reciprocal via /1000 (compiled to __umulsidi3, ~2.5 μs). Only fires after a long block (WiFi scan, boot, component stall), where the extra latency is negligible relative to the block that caused it. Addresses the concern that a multi-second WiFi scan block could cause the unbounded while loop to hold interrupts for tens of microseconds. --- esphome/components/esp8266/core.cpp | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/esphome/components/esp8266/core.cpp b/esphome/components/esp8266/core.cpp index 02aea6f2e8d..f7106e077e5 100644 --- a/esphome/components/esp8266/core.cpp +++ b/esphome/components/esp8266/core.cpp @@ -33,8 +33,10 @@ void HOT yield() { ::yield(); } // 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 -// (~10 instructions, ~125 ns at 80 MHz) to protect the static state from -// concurrent ISR access. +// 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). uint32_t IRAM_ATTR HOT millis() { // Struct packs the three statics so the compiler loads one base address // instead of three separate literal pool entries (saves ~8 bytes IRAM). @@ -48,9 +50,22 @@ uint32_t IRAM_ATTR HOT millis() { uint32_t delta = now_us - state.last_us; state.last_us = now_us; state.remainder += delta; - while (state.remainder >= 1000) { - state.cache++; - state.remainder -= 1000; + if (state.remainder >= 10000) { + // Rare path: large gap (>10 ms — WiFi scan, boot, long block). + // Use constant-time multiply-by-reciprocal (compiled to __umulsidi3, + // ~2.5 μs) to keep the critical section bounded. Only fires when the + // caller was already blocked for >10 ms, so the extra latency is + // negligible relative to the block that caused it. + uint32_t ms = state.remainder / 1000; + state.cache += ms; + state.remainder -= ms * 1000; + } else { + // Common path: small gap (<10 ms). Loop runs at most 10 times + // (~100 ns), well within the WiFi stack's ~10 μs interrupt budget. + while (state.remainder >= 1000) { + state.cache++; + state.remainder -= 1000; + } } uint32_t result = state.cache; xt_wsr_ps(ps);