perf: stop per-frame hardware endpoint re-enumeration by parking failed output sinks

docs/adr/2026-07-31-output-sink-retry-policy.md

@@ -0,0 +1,105 @@
+# ADR: Output sinks wait for a hardware change instead of asking every frame
+
+- **Status:** Accepted
+- **Date:** 2026-07-31
+- **Deciders:** Photomancer
+- **Supersedes:** None
+- **Superseded by:** None
+- **Relates:** `docs/debt/s3-frame-cost-scales-per-fixture.md` (the measurement
+  that prompted this; its other half — per-frame dataflow re-resolution — is a
+  separate change)
+
+## Context
+
+An output sink whose channel is not open re-attempted `OutputProvider::open`
+on every flush, which is every frame. For a sink that *can* open, that is one
+attempt and then nothing. For a sink that cannot — a project authored for
+`ws281x:rmt:D9` loaded onto a board that has no `D9`, a pin another node holds,
+a fifth strip on a four-channel board — it is an attempt per frame, forever.
+
+Each attempt is not cheap. Opening by spec enumerates every endpoint every
+registered driver offers; each endpoint is constructed with a formatted spec
+string, parsed and validated, and carries a freshly computed status, which is
+itself several registry lookups. On `projects/test/quad-strips` under the
+emulator (esp32-c6 cycle model), where three of four authored endpoints do not
+exist on the virtual board, this was **45.8% of all self cycles** in
+`HwRegistry::endpoint_status_for` and **90.2% inclusive** under
+`OutputProvider::open` — the frame spent mostly on relearning the same "no",
+sixty times a second, plus a `log::warn!` per failed sink per frame.
+
+The obvious framing — "cache endpoint status in the registry" — is the wrong
+one. A sink that opened successfully never asks again, so there is no repeated
+query to memoize on the success path; the repetition is entirely the retry
+loop. And caching status is actively dangerous: a cached `Available` on a pin
+that has since been claimed would hand two owners the same hardware. The
+question was never "what did status say last time" but "is it worth asking
+again at all".
+
+## Decision
+
+**Sinks park on failure, and a hardware generation counter tells them when to
+wake.**
+
+Three pieces, one per layer:
+
+1. `HwRegistry::generation() -> u64` changes whenever a claim succeeds or a
+   lease is released — the only two events that can turn a refusal into an
+   acceptance. Reserved status and capability support come from the manifest,
+   which is immutable after construction, so they need no signal. Failed
+   claims deliberately do **not** bump: signalling them would let one hopeless
+   endpoint wake every parked sink on every attempt, which is the storm this
+   ends.
+
+2. `OutputProvider::hardware_generation() -> u64` forwards it. The default is
+   `0` — "this hardware never changes" — which is the honest answer for a
+   provider that owns no registry: its failures are about the caller's own
+   configuration, not about ownership. Registry-backed providers forward;
+   wrappers delegate.
+
+3. `EngineServices` records `parked_at_generation` on a sink whose open
+   failed, and skips that sink while the generation still matches. The value
+   is sampled **before** any open in the flush, so a claim made by this frame's
+   own opens cannot park a sink against a number that already includes it.
+   Parking is cleared on re-authored config, on provider swap, and on success.
+
+Consequences worth stating:
+
+- A parked sink does not fail its frame. The first failure is logged and
+  returned exactly as before; subsequent frames are silent and free. Recovery
+  logs one line. This turns per-frame log spam — which on silicon is per-frame
+  serial traffic — into two lines per episode.
+- Recovery latency is unchanged: a released pin bumps the generation, and the
+  next flush retries. Nothing polls, nothing waits out a timer.
+- Nothing caches endpoint status. Reserved pins and claim conflicts are still
+  computed from live registry state at every open attempt, so the semantics
+  that would be dangerous to stale cannot go stale.
+
+## Alternatives considered
+
+**Cache `HwEndpointStatus` in the registry.** Rejected: it optimizes a query
+the success path does not make, and a stale `Available` on a claimed pin is a
+correctness bug in exchange for no measured win.
+
+**Time-based backoff** (retry a failed sink about once a second). Rejected: it
+delays legitimate recovery by an arbitrary constant, keeps re-enumerating
+forever for endpoints that will never exist, and puts a timing policy in a
+layer that has no clock of its own. It would also still need the generation to
+recover promptly, so it is strictly additional machinery.
+
+**Give up on failed sinks permanently.** Rejected: contention is normal and
+transient. A sink that lost a race for the RMT channel must light when the
+winner releases it, and today it does.
+
+If a transient failure ever appears that no claim or release follows — an RMT
+peripheral that fails to initialize and later would not — a slow fallback
+retry can be added on top of this without changing the contract.
+
+## Notes
+
+The measured win is emulator-side, because that board really does lack the
+authored pins. On the desk ESP32-S3 all four channels open on the first frame,
+so steady-state silicon paid little for this and gains little back; its own
+flat per-fixture cost is the dataflow resolver, tracked separately in the debt
+entry. What silicon does gain is that a *misconfigured* output — the common
+authoring mistake — no longer costs a frame's worth of enumeration and a line
+of serial output every frame it stays wrong.

docs/debt/s3-frame-cost-scales-per-fixture.md

@@ -68,6 +68,60 @@ nothing else helps, by measurement.
   resolution machinery (dataflow resolver + endpoint status) instead. The
   suspicious shapes: `clear_frame_cache` discarding all resolution work every
   tick, and endpoint status recomputed per frame per channel.
+- **2026-07-31 (later)** — **Endpoint-status half closed** (PR #244,
+  plan `2026-07-31-2224-hw-endpoint-status-cache`, ADR
+  `2026-07-31-output-sink-retry-policy.md`). The 45.8% was not status lookup
+  but a **failed-open retry storm**: `ensure_channel_open` re-attempted any
+  handle-less sink every frame, and the emulator board declared one WS281x
+  channel and no `D9`/`D8`/`D7`, so three of quad-strips' four sinks could
+  never open and re-enumerated the whole board — 256 endpoints, each with a
+  formatted spec and a live status — sixty times a second, forever.
+
+  Fixed by *not asking*, not by caching: sinks park on a new
+  `HwRegistry::generation()` (bumped only on successful claim/release) and
+  wake when hardware ownership actually moves. No endpoint status is stored
+  anywhere, so reserved-pin and claim-conflict semantics cannot go stale. Also
+  collapsed the 3 enumerations per open attempt to 1, and stopped
+  `refresh_output_sink_configs` cloning every output def per tick.
+
+  Measured, frame-for-frame (8 frames both runs, `events.jsonl` B→E):
+  **steady frame 16.42M → 1.53M cycles, 10.7×**; total attributed 65.8M →
+  6.2M. `endpoint_status_for`, `VirtualWs281xDriver::endpoints`,
+  `endpoint_for_spec`, `validate_spec` and the `core::fmt` machinery are all
+  **absent from the top-20 self cycles**. Per-frame warn spam → 7 lines for
+  the whole run. Profiles:
+  `2026-07-31T22-42-28--…quad-strips--steady-render` (before) and
+  `…23-29-27` (after); `…23-33-40` is after the emulator board was given the
+  S3's four channels (steady frame 1.554M — the +1.3% is three more strips
+  actually being written).
+
+  **Desk-S3 re-measured 2026-08-01** (d8:3b:da:47:29:70, identified by MAC via
+  `espflash board-info`; branch firmware flashed, quad-strips pushed):
+  **20 fps, tick 48 ms — flat**, stable over 13 consecutive `[perf]` readings.
+  That is exactly the prediction: the S3 opens all four channels on frame one,
+  so it never paid this cost in steady state, and its flat ~8.4 ms/fixture is
+  the resolver.
+
+  Since an unchanged fps cannot itself prove the new image was running, the
+  parked-sink path was exercised on silicon instead: quad-strips with one
+  output re-pointed at `ws281x:rmt:NOT-A-PIN` produced **2 warnings and then
+  silence across ~1,250 frames** (the two being the designed settle — first
+  attempt, then one retry after the other three opens bumped the generation).
+  The old code logs one per frame, so this both proves the image and confirms
+  the fix on hardware. fps held at 20 with the dead output; tick 47 ms, the
+  1 ms being one fewer strip to write.
+
+  Not measured: the same misconfigured-output case on *pre-fix* firmware, which
+  would quantify what silicon saves there. The saving is a board enumeration
+  plus a serial line per frame; on the S3's ~40-resource manifest that is far
+  smaller than the emulator's 256, and it was not worth a second flash cycle to
+  put a number on.
+
+  **Still open: the resolver half** — the profile is now dominated by exactly
+  what this entry predicted would remain: memcpy 18.7%, allocator 12.8%+8.0%,
+  `QueryKey::eq` 5.7%, `EngineSession::resolve` 2.7%, `SlotPath::parse`. That
+  is the dataflow resolver re-resolving from cold each tick, owned by plan
+  `2026-07-31-2225-persist-dataflow-resolution`; `…23-33-40` is its baseline.
 
 **Exit criteria** — A profiled optimization pass that makes resolved bindings
 and endpoint status persist across frames (invalidate on tree/binding/

lp-app/lpa-server/src/project.rs

@@ -491,6 +491,10 @@ impl OutputProvider for SharedOutputProvider {
     fn close(&self, handle: OutputChannelHandle) -> Result<(), lpc_hardware::OutputError> {
         self.0.borrow().close(handle)
     }
+
+    fn hardware_generation(&self) -> u64 {
+        self.0.borrow().hardware_generation()
+    }
 }
 
 fn project_root_path(name: &str) -> Result<TreePath, ServerError> {

lp-core/lpc-engine/src/engine/engine.rs

@@ -314,15 +314,6 @@ impl Engine {
         }
     }
 
-    pub(crate) fn loaded_node_def_for_entry<'a, N>(
-        &self,
-        registry: &'a ProjectRegistry,
-        entry: &RuntimeNodeEntry<N>,
-    ) -> Option<&'a NodeDef> {
-        let location = entry.def_location.as_ref()?;
-        loaded_registry_def(registry, location).ok()
-    }
-
     // Consumed by texture and shader node test modules (and the
     // texture-def-root test in `project_read_stream`).
     #[cfg(all(test, any(feature = "node-texture", feature = "node-shader")))]
@@ -397,20 +388,31 @@ impl Engine {
         result
     }
 
+    /// Re-read every output's authored configuration for this tick.
+    ///
+    /// The tree and the services are borrowed as separate fields so the defs
+    /// can be read while the sinks are updated. Collecting the work first
+    /// instead cost a full clone of every output's definition — endpoint spec
+    /// and all — on every frame, to almost always discover that nothing had
+    /// changed.
     fn refresh_output_sink_configs(&mut self, registry: &ProjectRegistry) {
-        let mut updates = Vec::new();
-        for entry in self.tree.entries() {
-            let Some(buffer_id) = self.runtime_output_sink_buffer_id(entry.id) else {
+        let tree = &self.tree;
+        let services = &mut self.services;
+        for entry in tree.entries() {
+            let buffer_id = match entry.state.value() {
+                NodeEntryState::Alive(node) => node.runtime_output_sink_buffer_id(),
+                _ => None,
+            };
+            let Some(buffer_id) = buffer_id else {
                 continue;
             };
-            let Some(NodeDef::Output(def)) = self.loaded_node_def_for_entry(registry, entry) else {
+            let Some(location) = entry.def_location.as_ref() else {
                 continue;
             };
-            updates.push((buffer_id, def.clone()));
-        }
-
-        for (buffer_id, def) in updates {
-            self.services.update_output_sink_config(buffer_id, &def);
+            let Ok(NodeDef::Output(def)) = loaded_registry_def(registry, location) else {
+                continue;
+            };
+            services.update_output_sink_config(buffer_id, def);
         }
     }