diff options
author | Jimmy <jimmy@spalge.com> | 2022-03-26 12:35:36 +1300 |
---|---|---|
committer | Jimmy <jimmy@spalge.com> | 2022-03-26 13:00:54 +1300 |
commit | b830c13c0bece14201ac1cf264c0550ed1f3dd4b (patch) | |
tree | 31537cd1c3d6e567df4423e997ed10a94fa285f1 | |
parent | 87b9dab1d8647c09fe45b8355e27b3bafa455936 (diff) | |
download | qutebrowser-b830c13c0bece14201ac1cf264c0550ed1f3dd4b.tar.gz qutebrowser-b830c13c0bece14201ac1cf264c0550ed1f3dd4b.zip |
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
-rw-r--r-- | qutebrowser/mainwindow/tabwidget.py | 20 |
1 files changed, 13 insertions, 7 deletions
diff --git a/qutebrowser/mainwindow/tabwidget.py b/qutebrowser/mainwindow/tabwidget.py index 0e95b7745..511c2c309 100644 --- a/qutebrowser/mainwindow/tabwidget.py +++ b/qutebrowser/mainwindow/tabwidget.py @@ -410,6 +410,15 @@ class TabBar(QTabBar): 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 @@ class TabBar(QTabBar): 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 @@ class TabBar(QTabBar): 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 |