Commit 128de62d by PLN (Algolia)

perf(lcxl): the LEDs lagged because we deduped on VALUE, and colour is a step function

PLN, watching the board for the first time on real hardware: "its slow to track if
i move all fders quickly i see the animations lagging a sec or two behind".

Not a slow device. Three compounding faults, each of which alone would have been
survivable:

1. THE BUG. The read loop repainted whenever a CC *value* changed. But colour is a
   STEP function of value -- six steps in value_ramp, seven in filter_colour. Sweeping
   one knob 0 -> 127 emits ~128 events and can change the board at most 6 times. We
   were asking the wire for ~20x the work that could possibly be visible.
2. Every send re-resolved the port from scratch: `aconnect -o` AND `amidi -l` AND then
   `aseqsend`. Three forks per LED, ~1.7 ms each, measured.
3. All of it ran INSIDE `for line in proc.stdout`, so a write in flight stopped us
   reading the next MIDI event. aseqdump's pipe QUEUES rather than drops, so nothing
   was lost -- everything just arrived later, and later, without bound. That
   unboundedness is why it read as "a second or two" rather than a constant delay.

The fix, in payoff order. A new `Painter` thread owns the wire; the reader only touches
the model and hands it colours, never blocking. The painter dedupes on the COLOUR the
board will show, coalesces so only the last colour per index within a frame reaches the
wire, and batches every dirty index into ONE SysEx (the Launchpad dialect takes
(index, colour) pairs, so a whole-surface repaint is a single write). Port resolution
is cached for 2 s and thrown away the instant a write fails -- evidence, not a timer --
which keeps the replug-recovery property that made it uncached in the first place.
Rate limiting is a floor on the GAP between writes, not a fixed tick, so an isolated
button press still goes out immediately.

Measured, not claimed. `--bench` replays PLN's own complaint (8 knobs + 8 faders swept
together, 500 events/s) against a transport modelled at its real cost, and runs the
legacy path beside the new one:

    legacy     744 wire msgs   wall 11.23s   overrun +8.23s
    coalesced   48 wire msgs   wall  3.00s   overrun +0.00s
    15.5x fewer messages; latency p50 7.9 ms, p99 20.2 ms

Overrun IS the visible lag -- it is how far behind his hands the board finishes. On the
real device the per-write cost also fell 7.70 ms -> 2.72 ms with ports cached, so the
total wire work is down roughly 44x.

Twelve regression tests, no hardware needed. Speed regresses silently -- nothing goes
red, it just gets slow again -- so the assertions are numeric: wire rate bounded by the
frame rate and not the input rate, reader never falls behind, p99 under 50 ms, a lone
press not delayed by the frame boundary. The last one is the one that matters: fast and
wrong beats nothing, so we decode every SysEx the coalescer emitted through the mock
surface and assert the board that LANDS is exactly what a full `build_frame`
recomputation would have produced.

Also: verbose logging now fires only when the board actually changes. Printing 400
lines a second of "CC77 = 63" was itself I/O in the hot loop, and told nobody anything.
parent 89d9f127
"""Regression tests for the LED paint pipeline's SPEED — and for its honesty.
The bug (2026-07-28): "its slow to track if i move all fders quickly i see the
animations lagging a sec or two behind". Not a slow device, a structural one — the
reader loop did its own blocking I/O, so it could not read the next MIDI event while a
write was in flight, and aseqdump's pipe QUEUES rather than drops. Every event was
faithfully rendered, just later and later.
Speed is the easiest thing to regress silently, because nothing goes red — it just gets
slow again. So these tests are numeric and they run without hardware: the transport is
modelled at its measured cost (~1.7 ms of fork+exec per subprocess) and the assertions
are on messages, latency and overrun.
The last test is the one that matters most. A coalescer that is fast but paints the
wrong board is worse than a slow correct one, so we assert that whatever the coalescer
folds away, the frame that LANDS is byte-for-byte the frame a full recomputation would
have produced — decoded through the same mock surface PLN's colour convention was
designed against.
"""
from __future__ import annotations
import importlib.util
import sys
import time
from pathlib import Path
import pytest
TOOLS = Path(__file__).resolve().parents[2]
def _load(name: str, path: Path):
spec = importlib.util.spec_from_file_location(name, path)
mod = importlib.util.module_from_spec(spec)
sys.modules[name] = mod
spec.loader.exec_module(mod)
return mod
leds = _load("lcxl_leds", TOOLS / "lcxl-leds.py")
mock = _load("mock_lcxl", TOOLS / "mock-lcxl.py")
class RecordingSender(leds.Sender):
"""Costs what the wire costs, and remembers every byte it was asked to write."""
def __init__(self, send_ms: float = 5.0):
super().__init__(dry_run=False, verbose=False)
self.cost = send_ms / 1000.0
self.messages: list[str] = []
def send(self, hexmsg: str) -> bool:
self.messages.append(hexmsg)
time.sleep(self.cost)
self.sends += 1
return True
def drive(events, rate=500.0, fps=60.0, send_ms=5.0, bindings=None):
"""Feed events through the REAL pipeline at a real arrival rate."""
bindings = bindings if bindings is not None else leds.convention_bindings()
s = RecordingSender(send_ms)
p = leds.Painter(s, fps=fps).start()
period = 1.0 / rate
t0 = time.monotonic()
for k, (cc, val) in enumerate(events):
hit = leds.resolve_paint(cc, val, bindings)
if hit:
p.set(*hit)
slack = t0 + (k + 1) * period - time.monotonic()
if slack > 0:
time.sleep(slack)
read_wall = time.monotonic() - t0
p.flush(timeout=10.0)
p.stop()
return s, p, read_wall
# ------------------------------------------------------------------ dedupe
def test_a_full_fader_sweep_costs_at_most_one_update_per_colour_step():
"""THE bug. The old loop repainted on every VALUE change; colour is a step
function of value, so a 128-event sweep asked for 128 writes and needed six."""
s = RecordingSender(send_ms=0.0)
p = leds.Painter(s, fps=0.0)
idx = leds.cc_to_index(13)
accepted = sum(p.set(idx, leds.control_colour(13, "rhythm", v, True))
for v in range(128))
distinct = len({leds.control_colour(13, "rhythm", v, True) for v in range(128)})
assert accepted <= distinct
assert p.deduped >= 128 - distinct
def test_dedupe_is_against_what_the_wire_last_saw_not_what_is_pending():
"""Setting a colour back to a still-unsent pending value must not queue a write."""
p = leds.Painter(RecordingSender(0.0), fps=0.0)
assert p.set(3, 60) is True
assert p.set(3, 60) is False # already pending
assert p.set(3, 28) is True # genuinely different
assert p.set(3, 60) is True # back again, but 28 is what's pending
# ------------------------------------------------------- coalescing + rate
@pytest.mark.parametrize("rate", [200.0, 500.0])
def test_wire_rate_is_bounded_by_the_frame_rate_not_the_input_rate(rate):
"""The guarantee: however fast his hands are, the wire is bounded."""
fps, secs = 60.0, 1.0
events = list(leds.sweep_events(secs, rate))
s, p, _ = drive(events, rate=rate, fps=fps)
assert len(s.messages) <= fps * secs * 1.5, (
f"{len(s.messages)} messages for {len(events)} events at {rate}/s")
def test_the_reader_never_falls_behind_the_input():
"""Overrun IS the visible lag. The legacy path overran 3 s of input by +8.2 s."""
secs, rate = 1.0, 500.0
_, _, read_wall = drive(list(leds.sweep_events(secs, rate)), rate=rate)
assert read_wall < secs * 1.25, f"reader took {read_wall:.2f}s for {secs}s of input"
def test_p99_latency_stays_perceptually_instant_under_a_full_surface_sweep():
secs, rate = 1.0, 500.0
_, p, _ = drive(list(leds.sweep_events(secs, rate)), rate=rate)
st = p.stats()
assert st["p99_ms"] < 50.0, st
assert st["max_ms"] < 150.0, st
def test_an_isolated_press_is_not_delayed_by_the_frame_rate():
"""Rate limiting is a floor on the GAP between writes, not a fixed tick — a lone
button press must not wait for the next frame boundary."""
s = RecordingSender(send_ms=0.0)
p = leds.Painter(s, fps=30.0).start()
try:
t0 = time.monotonic()
p.set(leds.cc_to_index(41), leds.HUE["green"]["full"])
p.flush(timeout=1.0)
assert time.monotonic() - t0 < 0.020
finally:
p.stop()
def test_a_burst_of_forty_leds_leaves_as_one_sysex_not_forty():
"""Batching: the Launchpad dialect takes (index, colour) pairs, so a whole-surface
repaint is one write."""
s = RecordingSender(send_ms=0.0)
p = leds.Painter(s, fps=0.0)
p.set_frame([leds.HUE["green"]["full"]] * leds.LIT_INDICES)
pairs, _ = p._drain()
msg = leds.sysex_pairs(pairs)
assert len(mock.parse_frames(msg)) == leds.LIT_INDICES
# -------------------------------------------------------- the invariant
def test_the_board_that_lands_equals_a_full_recomputation():
"""Correctness under coalescing, checked through the mock surface.
Fast and wrong is worse than slow and right. Whatever the coalescer drops or folds,
decoding every SysEx it emitted must reproduce exactly `build_frame` over the final
model — the same pure function the colour convention is defined by.
"""
bindings = leds.convention_bindings()
s = RecordingSender(send_ms=1.0)
p = leds.Painter(s, fps=60.0).start()
p.set_frame(leds.build_frame(bindings, {}, set()))
values: dict[int, int] = {}
for cc, val in leds.sweep_events(0.4, 500.0):
values[cc] = val
hit = leds.resolve_paint(cc, val, bindings)
if hit:
p.set(*hit)
p.flush(timeout=5.0)
p.stop()
landed = mock.parse_frames("\n".join(s.messages))
expected = leds.build_frame(bindings, values, set(values))
assert landed == {i: c for i, c in enumerate(expected)}
def test_faders_never_receive_a_write():
"""CC 77-84 are Ardour's track gains and have no LEDs. They generate load and
nothing else — a write there would be both meaningless and dangerous."""
for cc in range(77, 85):
assert leds.cc_to_index(cc) is None
assert leds.resolve_paint(cc, 100, leds.convention_bindings()) is None
# --------------------------------------------------------- port caching
def test_port_lookup_is_cached_then_dropped_on_failure(monkeypatch):
"""The cache buys 3 forks -> 1 per write, but must not survive the one event that
makes it wrong. A binding resolved once and silently invalidated by a device event
is this rig's most expensive recurring bug class."""
calls = {"n": 0}
def fake():
calls["n"] += 1
return "24:0"
leds.invalidate_ports()
monkeypatch.setattr(leds, "_find_hw_port", fake)
assert leds.find_hw_port() == "24:0"
assert leds.find_hw_port() == "24:0"
assert calls["n"] == 1, "second lookup should have been served from cache"
leds.Sender(verbose=False)._fail("simulated write failure")
assert leds.find_hw_port() == "24:0"
assert calls["n"] == 2, "a failed write must force re-resolution"
leds.invalidate_ports()
def test_port_cache_ttl_is_short_enough_to_be_invisible():
assert leds.PORT_TTL <= 5.0
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment