perf(display): throttle the per-frame update tick; check strips without building them

CHANGELOG.md

@@ -19,6 +19,39 @@ accepts both, but the store flags the old spelling as deprecated
 
 ## Unreleased
 
+### Plugin update tick: a few times a second, not every frame
+
+- The frame loops and the dwell sleep ran
+  `PluginManager.run_scheduled_updates()` after every frame, about 125 times
+  a second on a scroller. Each pass copies the plugin dict and takes several
+  locks per plugin, almost always to find nothing due: about 100 us with 20
+  plugins on a Pi 4, 1.2% of the render thread. They now call
+  `DisplayController._tick_plugin_updates_if_due()`, which runs the pass at
+  most every `PLUGIN_UPDATE_TICK_INTERVAL` (0.25 s), so a 4 s scroll runs 16
+  passes instead of 500. No update interval is shorter than 5 s
+  (`MIN_DYNAMIC_UPDATE_INTERVAL`), and the 1 Hz frame loop already ticked
+  once a second, so an update starts at most a quarter second later.
+- The top of each loop pass still runs it unthrottled, so a plugin just
+  loaded, reloaded or enabled for on-demand is updated at once. Vegas's own
+  update thread (`_tick_plugin_updates_for_vegas`) is unchanged.
+  `test/test_plugin_update_tick_throttle.py` covers both, on the real
+  `run()` through the golden-trace harness.
+
+### Strip checks no longer build the PIL image
+
+- `SportsScrollDisplay.display_scroll_frame` (every frame) and
+  `has_cached_content`, and the sync follower's per-frame check of the Vegas
+  strip, asked whether there was a strip by reading
+  `ScrollHelper.cached_image`. After the helper deferred the image (an
+  append, trim or patch), that read built it from `cached_array` and kept
+  it: 3.5 ms and about 1 MiB more held for a 4288x64 strip, on top of the
+  array's 0.8 MiB. They now ask `has_strip()`, which gives the same answer
+  from the helper's bookkeeping. A scoreboard whose `scroll_helper` has no
+  `has_strip` (its own helper, a test double) is still asked
+  `cached_image`.
+- A strip built with `create_scrolling_image` or `set_scrolling_image`
+  still keeps both the image and the array, as before.
+
 ### Faster frame copy into the panel (library patch, applied at build time)
 
 - Copying each frame into the panel buffer (`SetImage`) was the biggest CPU

src/common/sports_scroll.py

@@ -323,7 +323,7 @@ def display_scroll_frame(self) -> bool:
         :returns: True if a frame was drawn; False when there is no content or
             the frame could not be rendered.
         """
-        if not self.scroll_helper.cached_image:
+        if not self._has_strip():
             return False
 
         try:
@@ -416,7 +416,21 @@ def get_dynamic_duration(self) -> int:
 
     def has_cached_content(self) -> bool:
         """Whether content is prepared and ready to scroll."""
-        return bool(self.scroll_helper.cached_image)
+        return self._has_strip()
+
+    def _has_strip(self) -> bool:
+        """Whether the helper holds a strip, without building its PIL image.
+
+        Reading ``cached_image`` after the strip was extended or trimmed builds
+        the image from the array and keeps it, so the strip is held twice;
+        display_scroll_frame asks this every frame. ``has_strip()`` answers
+        from the helper's bookkeeping. A helper without it (a plugin's own, a
+        test double) is asked the old way.
+        """
+        helper = self.scroll_helper
+        if callable(getattr(type(helper), "has_strip", None)):
+            return bool(helper.has_strip())
+        return bool(helper.cached_image)
 
     # ------------------------------------------------------------------
     # Live Vegas cards

src/display_controller.py

@@ -330,6 +330,9 @@ def _follower_gated_update():
         # Monotonic stamp of the last _service_pending_changes pass; same
         # "None means never" convention as _last_on_demand_poll.
         self._last_pending_service: Optional[float] = None
+        # Monotonic stamp of the last scheduled-update pass; see
+        # _tick_plugin_updates_if_due. Same "None means never" convention.
+        self._last_plugin_update_tick: Optional[float] = None
         # The control socket (src/ipc), started by run(). None when it is not
         # served (Windows, LEDMATRIX_CONTROL_SOCKET=off, a bind failure);
         # the file mailbox works either way.
@@ -1096,10 +1099,37 @@ def _tick_plugin_updates_for_vegas(self) -> None:
                 except Exception:  # pylint: disable=broad-except
                     logger.exception("Error marking plugin %s updated for Vegas", plugin_id)
 
+    #: Shortest gap between scheduled-update passes from the frame loops and
+    #: the dwell sleep. The pass (PluginManager.run_scheduled_updates) copies
+    #: the plugin dict and takes several locks per plugin to find, almost
+    #: always, that nothing is due: about 95 us with 20 plugins on a Pi, or
+    #: 1.2% of the render thread at 125 frames a second. No interval is
+    #: shorter than PluginManager.MIN_DYNAMIC_UPDATE_INTERVAL (5 s), and the
+    #: 1 Hz frame loop already ticks once a second, so a quarter second late
+    #: is not noticed.
+    PLUGIN_UPDATE_TICK_INTERVAL = 0.25
+
+    #: Class-level default for controllers built without __init__ (tests).
+    _last_plugin_update_tick: Optional[float] = None
+
+    def _tick_plugin_updates_if_due(self) -> None:
+        """_tick_plugin_updates, at most once per PLUGIN_UPDATE_TICK_INTERVAL.
+
+        For the per-frame callers. The top of each loop pass calls
+        _tick_plugin_updates itself, unthrottled, because that is where a
+        plugin just loaded, reloaded or enabled for on-demand gets its first
+        update, and it must not wait out the floor.
+        """
+        last = self._last_plugin_update_tick
+        if last is not None and time.monotonic() - last < self.PLUGIN_UPDATE_TICK_INTERVAL:
+            return
+        self._tick_plugin_updates()
+
     def _tick_plugin_updates(self):
         """Run any plugin updates that are due."""
         if not self.plugin_manager:
             return
+        self._last_plugin_update_tick = time.monotonic()
         try:
             self.plugin_manager.run_scheduled_updates()
         except Exception:  # pylint: disable=broad-except
@@ -1267,7 +1297,7 @@ def _sleep_with_plugin_updates(self, duration: float, tick_interval: float = 1.0
             # A dwell can be a minute long (sixty seconds while scheduled
             # off); the watchdog must hear from this thread throughout.
             display_watchdog.watchdog.beat()
-            self._tick_plugin_updates()
+            self._tick_plugin_updates_if_due()
             self._service_pending_changes()
             self._check_live_takeover()
             if (self.current_display_mode != mode
@@ -2905,7 +2935,10 @@ def _run_follower_frame(self) -> None:
 
         self._follower_local_x = local_x
 
-        if rp and rp.scroll_helper.cached_image is not None:
+        # has_strip(), not cached_image: every follower frame asks, and
+        # reading a strip the follower's own rebuild deferred would build
+        # and keep a second copy of it as a PIL image.
+        if rp and rp.scroll_helper.has_strip():
             # Hold last frame until TCP image arrives after cycle reset
             if not self._follower_pending_new_image and local_x >= width:
                 rp.scroll_helper.scroll_position = (
@@ -3595,6 +3628,8 @@ def run(self):
                     self._release_on_demand_plugins()
                     if not self.available_modes:
                         continue  # it was all there was; idle as above
+                # Unthrottled, unlike the frame loops: a plugin loaded,
+                # reloaded or enabled for on-demand above is due at once.
                 self._tick_plugin_updates()
 
                 # Clean up expired WiFi status messages
@@ -3857,7 +3892,7 @@ def _should_exit_dynamic(elapsed_time: float) -> bool:
                             # Multi-display sync: send follower frame after each render
                             self._send_follower_frame(manager_to_display)
 
-                            self._tick_plugin_updates()
+                            self._tick_plugin_updates_if_due()
                             # Throttled: one clock compare between passes.
                             self._service_pending_changes()
                             self._check_live_takeover()
@@ -3916,7 +3951,7 @@ def _should_exit_dynamic(elapsed_time: float) -> bool:
                                             "breaking early", active_mode,
                                             self.current_display_mode)
                                 break
-                            self._tick_plugin_updates()
+                            self._tick_plugin_updates_if_due()
 
                             elapsed = time.time() - start_time
                             if elapsed >= target_duration:

test/test_follower_scroll_image_handoff.py

@@ -76,3 +76,44 @@ def test_only_the_latest_image_is_adopted():
     dc._adopt_follower_scroll_image(rp)
     assert helper.cached_image is latest
     assert helper.total_scroll_width == 400
+
+
+def test_a_follower_frame_does_not_build_a_deferred_strip_image():
+    """Every follower frame asks whether the strip is there. A strip the
+    follower's own rebuild deferred (append_content) must be answered from
+    the helper's bookkeeping, not by building and keeping its PIL image."""
+    import numpy as np
+
+    from src.common.scroll_helper import ScrollHelper
+
+    width, height = 64, 16
+    helper = ScrollHelper(width, height)
+    helper.append_content([Image.new("RGB", (300, height), (0, 200, 0))])
+    helper.append_content([Image.new("RGB", (300, height), (0, 0, 200))])
+    assert helper.__dict__.get("_cached_image") is None
+    assert helper.has_strip()
+
+    dc = object.__new__(DisplayController)
+    dc.config = {"sync": {}}
+    dc.vegas_coordinator = SimpleNamespace(
+        render_pipeline=SimpleNamespace(scroll_helper=helper))
+    dc.display_manager = MagicMock(width=width)
+    dc.sync_manager = MagicMock()
+    dc.sync_manager.get_latest_scroll_x.return_value = 200
+    dc._follower_incoming_image = deque(maxlen=1)
+    dc._follower_dr_last_t = None
+    dc._follower_local_x = 200.0
+    dc._follower_pending_new_image = False
+    dc._follower_last_frame = None
+    dc._follower_deadline = None
+    dc._scroll_speed = 0
+
+    for _ in range(3):
+        dc._run_follower_frame()
+
+    # The frame was cut from the array...
+    frame = np.asarray(dc._follower_last_frame)
+    assert frame.shape == (height, width, 3)
+    assert frame.any()
+    # ...and the strip is still held once.
+    assert helper.__dict__.get("_cached_image") is None

test/test_plugin_update_tick_throttle.py

@@ -0,0 +1,134 @@
+"""The render loop's scheduled-update pass, throttled per frame.
+
+The frame loops called _tick_plugin_updates() after every frame, about 125
+times a second on a scroller, and each call ran
+PluginManager.run_scheduled_updates(): a copy of the plugin dict and several
+locks per plugin, to find that nothing was due (no interval is shorter than
+5 s). The frame loops and the dwell sleep now call
+_tick_plugin_updates_if_due(), which runs it at most once per
+PLUGIN_UPDATE_TICK_INTERVAL. The top of each loop pass still calls
+_tick_plugin_updates() unthrottled, because a plugin loaded, reloaded or
+enabled for on-demand there is due at once.
+"""
+
+import os
+from types import SimpleNamespace
+from unittest.mock import MagicMock
+
+import pytest
+
+os.environ.setdefault("EMULATOR", "true")
+
+from src import display_controller as dc_mod  # noqa: E402
+from src.display_controller import DisplayController  # noqa: E402
+from test._run_loop_harness import FakePlugin, RunLoopHarness  # noqa: E402
+
+
+@pytest.fixture
+def clock(monkeypatch):
+    now = {"t": 1000.0}
+    fake = SimpleNamespace(monotonic=lambda: now["t"], time=lambda: now["t"])
+    monkeypatch.setattr(dc_mod, "time", fake)
+    return now
+
+
+@pytest.fixture
+def dc():
+    controller = object.__new__(DisplayController)
+    controller.plugin_manager = MagicMock()
+    return controller
+
+
+def _calls(dc):
+    return dc.plugin_manager.run_scheduled_updates.call_count
+
+
+class TestThrottle:
+    def test_the_first_call_runs(self, dc, clock):
+        dc._tick_plugin_updates_if_due()
+        assert _calls(dc) == 1
+
+    def test_calls_inside_the_window_are_skipped(self, dc, clock):
+        dc._tick_plugin_updates_if_due()
+        for _ in range(30):  # a scroller's frames, 8 ms apart
+            clock["t"] += 0.008
+            dc._tick_plugin_updates_if_due()
+        assert clock["t"] - 1000.0 < dc.PLUGIN_UPDATE_TICK_INTERVAL
+        assert _calls(dc) == 1
+
+    def test_it_runs_again_once_the_window_has_passed(self, dc, clock):
+        dc._tick_plugin_updates_if_due()
+        clock["t"] += dc.PLUGIN_UPDATE_TICK_INTERVAL
+        dc._tick_plugin_updates_if_due()
+        assert _calls(dc) == 2
+
+    def test_the_unthrottled_tick_always_runs_and_restarts_the_window(self, dc, clock):
+        # The top of the loop pass: a plugin just (re)loaded is due now.
+        dc._tick_plugin_updates_if_due()
+        clock["t"] += 0.01
+        dc._tick_plugin_updates()
+        assert _calls(dc) == 2
+        # ...and the frame right after it does not repeat the pass.
+        clock["t"] += 0.01
+        dc._tick_plugin_updates_if_due()
+        assert _calls(dc) == 2
+
+    def test_no_plugin_manager(self, clock):
+        controller = object.__new__(DisplayController)
+        controller.plugin_manager = None
+        controller._tick_plugin_updates_if_due()  # must not raise
+        controller._tick_plugin_updates()
+
+    def test_a_failing_pass_is_contained_and_still_throttled(self, dc, clock):
+        dc.plugin_manager.run_scheduled_updates.side_effect = RuntimeError("boom")
+        dc._tick_plugin_updates_if_due()
+        dc._tick_plugin_updates_if_due()
+        assert _calls(dc) == 1
+
+    def test_the_vegas_tick_is_not_throttled(self, dc, clock):
+        # Vegas's own update thread calls the plugin manager directly.
+        dc.vegas_coordinator = None
+        dc.plugin_manager.run_scheduled_updates_with_changes.return_value = []
+        dc._tick_plugin_updates()
+        dc._tick_plugin_updates_for_vegas()
+        dc._tick_plugin_updates_for_vegas()
+        assert dc.plugin_manager.run_scheduled_updates_with_changes.call_count == 2
+
+
+class TestInTheRunLoop:
+    """The real run() on the golden-trace harness's fake clock."""
+
+    @pytest.fixture
+    def run(self, tmp_path):
+        h = RunLoopHarness(tmp_path, horizon=40)
+        h.add_plugin(FakePlugin("ticker", ["ticker"], duration=10, enable_scrolling=True))
+        h.add_plugin(FakePlugin("clock", ["clock"], duration=10))
+        ticks = []
+        h.pm.run_scheduled_updates = lambda: ticks.append(round(h.clock.rel(), 3))
+        h.run()
+        passes = sorted({t for t, kind, _s, _d in h.events if kind == "pass"})
+        frames = [t for t, kind, s, _d in h.events if kind in ("first", "frame") and s == "ticker"]
+        return SimpleNamespace(ticks=ticks, passes=passes, frames=frames,
+                               interval=h.controller.PLUGIN_UPDATE_TICK_INTERVAL)
+
+    def test_a_scroller_ticks_a_few_times_a_second_not_every_frame(self, run):
+        ticker_frames = len(run.frames)
+        assert ticker_frames > 500  # 10 s screens at 125 Hz, twice
+        # 40 s at four a second, plus one per pass.
+        assert len(run.ticks) <= 40 / run.interval + len(run.passes) + 1
+
+    def test_ticks_are_never_closer_than_the_floor_except_at_a_pass(self, run):
+        passes = set(run.passes)
+        for prev, cur in zip(run.ticks, run.ticks[1:]):
+            if cur - prev < run.interval - 1e-6:
+                assert cur in passes, (prev, cur)
+
+    def test_every_pass_ticks_at_once(self, run):
+        # Unthrottled: the frame loop ticked just before the screen ended, and
+        # the pass ticks again anyway, so a plugin it just loaded is not kept
+        # waiting.
+        ticks = set(run.ticks)
+        for t in run.passes:
+            assert t in ticks, t
+        assert any(cur - prev < run.interval
+                   for prev, cur in zip(run.ticks, run.ticks[1:]))