Repository navigation
perf(sports): reuse an unchanged scroll strip at the start of a turn - #719
Merged
Merged
Conversation
SportsScrollDisplayManager.prepare_and_display() drew every card of a scoreboard's strip at the start of every scroll-mode turn, and again after each completed cycle, while the render thread waited in execute_display: ~1.4s for seven football cards at 192x48 on ledpi (Pi 4), logged as 1.1-1.6s "Render stall" freezes. Recent and upcoming slates rarely change between turns. Each build is now keyed on everything the strip is drawn from: every game dict (sports_vegas.game_fingerprint), the rankings, the plugin config by value (not id(), so an in-place edit counts), the panel size and the local date. When the key matches, the helper still holds the very array that build made, and the strip is under 10 minutes old, the strip is rewound (reset_scroll) instead of calling prepare_scroll_content(). Live, 'mixed' and any other game type are drawn every time as before, and so is anything that cannot be keyed (no games, a one-shot iterable, unhashable leagues). One display per game type could never hit: nfl_recent and ncaa_fb_recent take turns on the shared 'recent' display. So recent and upcoming keep one display per slate (game type + leagues), and _scroll_displays[game_type] always points at the slate on screen: display_frame, is_complete, get_scroll_display and the plugins' own readers of _scroll_displays (get_dynamic_duration, has_cached_content) see what they did before. Separate displays also keep plugin-only per-league state (football's _settings_league and helper speed) with their strip, so core never has to restore state it cannot see. Bounded: 4 displays per game type (the least recently shown one draws a new slate, as the shared display did) and 6MB per plugin of strips parked off screen, released without clear() so the display manager's scrolling state is left alone. A build overtaken by another (a display() call that outlived its timeout) records nothing (slot epoch, active-display check), and a failed build puts the previous strip back on screen, as the shared display left it. Checked against ledmatrix-plugins origin/main (20ba7262), real plugin code on this host (x86, Python 3.14, Pillow 12.3): football, basketball, soccer and baseball reuse from the second turn of each slate, hockey from the third (its renderer fills in status fields in place on the first draw, so the first repeat misses). The reused strip, Vegas items, dynamic duration, helper state and frame 200 equal a fresh build's. Here a 7-card football build is 50-100ms and a reuse 0.1-0.3ms; the key costs ~0.1ms. No plugin change is needed. Scroll mode is opt-in per league and mode, so only rigs that use it gain. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…oreboards Review of the strip memo found behaviour the docs promise that no test held: deleting any of these left all 42 tests passing. - Reuse counts as being shown. The slate let go for a new one is the least recently shown, not the least recently drawn, and the byte trim releases by the same order. - A reuse trims too. One that parks a bigger strip than it brings back would otherwise leave the parked strips over the 6MB budget until they aged out, because only a build trimmed. - The has_strip guard. Without it, an array kept with its image dropped would be reused and drawn blank every turn. - An empty slate is never keyed. New tests also cover game dicts changed in place or while being drawn, and two plugin habits the plain fake lacked: per-league scroll settings (each slate keeps its own pacing) and a renderer that fills in its game dicts (hockey). With LEDMATRIX_PLUGINS set, the real football and hockey scoreboards run two leagues through the memo in a child process. Builds stay bounded, and each reused strip, its Vegas items, dynamic duration, pacing and frame 150 all equal a fresh build. Before, that evidence existed only as a scratch script. The two overtaken-build tests asserted inside the build, where _prepare_on's except swallowed a failure. They now assert afterwards. Comment and docstring corrections, with no code change: - STRIP_MEMO_MAX_PARKED_BYTES bounds strip pixels only, not each extra display's logo/icon caches or the frozen config in each key. - The strip lock also covers creating a slate's display the first time. - The test docstring now matches the measured ~1.4s, which is spent while the render thread waits. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Brings in 9edeb6d: #708 (35 deprecated APIs removed), #709 (3.8.0 release), #710 (crisp speed advice, 30fps stepped penalty), #711 (scan compensation at held frames), #704 (run() golden traces and helpers), #707 (a raising display() counts toward the circuit breaker). Conflicts: - CHANGELOG.md: the branch's strip-memo entry sat in what was Unreleased at 695ff92, which #709 renamed 3.8.0, beside main's new Vegas-extension entry. Main's 3.8.0 section is kept exactly as on main; the strip-memo entry moves, unchanged, to a new "### Scrolling" subsection at the end of "## Unreleased". The memo is not part of 3.8.0. No other file conflicted. src/common/README.md auto-merged (main renamed module versions in the table; the branch added a paragraph under sports_scroll). Main did not touch sports_scroll.py, scroll_helper.py, sports_vegas.py or sports_live_scroll.py, so the memo's dependencies (has_strip, cached_array, reset_scroll, clear_cache, game_fingerprint, _freeze) are unchanged; none of #708's removed APIs is used. #710 changes which crisp speed solve_crisp picks, not how it is applied: each slate's display still keeps the pacing its own build set, and the real-scoreboard tests compare it with a fresh build. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 50 minutes. View limit detailsLimit details: You’ve used the included review currently available. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (4)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 165 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
CHANGELOG.md: this branch's Scrolling section kept under Unreleased next to main's new Web preview and Scroller-to-static handover sections. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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.
Why
A scoreboard in scroll mode froze the panel at the start of every recent/upcoming turn.
SportsScrollDisplayManager.prepare_and_display()redrew every card each turn while the render thread waited: about 1.4 s for seven football cards at 192×48 on ledpi. Mostly the games had not changed since the last turn.A naive "last build" memo would never hit.
_scroll_displayswas keyed by game type alone, and leagues that take turns share it (nfl_recent, thenncaa_fb_recent).What
Per-slate displays. Each slate, meaning game type plus leagues, keeps its own display in a small LRU: up to 4 per game type, with at most 6 MB per plugin of strips kept for slates not on screen.
get_scroll_display()and_scroll_displaysstill answer with the display on screen.Reuse instead of redraw. On a new turn the manager rewinds the strip it built last time, but only when everything it was drawn from is unchanged:
The strip must also still be present and under 10 minutes old.
Drawn as before: the first turn of each slate after a start, a slate whose games changed, live strips and a turn with no games.
Correctness guards:
clear()/clear_all()forget everything.Untouched: Vegas
prepare_contentand live-game scroll preservation (feat(common): sports consolidation stage 4 -- the identical sweep (plugin host, live scroll, display rules, font path) #705).Who it reaches: scoreboards set to scroll mode only. That is opt-in; the schema default is
switch. Switch-mode and Vegas users see no change.Known trade-off: a logo that was missing at the first build, or a card that raised once, stays that way for up to 10 minutes. Before, the next turn retried.
Tests
test/test_sports_scroll_strip_reuse.py(new, 53 tests):clear_allforces a rebuild;LEDMATRIX_PLUGINSset, real scoreboards run through the memo.41db488c.frame_soakhandover gaps (from fix(display): end the scroll state at scroller-to-static handovers #716) before and after.🤖 Generated with Claude Code