Project's .clang-tidy applies different naming rules to static methods
(ClassMethodCase, no suffix) vs instance methods (PrivateMethodCase /
ProtectedMethodCase, _ suffix required). The helper had a trailing _ but
was static, so clang-tidy flagged it. Drop static -- the helper is already
noinline'd and called only at trim time, so the hidden this arg is free
in any meaningful sense.
The three swap-with-copy lines in trim_freelist() inlined the
construct-swap-destruct dance per call site, costing ~440 B flash on ESP32.
Move the swap-shrink into a noinline private static helper so all three
callers share one body. Net flash cost of vector shrinking drops to ~296 B
(from ~444 B) while preserving the same RAM-reclaim behaviour.
shrink_to_fit() is a non-binding hint that the toolchain ignores, so the
swap-with-copy idiom is still required.
The freelist holds the boot-peak count of recycled SchedulerItem*; items_,
to_add_, and defer_queue_ hold the boot-peak vector *capacity* of live
SchedulerItem*. std::vector grows by doubling and retains capacity even when
items drain, so post-boot the vector slack can be larger than the freelist
itself. Swap each with a same-content copy to size them exactly to their
current contents.
Live items are preserved -- the swap-copy idiom builds the new vector from
the existing pointers, then the old (over-capacity) vector is destroyed.
Boot churn (component init, first sensor reads, retries) inflates the freelist
beyond the steady-state high-water mark. Without a one-shot trim that peak
would be retained forever. Adds Scheduler::trim_freelist() and schedules it
from Application::setup() to fire SCHEDULER_FREELIST_TRIM_DELAY_MS (10s) after
setup completes -- well past the bulk of post-setup async work.
Items currently in items_/to_add_/defer_queue_ are untouched; only the
freelist's recycled items are deleted. Post-trim, the freelist regrows to
the new (post-startup) high-water mark.
set_timer_common_'s self-key cancel path scans items_ and to_add_ linearly,
so the cancel-and-replace is O(N) in the global scheduler item count, not
O(1). Reword to describe the behavior without the complexity claim.
Address Copilot review feedback.
Adds an assertion that the observed peak pool size exceeds the old
MAX_POOL_SIZE=5 cap. Without this, a silent regression that re-introduced a
small cap could pass the existing pool_full_count == 0 invariant. Phase 5 + 6
of the fixture schedule 8 + 10 same-component timeouts, so the peak should
comfortably exceed 5.
Address Copilot review feedback on PR.
Migrates TimeoutFilterBase / TimeoutFilterLast / TimeoutFilterConfigured off
Component, mirroring the migration #16132 did for the other Component-based
sensor filters. They now arm/re-arm via App.scheduler.set_timeout(this, ...)
keyed on the filter pointer; the scheduler cancels and replaces any pending
arm in O(1) on each new_value(). Filters live for the program's lifetime, so
the self-key never dangles.
Net effect: this reverts the design from #11922, which moved timeout filters
off the scheduler specifically because the bounded SchedulerItem pool churned
on devices with many timers (LD2450 etc.). With #16172 replacing that pool
with an unbounded intrusive freelist, the original churn problem is gone and
the scheduler is once again the right primitive for this workload.
Per-instance footprint shrinks: lose Component (second vptr, packed bookkeeping
bytes, ComponentRuntimeStats block when runtime_stats: is enabled) and drop
the now-unused timeout_start_time_ field. get_setup_priority() and the loop()
poll go with it.
Sensor timeout-filter syntax is unchanged. No user-facing change.
The fixed MAX_POOL_SIZE=5 cap was the source of the heap churn the pool was
meant to prevent: any device with more than 5 concurrent timers (e.g. a board
with 30+ LD2450 sensors) hit a steady-state oscillation of recycle->delete and
acquire->new on every loop iteration.
Replace std::vector<SchedulerItem*> with a singly-linked freelist threaded
through SchedulerItem::next_free, which shares storage with `component` via an
anonymous union (zero per-item overhead -- the component pointer is dead while
pooled). Drop the cap entirely: the freelist quiesces at the application's
natural concurrent-timer high-water mark, which is the working set the device
already needs while those timers are active.
No std::vector means no growth-doubling slack and no realloc copies during
warm-up. Caller of get_item_from_pool_locked_() must overwrite item->component
before unlocking (already true at the sole call site); nullptr remains a valid
live `component` value for SELF_POINTER items, so we cannot pre-clear it.
Saves 4 B per ThrottleAverageFilter instance (28 B → 24 B on 32-bit).
have_nan_ is a single boolean that previously cost 4 B due to padding;
fold it into the high bit of n_ as a 31-bit + 1-bit bitfield.
To guarantee n_ cannot overflow under realistic configurations, cap
the YAML time_period at 24 h. At a pessimistic 1 kHz source rate the
counter peaks at 86.4M, leaving 25x headroom against 2^31. Anyone
needing multi-day rolling averages should be using a different
filtering strategy anyway.
Breaking change: configurations with throttle_average periods longer
than 24 h will now fail validation. No real-world configs are
expected to use such values.
The previous gate captured each subexpression in a named local
(high_frequency, elapsed, woke). Locals are easy to reuse later in
the function, and we have hit bugs from stale reuse before — most
recently is_high_frequency being read again instead of re-checked.
Inline the expression directly into do_component_phase so there is
nothing tempting to reuse downstream.
Both messages are unconditionally exercised in the proxy hot paths
(every Z-Wave frame received from the controller; every UART read on a
configured serial proxy), so the modest flash cost of forcing -O2 on
their encode/calculate_size pays off on every device that has the proxy
enabled. Same treatment as InfraredRFReceiveEvent in the prior commit
and the other high-volume server-emitted messages.
This message is emitted on every IR/RF receive event with a packed sint32
timings array (typically ~50-200 entries). The encode walks the vector
calling ProtoEncode::encode_sint32 per element; under -Os GCC does not
inline those helpers, so each element pays the call overhead.
Adding option (speed_optimized) = true to the proto definition causes
the codegen to emit __attribute__((optimize("O2"))) on encode and
calculate_size, matching what already exists on SensorStateResponse,
SubscribeLogsResponse, and BluetoothLERawAdvertisementsResponse — the
other high-volume server-emitted messages.
The CodSpeed Encode_InfraredRFReceiveEvent / CalculateSize_InfraredRFReceiveEvent
benchmarks added in the parent PR will quantify the improvement.
- Make UARTFlushResult in the serial_proxy stub a scoped enum class with
matching scoped enumerator return in flush_port(), so the stub
signature lines up with the real esphome::uart::UARTFlushResult.
- Replace heap-leaking lazy-init in get_ir_timings_100() with a
function-local static const std::vector populated by a regular helper
function. Same lazy-init behavior, no leak in valgrind/ASan, no lambda
IIFE.
- Emit field 6 (modulation = 1) in build_infrared_rf_transmit_wire() so
the bytes match the documented field list and the decode benchmark
also exercises the field-6 decode_varint path.
Simplifies the decode benchmarks to mirror the encode pattern more
closely: no per-iteration asm volatile barrier, no return-by-value of
APIBuffer through encode_message_for_proxy. CodSpeed callgrind has been
crashing inside Decode_ZWaveProxyFrame and the previous setup was the
main thing it had that the (passing) Encode_ZWaveProxyFrame did not.
Splitting these out from bench_proto_encode.cpp and bench_proto_decode.cpp
moves them to the end of the linker's static-init order. CodSpeed's
callgrind runner has been segfaulting immediately after measuring the
last existing decode benchmark (Decode_SwitchCommandRequest), and
isolating the new code into its own translation unit lets us see whether
the crash is triggered by one of the new benchmarks or by something
about the new USE_*_PROXY/USE_INFRARED/USE_RADIO_FREQUENCY defines
changing how api_pb2.cpp compiles.
The InfraredRFReceiveEvent encode benchmark used a C++17 lambda IIFE
(`[]{...}()`) to seed a function-static vector, and the
InfraredRFTransmitRawTimingsRequest decode benchmark grew its APIBuffer
one byte at a time (~210 grow_() calls), each allocating a fresh
exact-fit buffer and memcpy'ing the prior contents. Both patterns are
fine under direct execution but appear to hit a CodSpeed/valgrind
edge case during the simulated benchmark run.
Switch to a plain heap-init pattern for the vector and build the wire
bytes into a stack array first, then resize+memcpy into the APIBuffer
once.
Mirrors the existing BluetoothLERawAdvertisementsResponse benchmarks for
the remaining proxy message families: ZWaveProxyFrame/ZWaveProxyRequest,
SerialProxyDataReceived/SerialProxyWriteRequest, and
InfraredRFReceiveEvent/InfraredRFTransmitRawTimingsRequest.
Adds minimal stub headers under tests/benchmarks/stubs/ for the
zwave_proxy, infrared, radio_frequency, and serial_proxy components so
api_connection.cpp compiles without dragging in their UART/RMT/BLE
hardware dependencies.
Followup to #16129. DelayAction now keys its scheduler entry by
self-pointer (NameType::SELF_POINTER), so the DELAY_ACTION enum
value is no longer referenced anywhere.
Self-keyed scheduler items (Scheduler::set_timeout(self, ...) and
set_interval(self, ...)) intentionally store component == nullptr. When
USE_RUNTIME_STATS is enabled, WarnIfComponentBlockingGuard::finish()
dereferenced component_ to call runtime_stats_.record_time(), causing a
load access fault on RISC-V (e.g. ESP32-C3) when these timers fire.
Skip the per-component bookkeeping when component_ is null, but still
accumulate into ComponentRuntimeStats::global_recorded_us so
Application::loop() overhead accounting (which subtracts scheduled
callback time from before_loop_tasks_) stays accurate.
J. Nick Koston