perf: stop per-frame hardware endpoint re-enumeration by parking failed output sinks - #244
Merged
Merged
Conversation
Endpoint availability changes on exactly two events — a claim succeeds, or a lease is released — but the registry had no way to say so. Consumers that wanted to know were left to ask again and again, which is how the output flush seam ended up re-enumerating every endpoint on the board every frame for a sink that could never open. The counter starts nonzero so a consumer defaulting its "last seen" to zero sees a change instead of mistaking a fresh registry for a familiar one, and only successful mutations bump it: a failed claim changes nothing anyone can observe, and signalling it would let one hopeless endpoint wake every parked consumer on every attempt. Plan: 2026-07-31-2224-hw-endpoint-status-cache Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ry frame A sink that cannot open — a project authored for four strips loaded onto a one-strip board, a pin another node holds — re-attempted the open on every flush, and every attempt enumerates every endpoint every driver offers, each one built with a formatted spec, parsed, validated, and given a freshly computed status. On quad-strips under the emulator that was 45.8% of all cycles spent relearning the same "no" sixty times a second, with a warning logged each time. Sinks now park on the registry's generation and are skipped until hardware ownership actually moves. Recovery is no slower for it: releasing a pin bumps the generation and the very next flush picks the waiting sink up. The generation is sampled before any open in the flush, so a claim this frame's own opens made cannot park a sink against a number that already counts it. Nothing caches endpoint status. The tempting version of this change — memoize what the registry answered — optimizes a query the success path never makes and would hand two owners the same pin the first time a cached "available" went stale. ADR: docs/adr/2026-07-31-output-sink-retry-policy.md Plan: 2026-07-31-2224-hw-endpoint-status-cache Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Opening an endpoint by spec enumerated the drivers three times over: once to choose the endpoint, once to work out which driver owned the id it returned, and once more inside the driver to recover the GPIO the id already named. Each pass builds every endpoint the board declares, and each endpoint costs a formatted spec, a parse, and a live status made of several registry lookups — on a board declaring every GPIO, hundreds of them per pass. The by-spec and by-address opens now walk the drivers once, stopping at the first available match, and open on the driver they found it in. The virtual driver answers "which GPIO is this?" from the manifest instead of from a list of statuses it does not use. Preference and error selection are unchanged: an available endpoint still wins over a claimed one, a claimed one still reaches its driver so the failure comes with that driver's account of why, and a missing one is still UnknownEndpoint. A new test holds the survey to a single pass. Plan: 2026-07-31-2224-hw-endpoint-status-cache Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Re-reading the authored output configs each tick cloned every output's definition — endpoint spec and all — purely to escape a borrow, then almost always discovered nothing had changed and threw the copies away. On silicon that is a heap allocation per output per frame for no result. The tree and the services are now borrowed as the separate fields they are, so the defs can be read in place while the sinks are updated, and the comparison borrows the authored endpoint rather than copying it first. Only a genuine change pays for a copy. The dead `loaded_node_def_for_entry` goes with it: the loop calls the same lookup it wrapped, and leaving an unused method behind to be greeted by a warning helps nobody. Plan: 2026-07-31-2224-hw-endpoint-status-cache Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The emulator ran on a board declaring a single timing resource, so a four-strip project lit one strip there and left three endpoints that could never open — while the same project on the desk S3 lights all four. Every emulator profile of a multi-output project was therefore measuring a failure path that silicon does not take. The new board is the old one plus exactly what it lacked: three more timing resources and the D9/D8/D7 labels four-strip projects address, using the S3's own GPIO numbers for them. D10 keeps GPIO 18, where the virtual board has always had it. The single-RMT board stays as it is — several tests want a board where outputs contend, and that is a fair thing to test. Deliberately sequenced after the measurement: making the endpoints exist first would have made the retry cost vanish without anything being fixed. Profiled after the swap, the steady frame is 1.554M cycles against 1.534M before it — the difference is three more strips actually being written. Plan: 2026-07-31-2224-hw-endpoint-status-cache Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…osed The debt entry predicted the profile would end up dominated by the resolver once endpoint status stopped being recomputed per frame, and it is: memcpy, the allocator, QueryKey and SlotPath::parse are what remains. Says so, names the sibling plan that owns them, and points at the profile that is now their baseline. Also records what the fix actually was, since the entry's framing — "make endpoint status persist across frames" — turned out to describe the wrong mechanism. Nothing persists; the sinks simply stopped asking. The desk-S3 re-measurement the exit criteria call for is recorded as not done: the board was not attached. Left explicit rather than quietly dropped. Plan: 2026-07-31-2224-hw-endpoint-status-cache Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Yona-Appletree
marked this pull request as ready for review
August 1, 2026 07:12
The desk S3 was attached the day after the fix landed, so the measurement the exit criteria asked for is no longer an IOU. quad-strips holds 20 fps at a 48 ms tick — flat, as predicted: the S3 opens all four channels on frame one and so never paid the retry cost that dominated the emulator. A flat number cannot prove which image produced it, so the parked-sink path was exercised on the board instead: one output aimed at a pin that does not exist gives two warnings and then silence across ~1,250 frames, where the old code would have logged one per frame. That identifies the build and demonstrates the fix on silicon at the same time. Plan: 2026-07-31-2224-hw-endpoint-status-cache Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Yona-Appletree
added a commit
that referenced
this pull request
Aug 1, 2026
With #243 and #244 on one image, measured together on d8:3b:da:47:29:70: 4 fixtures 25 fps / 37 ms, 1 fixture 68 fps / 13 ms. The emulator says 21.9×; silicon says +25%, because the endpoint-status half was a virtual-board retry storm the S3 never entered — it opens all four channels on frame one. Decomposed, that is the uncomfortable part: per fixture+output chain 9.7 ms -> 8.0 ms (-17%) fixed per-frame overhead 9.3 ms -> 5.0 ms (-46%) This entry is named for the per-chain term, and the per-chain term barely moved. Ten fixtures goes from 9 fps to 12 — still linear, still unusable, still not something an author can buy down with resolution or LED count. So it stays open, now on the number rather than on either mechanism, and without a fresh guess at the cause: the honest state is that ~8 ms per chain is unattributed, with quad-strips-1fix vs quad-strips as the differential to attribute it. Plan: 2026-07-31-2225-persist-dataflow-resolution
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Plan: lp2025/2026-07-31-2224-hw-endpoint-status-cache
Path: ~/.photomancer/planning/lp2025/2026-07-31-2224-hw-endpoint-status-cache/plan.md
What this fixes
On
projects/test/quad-stripsthe emulator profile attributed 45.8% of allself cycles to
HwRegistry::endpoint_status_forand 90.2% inclusive toOutputProvider::open.It turned out not to be a status-lookup problem at all. It was a failed-open
retry storm:
ensure_channel_openre-attempted the open of any handle-lesssink every frame, forever, and each attempt enumerated the whole board — 256
endpoints, each with a formatted spec, a parse, a validate, and a live status
made of several registry lookups. fw-emu's board declared one WS281x channel
and no
D9/D8/D7, so three of quad-strips' four sinks could never open andpaid that price sixty times a second.
So nothing is cached. A sink that opens successfully never asks again, so
there was no repeated query to memoize — and a cached
Availableon a claimedpin would hand two owners the same hardware. Instead the registry gained the
invalidation primitive it lacked, and the flush seam stopped asking:
HwRegistry::generation()changes when a claim succeeds or a lease isreleased — the only two events that can turn a refusal into an acceptance.
Failed claims deliberately do not bump.
OutputProvider::hardware_generation()forwards it (default0= statichardware).
EngineServicesparks a failed sink on that value and skips it until thevalue changes. The generation 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 counts it.
Recovery is not slower for it: a released pin bumps the generation and the very
next flush picks the waiting sink up.
Measured
Frame-for-frame, 8 frames in both runs (
events.jsonlB→E pairs):10.7× cheaper per steady frame.
endpoint_status_for,VirtualWs281xDriver::endpoints,endpoint_for_spec,validate_specand thecore::fmtmachinery are all absent from the top-20 self cycles. Per-framewarn spam became 7 lines for the whole run.
What remains at the top is the sibling plan's target — memcpy, the allocator,
QueryKey::eq,SlotPath::parse: the dataflow resolver re-resolving from coldeach tick, owned by
2026-07-31-2225-persist-dataflow-resolution.Also here
enumerated to pick an endpoint, again to find which driver owned the id, and
the virtual driver again inside
opento recover a GPIO the id alreadynamed. Error selection and driver ordering are unchanged, and a new test
holds it to one pass.
OutputDefclones —refresh_output_sink_configsclonedevery output's definition each tick to dodge a borrow, then almost always
found nothing had changed. Now the tree and services are borrowed as the
separate fields they are.
after the measurement, since making the endpoints exist first would have
made the cost vanish without anything being fixed.
Honest scope note
On the desk S3 all four channels open on frame one, so silicon never paid this
in steady state and should be ~flat at its 20 fps. What silicon gains is that a
misconfigured output no longer costs a board enumeration and a serial log
line every frame it stays wrong.
The S3 fps was not re-measured: the board was not attached (the two ports
present were an esp32c6 and one that would not connect; identified via
espflash board-info, never auto-picked). That acceptance criterion is leftopen rather than satisfied with a substitute board — see
_DONE.md.Phases
OutputProvider::hardware_generation()refresh_output_sink_configswithout per-frame clonesADR:
docs/adr/2026-07-31-output-sink-retry-policy.mdDebt:
docs/debt/s3-frame-cost-scales-per-fixture.md— endpoint-status halfclosed with numbers; resolver half explicitly still open, so the entry is not
marked resolved.
🤖 Generated with Claude Code