Repository: qutebrowser/qutebrowser
Pull request: Improve scroll performance when using mouse, touchpad or touchscreens
Previously, the scroll widget was adding a lot of overhead when smooth-scrolling, leading to choppy scrolling. By throttling the scroll widget, scroll performance is much improved (at the cost of the scroll widget seeming choppy instead). 

In order to minimize the impact, I wrote a small throttling script, which calls the function immediately if possible (within the time limit). If not, it will wait until the throttle time is up, and call it then. If more calls come in, it will update the pending call rather than spawning new ones.

I'm not sure if this is worth being configurable or not.

I also have a bunch of other minor optimizations in the first commit.

Fixes #4475

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/qutebrowser/qutebrowser/4498)
<!-- Reviewable:end -->


Problem statement:
Scroll widget affects scrolling performance
Turning the scroll widget (`statusbar.widgets`) off results in a noticeable increase in scrolling performance when using a touchpad or touchscreen. We should find some way to make the scrolling widget have a smaller impact, maybe by running setText in the background (since it's not essential we're correct all the time there).

We probably should add a benchmark for the entire scroll hook as well, as for smooth-scrolling, it's important for that to be as fast as possible.

Hints from issue discussion:
Also see #2233.

Commit message: Add throttling to scroll widget

Patch to review:
diff --git a/qutebrowser/browser/signalfilter.py b/qutebrowser/browser/signalfilter.py
index 7cc46abdb58..b6b5b56dc60 100644
--- a/qutebrowser/browser/signalfilter.py
+++ b/qutebrowser/browser/signalfilter.py
@@ -56,9 +56,10 @@ def create(self, signal, tab):
         Return:
             A partial function calling _filter_signals with a signal.
         """
-        return functools.partial(self._filter_signals, signal, tab)
+        log_signal = debug.signal_name(signal) not in self.BLACKLIST
+        return functools.partial(self._filter_signals, signal, log_signal, tab)
 
-    def _filter_signals(self, signal, tab, *args):
+    def _filter_signals(self, signal, log_signal, tab, *args):
         """Filter signals and trigger TabbedBrowser signals if needed.
 
         Triggers signal if the original signal was sent from the _current_ tab
@@ -72,7 +73,6 @@ def _filter_signals(self, signal, tab, *args):
             tab: The WebView which the filter belongs to.
             *args: The args to pass to the signal.
         """
-        log_signal = debug.signal_name(signal) not in self.BLACKLIST
         tabbed_browser = objreg.get('tabbed-browser', scope='window',
                                     window=self._win_id)
         try:
diff --git a/qutebrowser/mainwindow/statusbar/percentage.py b/qutebrowser/mainwindow/statusbar/percentage.py
index a362fd9d6a2..2e902ff6eeb 100644
--- a/qutebrowser/mainwindow/statusbar/percentage.py
+++ b/qutebrowser/mainwindow/statusbar/percentage.py
@@ -22,6 +22,7 @@
 from PyQt5.QtCore import pyqtSlot
 
 from qutebrowser.mainwindow.statusbar import textbase
+from qutebrowser.misc.throttle import throttle
 
 
 class Percentage(textbase.TextBase):
@@ -34,6 +35,7 @@ def __init__(self, parent=None):
         self.set_perc(0, 0)
         self.raw = False
 
+    @throttle(100)
     @pyqtSlot(int, int)
     def set_perc(self, x, y):  # pylint: disable=unused-argument
         """Setter to be used as a Qt slot.
@@ -49,8 +51,8 @@ def set_perc(self, x, y):  # pylint: disable=unused-argument
         elif y is None:
             self.setText('[???]')
         else:
-            text = '[{:02}]' if self.raw else '[{:02}%]'
-            self.setText(text.format(y))
+            text = '[' + str(y).zfill(2) + (']' if self.raw else '%]')
+            self.setText(text)
 
     def on_tab_changed(self, tab):
         """Update scroll position when tab changed."""
diff --git a/qutebrowser/mainwindow/tabbedbrowser.py b/qutebrowser/mainwindow/tabbedbrowser.py
index 9c14df3ae8d..13e5ab8dd0c 100644
--- a/qutebrowser/mainwindow/tabbedbrowser.py
+++ b/qutebrowser/mainwindow/tabbedbrowser.py
@@ -184,7 +184,7 @@ def _update_window_title(self, field=None):
             field: A field name which was updated. If given, the title
                    is only set if the given field is in the template.
         """
-        title_format = config.val.window.title_format
+        title_format = config.cache['window.title_format']
         if field is not None and ('{' + field + '}') not in title_format:
             return
 
diff --git a/qutebrowser/misc/throttle.py b/qutebrowser/misc/throttle.py
new file mode 100644
index 00000000000..615323ae87f
--- /dev/null
+++ b/qutebrowser/misc/throttle.py
@@ -0,0 +1,72 @@
+# vim: ft=python fileencoding=utf-8 sts=4 sw=4 et:
+
+# Copyright 2014-2018 Jay Kamat <jaygkamat@gmail.com>
+#
+# This file is part of qutebrowser.
+#
+# qutebrowser is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# qutebrowser is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with qutebrowser.  If not, see <http://www.gnu.org/licenses/>.
+
+"""A simple throttling decorator."""
+
+import typing
+import time
+
+from PyQt5.QtCore import QTimer
+
+
+class throttle:  # noqa: N801,N806 pylint: disable=invalid-name
+
+    """A simple function decorator to throttle calls.
+
+    If a request comes in, it will be processed immedaitely. If another request
+    comes in too soon, it is ignored, but will be processed when a timeout
+    ends. If another request comes in, it will update the pending request.
+
+    """
+
+    def __init__(self, throttle_ms: int) -> None:
+        """Save arguments for throttle decorator.
+
+        Args:
+            throttle_ms: The time to wait before allowing another call of the
+                         function.
+        """
+        self.throttle_ms = throttle_ms
+        # False if no call is pending, a list of arguments (to call with) if a
+        # call is pending.
+        self.pending_call = False
+        self.last_call_ms = 0
+
+    def __call__(self, func: typing.Callable) -> typing.Callable:
+        def wrapped_fn(*args):
+            cur_time_ms = int(time.time() * 1000)
+            if self.pending_call is False:
+                if cur_time_ms - self.last_call_ms > self.throttle_ms:
+                    # Call right now
+                    self.last_call_ms = cur_time_ms
+                    func(*args)
+
+                if self.pending_call is False:
+                    # Start a pending call
+                    def call_pending():
+                        func(*self.pending_call)
+                        self.pending_call = False
+                        self.last_call_ms = int(time.time() * 1000)
+
+                    QTimer.singleShot(
+                        self.throttle_ms - (cur_time_ms - self.last_call_ms),
+                        call_pending)
+            # Update arguments for an existing pending call
+            self.pending_call = args
+        return wrapped_fn
diff --git a/tests/unit/misc/test_throttle.py b/tests/unit/misc/test_throttle.py
new file mode 100644
index 00000000000..123d7bd16a1
--- /dev/null
+++ b/tests/unit/misc/test_throttle.py
@@ -0,0 +1,110 @@
+# vim: ft=python fileencoding=utf-8 sts=4 sw=4 et:
+
+# Copyright 2018 Jay Kamat <jaygkamat@gmail.com>:
+#
+# This file is part of qutebrowser.
+#
+# qutebrowser is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# qutebrowser is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with qutebrowser.  If not, see <http://www.gnu.org/licenses/>.
+
+# False-positives
+
+"""Tests for qutebrowser.misc.throttle."""
+
+from unittest import mock
+
+from qutebrowser.misc.throttle import throttle
+from qutebrowser.utils import usertypes
+
+
+def test_throttle_imm(qtbot):
+    func = mock.Mock()
+    throttled_func = throttle(100)(func)
+    throttled_func("foo")
+    throttled_func("foo")
+    func.assert_called_once_with("foo")
+
+
+def test_throttle_delay(qtbot):
+    func = mock.Mock()
+    throttled_func = throttle(100)(func)
+    throttled_func("foo")
+    throttled_func("foo")
+    throttled_func("foo")
+    throttled_func("bar")
+    func.assert_called_once_with("foo")
+    func.reset_mock()
+
+    t = usertypes.Timer()
+    with qtbot.waitSignal(t.timeout, timeout=500):
+        t.start(200)
+
+    func.assert_called_once_with("bar")
+
+
+def test_throttle_delay_imm_delay(qtbot):
+    func = mock.Mock()
+    throttled_func = throttle(100)(func)
+    throttled_func("foo")
+    throttled_func("foo")
+    throttled_func("foo")
+    throttled_func("bar")
+    func.assert_called_once_with("foo")
+    func.reset_mock()
+
+    t = usertypes.Timer()
+    with qtbot.waitSignal(t.timeout, timeout=500):
+        t.start(400)
+
+    func.assert_called_once_with("bar")
+    func.reset_mock()
+    throttled_func("baz")
+    throttled_func("baz")
+    throttled_func("bop")
+
+    func.assert_called_once_with("baz")
+    func.reset_mock()
+
+    t = usertypes.Timer()
+    with qtbot.waitSignal(t.timeout, timeout=500):
+        t.start(200)
+
+    func.assert_called_once_with("bop")
+
+
+def test_throttle_delay_delay(qtbot):
+    func = mock.Mock()
+    throttled_func = throttle(100)(func)
+    throttled_func("foo")
+    throttled_func("foo")
+    throttled_func("foo")
+    throttled_func("bar")
+    func.assert_called_once_with("foo")
+    func.reset_mock()
+
+    t = usertypes.Timer()
+    with qtbot.waitSignal(t.timeout, timeout=500):
+        t.start(150)
+
+    func.assert_called_once_with("bar")
+    func.reset_mock()
+    throttled_func("baz")
+    throttled_func("baz")
+    throttled_func("bop")
+
+    t = usertypes.Timer()
+    with qtbot.waitSignal(t.timeout, timeout=500):
+        t.start(200)
+
+    func.assert_called_once_with("bop")
+    func.reset_mock()


Code review comments:
diff_hunk: @@ -0,0 +1,72 @@
+# vim: ft=python fileencoding=utf-8 sts=4 sw=4 et:
+
+# Copyright 2014-2018 Jay Kamat <jaygkamat@gmail.com>
+#
+# This file is part of qutebrowser.
+#
+# qutebrowser is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# qutebrowser is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with qutebrowser.  If not, see <http://www.gnu.org/licenses/>.
+
+"""A simple throttling decorator."""
+
+import typing
+import time
+
+from PyQt5.QtCore import QTimer
+
+
+class throttle:  # noqa: N801,N806 pylint: disable=invalid-name
+
+    """A simple function decorator to throttle calls.
+
+    If a request comes in, it will be processed immedaitely. If another request
+    comes in too soon, it is ignored, but will be processed when a timeout
+    ends. If another request comes in, it will update the pending request.
+
+    """
+
+    def __init__(self, throttle_ms: int) -> None:
+        """Save arguments for throttle decorator.
+
+        Args:
+            throttle_ms: The time to wait before allowing another call of the
+                         function.
+        """
+        self.throttle_ms = throttle_ms
+        # False if no call is pending, a list of arguments (to call with) if a
+        # call is pending.
+        self.pending_call = False
path: qutebrowser/misc/throttle.py
text: @user1:
What do you think about supporting `**kwargs` as well? Maybe just make it a `([], {})` tuple?

diff_hunk: @@ -0,0 +1,72 @@
+# vim: ft=python fileencoding=utf-8 sts=4 sw=4 et:
+
+# Copyright 2014-2018 Jay Kamat <jaygkamat@gmail.com>
+#
+# This file is part of qutebrowser.
+#
+# qutebrowser is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# qutebrowser is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with qutebrowser.  If not, see <http://www.gnu.org/licenses/>.
+
+"""A simple throttling decorator."""
+
+import typing
+import time
+
+from PyQt5.QtCore import QTimer
+
+
+class throttle:  # noqa: N801,N806 pylint: disable=invalid-name
+
+    """A simple function decorator to throttle calls.
+
+    If a request comes in, it will be processed immedaitely. If another request
+    comes in too soon, it is ignored, but will be processed when a timeout
+    ends. If another request comes in, it will update the pending request.
+
+    """
+
+    def __init__(self, throttle_ms: int) -> None:
+        """Save arguments for throttle decorator.
+
+        Args:
+            throttle_ms: The time to wait before allowing another call of the
+                         function.
+        """
+        self.throttle_ms = throttle_ms
+        # False if no call is pending, a list of arguments (to call with) if a
+        # call is pending.
+        self.pending_call = False
+        self.last_call_ms = 0
+
+    def __call__(self, func: typing.Callable) -> typing.Callable:
+        def wrapped_fn(*args):
+            cur_time_ms = int(time.time() * 1000)
+            if self.pending_call is False:
+                if cur_time_ms - self.last_call_ms > self.throttle_ms:
+                    # Call right now
+                    self.last_call_ms = cur_time_ms
+                    func(*args)
+
+                if self.pending_call is False:
path: qutebrowser/misc/throttle.py
text: @user1:
Isn't this always going to be true here? Also it will be executed for an immediate call too. Maybe get rid of this conditional and add a return for the immediate call case.