perf(ui): memoize Backend list properties to avoid per-paint rebuilds#181
Open
hughesyadaddy wants to merge 1 commit into
Open
perf(ui): memoize Backend list properties to avoid per-paint rebuilds#181hughesyadaddy wants to merge 1 commit into
hughesyadaddy wants to merge 1 commit into
Conversation
The five list-typed ``@Property`` getters on ``Backend`` -- ``buttons``, ``profiles``, ``knownApps``, ``actionCategories``, ``allActions`` -- are read by QML bindings inside delegate rebuilds, so every paint of the mappings list, profile selector, or action picker rebuilt the lists from scratch. The worst offenders were ``profiles`` (re-resolving every profile's apps through ``get_icon_for_exe`` and ``app_catalog.get_app_label``) and ``knownApps`` (walking the full catalog and resolving an icon for every entry). Each getter now returns a cached snapshot until its notify signal -- or a structurally-dependent signal -- fires. The mapping of caches to invalidating signals is: * buttons -- mappingsChanged, deviceLayoutChanged * profiles -- profilesChanged, activeProfileChanged * knownApps -- knownAppsChanged * actionCategories -- deviceLayoutChanged * allActions -- deviceLayoutChanged The notify connections are wired once in ``__init__``; nothing else changes about the public signal contract. Pure rebuilds still happen exactly when the underlying data changes; redundant rebuilds during delegate paints no longer happen at all. Tests in ``tests/test_backend.py``: * identity check that consecutive reads of ``buttons`` return the same object (memoized); * each notify signal invalidates exactly the caches it must (mappings → buttons, device layout → buttons + actionCategories + allActions, active profile → profiles, known apps → knownApps); * unrelated signals do not invalidate caches that should be stable across them. No behavioral change: the cached lists are computed by the same code as before, factored into ``_compute_*`` helpers so the cache wrappers stay trivial.
hughesyadaddy
force-pushed
the
perf/backend-list-property-memoization
branch
from
25dd282 to
cccf841
Compare
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
The five list-typed
@Propertygetters onBackend—buttons,profiles,knownApps,actionCategories,allActions— are read by QML bindings inside delegate rebuilds. Every paint of the mappings list, the profile selector, or the action picker rebuilt the corresponding list from scratch, which in the case ofprofilesmeant re-resolving every profile's apps throughget_icon_for_exeandapp_catalog.get_app_labelon every frame.Approach
Each getter returns a cached snapshot. The cache is invalidated by the property's notify signal — plus any other signal whose emission actually changes the list's contents.
buttonsmappingsChanged,deviceLayoutChangedprofilesprofilesChanged,activeProfileChangedknownAppsknownAppsChangedactionCategoriesdeviceLayoutChangedallActionsdeviceLayoutChangedThe notify connections are wired once in
Backend.__init__. The public signal contract is unchanged — every signal still fires exactly when it did before; we just stop recomputing when nothing has changed.Each getter is split into an internal
_compute_X()helper so the cache wrapper stays trivial and the original computation logic stays untouched (no behavioral risk).Test plan
New
BackendListPropertyMemoizationTestsintests/test_backend.py(7 cases):buttonsreturn the same object (memoized);mappingsChangedinvalidatesbuttons;deviceLayoutChangedinvalidatesbuttons,actionCategories,allActions;activeProfileChangedinvalidatesprofiles;knownAppsChangedinvalidatesknownApps;profilescache survivesknownAppsChangedandmappingsChanged;knownAppscache survivesprofilesChanged,mappingsChanged,deviceLayoutChanged;pytest tests/ -q— 502 passed, 1 skipped, 170 subtests passed.Notes
_compute_*split is deliberate: it keeps the diff readable as "wrap each getter" and makes the rebuild paths easy to test in isolation.