From 8e77076d0d6c8671ee562dea6b0fd6af253065d5 Mon Sep 17 00:00:00 2001 From: Jimmy Date: Sat, 26 Mar 2022 12:35:36 +1300 Subject: [PATCH] move tabbar lru_caches to instance level Flake8-bugbear correctly pointed out that TabBar instances would not be reliably cleaned up because the `self` reference would be cached in the lru_caches on a couple of the methods. And the caches are on the class so they last for the whole lifetime of the process. This commit move the caches to be created per instance, instead of on the class. Other options: 1. get rid of the caches From running the benchmark tests (eg `python3 -m pytest --benchmark-columns Min,Max,Median,Rounds -k test_update_tab_titles_benchmark`) it seems like the caches can still be helpful (even though when they were introduced in #3122 we didn't have the config cache either) 2. use cachetools to manage our own cache instead of using lru_cache in a non-standard way I don't feel like introducing a new dependency given this change didn't end up being too offensive. 3. clear the cache whenever a window get closed That would solve the "not getting deleted issue" but flake8 would still complain :) Possibly the cache size could be reduced now that there is going to be one per window. But the aren't caching large objects anyway. Flake8-bugbear change: https://github.com/PyCQA/flake8-bugbear/pull/218 Video that pointed out this way of using lru_cache: https://youtu.be/sVjtp6tGo0g --- qutebrowser/mainwindow/tabwidget.py | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/qutebrowser/mainwindow/tabwidget.py b/qutebrowser/mainwindow/tabwidget.py index 0e95b77457e..511c2c309b0 100644 --- a/qutebrowser/mainwindow/tabwidget.py +++ b/qutebrowser/mainwindow/tabwidget.py @@ -410,6 +410,15 @@ def __init__(self, win_id, parent=None): config.instance.changed.connect(self._on_config_changed) self._set_icon_size() QTimer.singleShot(0, self.maybe_hide) + self._minimum_tab_size_hint_helper = functools.lru_cache(maxsize=2**9)( + self._minimum_tab_size_hint_helper_uncached + ) + debugcachestats.register(name=f'tab width cache (win_id={win_id})')( + self._minimum_tab_size_hint_helper + ) + self._minimum_tab_height = functools.lru_cache(maxsize=1)( + self._minimum_tab_height_uncached + ) def __repr__(self): return utils.get_repr(self, count=self.count()) @@ -575,11 +584,9 @@ def minimumTabSizeHint(self, index: int, ellipsis: bool = True) -> QSize: icon_width, ellipsis, pinned) - @debugcachestats.register(name='tab width cache') - @functools.lru_cache(maxsize=2**9) - def _minimum_tab_size_hint_helper(self, tab_text: str, - icon_width: int, - ellipsis: bool, pinned: bool) -> QSize: + def _minimum_tab_size_hint_helper_uncached(self, tab_text: str, + icon_width: int, + ellipsis: bool, pinned: bool) -> QSize: """Helper function to cache tab results. Config values accessed in here should be added to _on_config_changed to @@ -610,8 +617,7 @@ def _text_to_width(text): width = max(min_width, width) return QSize(width, height) - @functools.lru_cache(maxsize=1) - def _minimum_tab_height(self): + def _minimum_tab_height_uncached(self): padding = config.cache['tabs.padding'] return self.fontMetrics().height() + padding.top + padding.bottom