From 643cf1957757378d6969839f9cb6b6a419e8358d Mon Sep 17 00:00:00 2001 From: ChuckBuilds Date: Thu, 1 Oct 2026 22:43:07 -0400 Subject: [PATCH 1/2] perf(display): throttle the per-frame plugin update tick to 4 Hz The frame loops and the dwell sleep called _tick_plugin_updates() after every frame, about 125 times a second on a scroller. Each call ran PluginManager.run_scheduled_updates(), which copies the plugin dict and takes several locks per plugin to find, almost always, that nothing is due: about 100 us with 20 plugins on a Pi 4, 1.2% of the render thread. No update interval is shorter than 5 s. They now call _tick_plugin_updates_if_due(), which runs the pass at most every PLUGIN_UPDATE_TICK_INTERVAL (0.25 s, monotonic clock, like _service_pending_changes). The top of each loop pass keeps calling _tick_plugin_updates() unthrottled, because that is where a plugin just loaded, reloaded or enabled for on-demand gets its first update. The Vegas update thread calls the plugin manager directly and is unchanged. Golden traces unchanged. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Uc5DAbSwGGUTm3MrCtpC2m --- CHANGELOG.md | 18 +++ src/display_controller.py | 38 ++++++- test/test_plugin_update_tick_throttle.py | 134 +++++++++++++++++++++++ 3 files changed, 187 insertions(+), 3 deletions(-) create mode 100644 test/test_plugin_update_tick_throttle.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 5913a565d..6a4de7e6c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,24 @@ 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. + ### Garbage-collection pauses in the frame stats - The display now times every Python garbage collection diff --git a/src/display_controller.py b/src/display_controller.py index d160398f0..ba384128f 100644 --- a/src/display_controller.py +++ b/src/display_controller.py @@ -320,6 +320,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. @@ -1083,10 +1086,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 @@ -1254,7 +1284,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 @@ -3469,6 +3499,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 @@ -3724,7 +3756,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() @@ -3783,7 +3815,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: diff --git a/test/test_plugin_update_tick_throttle.py b/test/test_plugin_update_tick_throttle.py new file mode 100644 index 000000000..5bb246101 --- /dev/null +++ b/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:])) From 6749afdca6d503497b968b76a25242b869ae0f6b Mon Sep 17 00:00:00 2001 From: ChuckBuilds Date: Thu, 1 Oct 2026 22:46:25 -0400 Subject: [PATCH 2/2] perf(scroll): ask has_strip() instead of building cached_image SportsScrollDisplay.display_scroll_frame (every frame) and has_cached_content, and the sync follower's per-frame check of the Vegas strip, tested ScrollHelper.cached_image for truth. When the helper had deferred the image (append_content, drop_scrolled_prefix, patch_columns), that read built it from cached_array and kept it, so the strip was held twice: 3.5 ms and about 1 MiB more for a 4288x64 strip on a Pi. has_strip() is true exactly when cached_image would be truthy (a PIL image is always truthy, the empty-content placeholder included), so the answers are unchanged. A scroll_helper without has_strip (a plugin's own helper, a test double) is still asked cached_image. Reads that need the image itself (the leader's sync push, Vegas capture in plugin_adapter, render_pipeline's image push, vegas_audit) are left alone. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Uc5DAbSwGGUTm3MrCtpC2m --- CHANGELOG.md | 15 +++ src/common/sports_scroll.py | 18 ++- src/display_controller.py | 5 +- test/test_follower_scroll_image_handoff.py | 41 +++++++ test/test_sports_scroll_strip_check.py | 126 +++++++++++++++++++++ 5 files changed, 202 insertions(+), 3 deletions(-) create mode 100644 test/test_sports_scroll_strip_check.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 6a4de7e6c..a57da881f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,6 +37,21 @@ accepts both, but the store flags the old spelling as deprecated `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. + ### Garbage-collection pauses in the frame stats - The display now times every Python garbage collection diff --git a/src/common/sports_scroll.py b/src/common/sports_scroll.py index eb99f83dc..4774bd0c5 100644 --- a/src/common/sports_scroll.py +++ b/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 diff --git a/src/display_controller.py b/src/display_controller.py index ba384128f..459cb77ac 100644 --- a/src/display_controller.py +++ b/src/display_controller.py @@ -2832,7 +2832,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 = ( diff --git a/test/test_follower_scroll_image_handoff.py b/test/test_follower_scroll_image_handoff.py index 59e7bd0b6..0bb4b597d 100644 --- a/test/test_follower_scroll_image_handoff.py +++ b/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 diff --git a/test/test_sports_scroll_strip_check.py b/test/test_sports_scroll_strip_check.py new file mode 100644 index 000000000..fdfad09e6 --- /dev/null +++ b/test/test_sports_scroll_strip_check.py @@ -0,0 +1,126 @@ +"""Asking a scoreboard whether it has a strip must not build its PIL image. + +SportsScrollDisplay.display_scroll_frame (every frame) and has_cached_content +asked ``scroll_helper.cached_image``, a property that builds the strip's PIL +image from ``cached_array`` when the helper deferred it (after a patch, +append or trim) and keeps it, so the strip ends up held twice. They now ask +``has_strip()``, which answers from the helper's bookkeeping. These pin down +that the answer is the same as ``bool(cached_image)`` in every state, and +that the image is not built. +""" + +import sys +from pathlib import Path +from unittest.mock import MagicMock + +import numpy as np +import pytest +from PIL import Image + +sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) +sys.modules.setdefault("rgbmatrix", MagicMock()) + +from src.common.sports_scroll import SportsScrollDisplay # noqa: E402 + +W, H = 128, 32 + + +class _Panel: + matrix = None + refresh_hz = 100.0 + + def __init__(self): + self.width, self.height = W, H + self.image = None + self.frames = 0 + + def set_scrolling_state(self, scrolling, frame_hold=1): + pass + + def update_display(self): + self.frames += 1 + + +class _Display(SportsScrollDisplay): + SCROLL_LEAGUE_KEYS = ("nfl",) + + def prepare_scroll_content(self, games, game_type, leagues, rankings_cache=None): + cards = [Image.new("RGB", (40, H), (200, 0, 0)) for _ in games] + self.scroll_helper.create_scrolling_image(cards, item_gap=8, element_gap=0) + return bool(cards) + + +@pytest.fixture +def display(): + return _Display(_Panel(), {}) + + +def _deferred(display): + """A strip whose PIL image the helper has dropped, to build on read.""" + display.prepare_scroll_content([1, 2, 3], "recent", ["nfl"]) + helper = display.scroll_helper + helper.patch_columns(W, np.zeros((H, 4, 3), dtype=np.uint8)) + assert helper.__dict__.get("_cached_image") is None + assert helper.__dict__.get("_image_source") is not None + return helper + + +class TestTheImageIsNotBuilt: + def test_has_cached_content(self, display): + helper = _deferred(display) + assert display.has_cached_content() is True + assert helper.__dict__.get("_cached_image") is None + + def test_display_scroll_frame(self, display): + helper = _deferred(display) + for _ in range(5): + assert display.display_scroll_frame() is True + assert display.display_manager.frames == 5 + assert helper.__dict__.get("_cached_image") is None + + +def _states(): + """(name, set-up) for every state a helper's strip can be in.""" + def none(d): + pass + + def placeholder(d): # create_scrolling_image([]): one blank panel + d.scroll_helper.create_scrolling_image([]) + + def built(d): + d.prepare_scroll_content([1, 2], "recent", ["nfl"]) + + def set_image(d): + d.scroll_helper.set_scrolling_image(Image.new("RGB", (300, H))) + + def deferred(d): + _deferred(d) + + def cleared(d): + built(d) + d.scroll_helper.clear_cache() + + def assigned_none(d): + built(d) + d.scroll_helper.cached_image = None + + return [none, placeholder, built, set_image, deferred, cleared, assigned_none] + + +@pytest.mark.parametrize("setup", _states(), ids=lambda f: f.__name__) +def test_same_answer_as_reading_the_image(display, setup): + setup(display) + answer = display.has_cached_content() + assert answer is bool(display.scroll_helper.cached_image) + + +def test_a_helper_without_has_strip_is_asked_the_old_way(display): + """A plugin's own helper or a test double: cached_image decides.""" + class _OldHelper: + cached_image = None + + display.scroll_helper = _OldHelper() + assert display.has_cached_content() is False + assert display.display_scroll_frame() is False + display.scroll_helper.cached_image = Image.new("RGB", (10, H)) + assert display.has_cached_content() is True