perf(display): throttle the per-frame update tick; check strips without building them - #731
Merged
Merged
Conversation
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Uc5DAbSwGGUTm3MrCtpC2m
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Uc5DAbSwGGUTm3MrCtpC2m
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 46 minutes. View limit detailsLimit details: You’ve used the included review currently available. Review configuration: ⚙️ Run configuration
📒 Files selected for processing (6)
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 | 0 |
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.
# Conflicts: # CHANGELOG.md
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
1 participant
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.
Summary
Two per-frame costs in the render loop:
PluginManager.run_scheduled_updates()after every frame, ~125×/s on a scroller, almost always to find nothing due (~100 µs with 20 plugins on a Pi 4). They now run it at most every 0.25 s. No update interval is shorter than 5 s, and the top of each loop pass still ticks unthrottled.SportsScrollDisplayand the sync follower asked "is there a strip?" by readingScrollHelper.cached_image, which builds and keeps the image from the array after a deferred rebuild (3.5 ms and ~1 MiB for a 4288x64 strip). They now askhas_strip(). A helper withouthas_stripis still asked the old way.Type of change
Test plan
Ran on a real Raspberry Pi with hardware: hdpi (Pi 4, 512x64), 40-min windows in an A/B/B/A soak, each from a fresh restart, Vegas mode, same music load. This branch vs the
mainwindow after it:mainThe CPU difference is about what the ~1% estimate predicted and is within run-to-run noise. The result is "no regression", not a measured win. The strip change mostly saves memory on sports scrollers, which these windows didn't spend much time in.
Ran the test suite (
pytest): newtest_plugin_update_tick_throttle.py(on the realrun()through the golden-trace harness),test_sports_scroll_strip_check.py,test_follower_scroll_image_handoff.py. Rebased onto fix(ipc): a plugin reload no longer freezes the panel during Vegas #723 and re-ran its reload/IPC tests (261 passed). The full suite passes, apart from 8psutil-dependent tests that fail the same way onmain.Plugin compatibility
Notes for reviewer
Since #723 a plugin reload can finish between frames rather than at the top of the loop. The reloaded plugin's first update then comes from the next throttled tick, at most 0.25 s later.
🤖 Generated with Claude Code