From cccf8410d0f8f02c60512e15d1de16f5babf64c2 Mon Sep 17 00:00:00 2001 From: hughesyadaddy Date: Mon, 18 May 2026 10:26:16 -0400 Subject: [PATCH] perf(ui): memoize Backend list properties to avoid per-paint rebuilds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- tests/test_backend.py | 69 ++++++++++++++++++++++++++++ ui/backend.py | 102 ++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 168 insertions(+), 3 deletions(-) diff --git a/tests/test_backend.py b/tests/test_backend.py index 13102769..34bbc6c0 100644 --- a/tests/test_backend.py +++ b/tests/test_backend.py @@ -1273,5 +1273,74 @@ def test_set_start_minimized_does_not_call_apply_login_startup(self): self.assertFalse(backend.startMinimized) +@unittest.skipIf(Backend is None, "PySide6 not installed in test environment") +class BackendListPropertyMemoizationTests(unittest.TestCase): + """The five ``@Property(list, ...)`` getters on ``Backend`` (``buttons``, + ``profiles``, ``knownApps``, ``actionCategories``, ``allActions``) are + read by every QML binding that depends on them, including those evaluated + inside delegate rebuilds. Without memoization the lists -- and their + per-entry catalog / icon lookups -- were rebuilt on every paint.""" + + def _build(self, cfg=None): + loaded = copy.deepcopy(cfg or DEFAULT_CONFIG) + with ( + patch("ui.backend.load_config", return_value=loaded), + patch("ui.backend.save_config"), + patch("ui.backend.supports_login_startup", return_value=False), + ): + return Backend(engine=None) + + def test_buttons_returns_same_object_across_reads(self): + backend = self._build() + first = backend.buttons + second = backend.buttons + self.assertIs(first, second) + + def test_mappings_changed_invalidates_buttons_cache(self): + backend = self._build() + before = backend.buttons + backend.mappingsChanged.emit() + after = backend.buttons + self.assertIsNot(before, after) + self.assertEqual(before, after) + + def test_device_layout_changed_invalidates_buttons_and_actions(self): + backend = self._build() + buttons_before = backend.buttons + cats_before = backend.actionCategories + actions_before = backend.allActions + backend.deviceLayoutChanged.emit() + self.assertIsNot(backend.buttons, buttons_before) + self.assertIsNot(backend.actionCategories, cats_before) + self.assertIsNot(backend.allActions, actions_before) + + def test_active_profile_changed_invalidates_profiles_cache(self): + backend = self._build() + before = backend.profiles + backend.activeProfileChanged.emit() + self.assertIsNot(backend.profiles, before) + + def test_known_apps_changed_invalidates_known_apps_cache(self): + backend = self._build() + before = backend.knownApps + backend.knownAppsChanged.emit() + self.assertIsNot(backend.knownApps, before) + + def test_profiles_cache_not_invalidated_by_unrelated_signals(self): + backend = self._build() + first = backend.profiles + backend.knownAppsChanged.emit() + backend.mappingsChanged.emit() + self.assertIs(backend.profiles, first) + + def test_known_apps_cache_not_invalidated_by_unrelated_signals(self): + backend = self._build() + first = backend.knownApps + backend.profilesChanged.emit() + backend.mappingsChanged.emit() + backend.deviceLayoutChanged.emit() + self.assertIs(backend.knownApps, first) + + if __name__ == "__main__": unittest.main() diff --git a/ui/backend.py b/ui/backend.py index db48f74b..51bbcdc2 100644 --- a/ui/backend.py +++ b/ui/backend.py @@ -281,6 +281,18 @@ def __init__(self, engine=None, parent=None, root_dir=None): self._update_timer.setInterval(DEFAULT_AUTO_CHECK_INTERVAL_SECONDS * 1000) self._update_timer.timeout.connect(lambda: self._startUpdateCheck(manual=False)) + # Lazily-computed list snapshots for QML bindings. Every read of a + # ``@Property(list, ...)`` returns the cached value until the + # property's notify signal (or a structurally-dependent signal) + # fires and clears it. Without these caches QML repaints rebuild + # the lists on every binding evaluation -- profiles re-runs + # ``app_catalog`` lookups, knownApps walks the catalog, etc. + self._buttons_cache: list | None = None + self._profiles_cache: list | None = None + self._known_apps_cache: list | None = None + self._action_categories_cache: list | None = None + self._all_actions_cache: list | None = None + # Cross-thread signal connections self._profileSwitchRequest.connect( self._handleProfileSwitch, Qt.QueuedConnection) @@ -307,6 +319,15 @@ def __init__(self, engine=None, parent=None, root_dir=None): self._updateInstallProgressRequest.connect( self._handleUpdateInstallProgress, Qt.QueuedConnection) + # List-property cache invalidation. Each notify signal maps to the + # subset of caches that depends on it; reads after the next emit + # rebuild lazily. + self.mappingsChanged.connect(self._invalidate_buttons_cache) + self.profilesChanged.connect(self._invalidate_profiles_cache) + self.activeProfileChanged.connect(self._invalidate_profiles_cache) + self.knownAppsChanged.connect(self._invalidate_known_apps_cache) + self.deviceLayoutChanged.connect(self._invalidate_device_dependent_caches) + # Wire engine callbacks if engine: engine.set_profile_change_callback(self._onEngineProfileSwitch) @@ -358,7 +379,20 @@ def __init__(self, engine=None, parent=None, root_dir=None): @Property(list, notify=mappingsChanged) def buttons(self): - """List of button dicts for the active profile, filtered by device.""" + """List of button dicts for the active profile, filtered by device. + + Cached -- invalidated by ``mappingsChanged`` (active-profile + mappings) and ``deviceLayoutChanged`` (effective supported-button + set). The QML mappings list binds to ``backend.buttons`` and + re-evaluates on every paint of the row delegate, so without the + cache this rebuilt a list of ~10 dicts and a per-button + ``_action_label`` lookup on every frame the user scrolled. + """ + if self._buttons_cache is None: + self._buttons_cache = self._compute_buttons() + return self._buttons_cache + + def _compute_buttons(self): mappings = get_active_mappings(self._cfg) device_buttons = set( self._effective_supported_buttons or BUTTON_NAMES.keys() @@ -379,6 +413,24 @@ def buttons(self): }) return result + def _invalidate_buttons_cache(self) -> None: + self._buttons_cache = None + + def _invalidate_profiles_cache(self) -> None: + self._profiles_cache = None + + def _invalidate_known_apps_cache(self) -> None: + self._known_apps_cache = None + + def _invalidate_device_dependent_caches(self) -> None: + # ``buttons`` depends on ``_effective_supported_buttons`` and + # ``actionCategories`` / ``allActions`` depend on the hidden-action + # filter that is itself derived from the layout. A device swap + # invalidates all three at once. + self._buttons_cache = None + self._action_categories_cache = None + self._all_actions_cache = None + def _hidden_actions(self): """Return set of action IDs to hide based on effective device buttons.""" btns = self._effective_supported_buttons @@ -390,7 +442,18 @@ def _hidden_actions(self): @Property(list, notify=deviceLayoutChanged) def actionCategories(self): - """Actions grouped by category, filtered by device capabilities.""" + """Actions grouped by category, filtered by device capabilities. + + Cached -- invalidated by ``deviceLayoutChanged``. The grouped + structure is rebuilt across the whole ``ACTIONS`` registry on + every read; the QML action picker binds to this property and + rebuilt it on every focus/visibility change before the cache. + """ + if self._action_categories_cache is None: + self._action_categories_cache = self._compute_action_categories() + return self._action_categories_cache + + def _compute_action_categories(self): from collections import OrderedDict hidden = self._hidden_actions() cats = OrderedDict() @@ -414,7 +477,15 @@ def actionCategories(self): @Property(list, notify=deviceLayoutChanged) def allActions(self): - """Flat sorted action list (Do Nothing first), filtered by device.""" + """Flat sorted action list (Do Nothing first), filtered by device. + + Cached -- invalidated by ``deviceLayoutChanged``. + """ + if self._all_actions_cache is None: + self._all_actions_cache = self._compute_all_actions() + return self._all_actions_cache + + def _compute_all_actions(self): hidden = self._hidden_actions() result = [] none_data = ACTIONS.get("none") @@ -711,6 +782,20 @@ def gestureRecords(self): @Property(list, notify=profilesChanged) def profiles(self): + """Profile snapshots for the QML profile selector. + + Cached -- invalidated by ``profilesChanged`` (catalog churn) and + ``activeProfileChanged`` (the ``isActive`` flag per row). The + compute path walks every profile's apps and resolves each through + ``get_icon_for_exe`` and ``app_catalog.get_app_label`` -- both + non-trivial in a profile with several apps, so rebuilding on + every QML read was the worst per-paint allocator in this file. + """ + if self._profiles_cache is None: + self._profiles_cache = self._compute_profiles() + return self._profiles_cache + + def _compute_profiles(self): result = [] active = self._cfg.get("active_profile", "default") for pname, pdata in self._cfg.get("profiles", {}).items(): @@ -727,6 +812,17 @@ def profiles(self): @Property(list, notify=knownAppsChanged) def knownApps(self): + """Catalog snapshot for the QML known-apps picker. + + Cached -- invalidated by ``knownAppsChanged``. The catalog itself + is essentially static for a session, so this is the highest-hit + memoization target of the five. + """ + if self._known_apps_cache is None: + self._known_apps_cache = self._compute_known_apps() + return self._known_apps_cache + + def _compute_known_apps(self): result = [] for entry in app_catalog.get_app_catalog(): icon = get_icon_for_exe(entry.get("path", ""))