[RFC] Even lazier load - delay creating WebView/WebEngineView until the tab is focused on

[coiby]

Hi,

This yet another attempt at solving #67 which is based on the work #4037 done by @phmongeau,

It would probably be possible to avoid creating the webview untill the tab is loaded, but I haven't looked into that yet.

It will not create WebView/WebEngineView until the tab is focused on thus no QtWebEngineProcess/QtWebProcess will be spawned initally.

If it's worthwile to do lazy load in this way, I'll fix the test failures introduced by the commit soon.

Any comment is welcome!

Thank you!

[The-Compiler]

Just a word of warning, it will likely take me some time until I'll get around to looking at this, because there is quite a lot going on here.

I might also need to look at #5359 first, which will probably refactor things which will cause a lot of conflicts with this change, but that's definitely the more urgent change as it should go in before Qt 5.15 is released.

[The-Compiler]

I looked at this yesterday before going to bed, that probably was a bad idea - now that I'm a bit more awake, I realize that the only difference to the existing PR is a6fdfbd.

I'm not sure if this will work correctly. I'm actually surprised as it works better as I thought when looking at the code. For example, using 2r to reload a tab which shouldn't have a widget yet (if I'm not mistaken?) seems to work just fine?

[coiby]

On Tue, Apr 21, 2020 at 01:18:18AM -0700, Florian Bruhin wrote:

I looked at this yesterday before going to bed, that probably was a bad idea - now that I'm a bit more awake, I realize that the only difference to the existing PR is a6fdfbd.

Thank you for reviewing the code! Sorry for using 'RFC' as the pull request's prefix which seems to be a bit misleading. It's more of proof of concept and I also expected to see some feedback similar to made by @blueyed since I am not sure if we should implement lazy loading like this pull request,

With 247 tabs, on master qutebrowser takes 45s to display the window, and ~3 minutes to report
a blocked request. (The (current) tab ends up being blank then additionally (maybe only sometimes))

VIRT RES SHR S %CPU %MEM TIME+ COMMAND
9796.9m 2.7g 206680 S 6.6 17.2 2:44.47 qutebrowser
On this branch (rebased on master) it displays the first tab after 5s already, and uses much less memory:

VIRT RES SHR S %CPU %MEM TIME+ COMMAND
2749876 389692 186612 T 0.0 2.4 0:08.22 qutebrowser

While playing with QWebEnginePage.LifecycleState API to suspend tabs, I was curious to see to what degree we can reduce memory consumption. So I discard all inactive tabs by setting LifecycleState to Discarded immediately when loading from a session if session.lazy_load=True. But this still leads to a spike of memory and CPU usage when starting qutebrowser. So why not stop spawning webengine process in the first place? When investigating phmongeau's work #4037, I realized I can quickly test out this idea based on his work. So thanks to @phmongeau's work, not much change is needed although some code can be cleaned out if we choose to implement lazy loading in this way.

I'm not sure if this will work correctly. I'm actually surprised as it works better as I thought when looking at the code. For example, using 2r to reload a tab which shouldn't have a widget yet (if I'm not mistaken?) seems to work just fine?

Actually it double-surprised me. First, I didn't know there is such syntax as 2r in qutebrowser. Secondly, I also expected the tab will only be loaded during showEvent. I'm going to check what happened.

[coiby]

I'm not sure if this will work correctly. I'm actually surprised as it works better as I thought when looking at the code. For example, using 2r to reload a tab which shouldn't have a widget yet (if I'm not mistaken?) seems to work just fine?

Actually it double-surprised me. First, I didn't know there is such syntax as 2r in qutebrowser. Secondly, I also expected the tab will only be loaded during showEvent. I'm going to check what happened.

Yet more surprises,

1. showEvent is triggered for each tab even no tab swiching has been done when starting qutebrowser. So WebEngineView widget has been created after qutebrowser is fully started.
2. @phmongeau's work has already achived of the aim of not spawaning QtWebEngineProcess for each tab. Initally I thought if onece WebEngineView object is created, QtWebEngineProcess will be spawned immediately which turned out to be a wrong assumption.

It's almost certian this approach of implementing lazy loading is futile since no additional benefits are gained in comparision to @phmongeau's approach.

[The-Compiler]

Actually it double-surprised me. First, I didn't know there is such syntax as 2r in qutebrowser.

That's using a keybinding/command with a count. For most commands, it just multiples an action (e.g. 10j will scroll down 10 times as much as a single j), but with some commands where it makes sense, a count can be used to select a tab instead.

1. showEvent is triggered for each tab even no tab swiching has been done when starting qutebrowser. So WebEngineView widget has been created after qutebrowser is fully started.

I'm guessing tabs are sometimes shown when they shouldn't be. There are various workarounds to that effect (for QtWebEngine issues, especially in older Qt versions). Also possibly related: #4923, #5377.

We should probably check if it's still needed to do that, and under what circumstances - but there are various subtle things going on there. I'll probably look into it as part of reviewing #4923.

2. @phmongeau's work has already achived of the aim of not spawaning QtWebEngineProcess for each tab. Initally I thought if onece WebEngineView object is created, QtWebEngineProcess will be spawned immediately which turned out to be a wrong assumption.

Yep. Confirmed by a small test script, even with older Qt versions those are initialized lazily:

import os

from PyQt5.QtCore import QUrl, QTimer
from PyQt5.QtWidgets import QApplication
from PyQt5.QtWebEngineWidgets import QWebEngineView

app = QApplication([])

view1 = QWebEngineView()
view1.load(QUrl('https://qutebrowser.org'))

view2 = QWebEngineView()

print(f"PID: {os.getpid()}")

def load():
    print("Loading page in second view")
    view2.load(QUrl('https://python.org'))

QTimer.singleShot(8000, load)

app.exec_()

This can also be seen in the QWebEnginePage sources (see the ensureInitialized() calls and the showEvent() override).

It's almost certian this approach of implementing lazy loading is futile since no additional benefits are gained in comparision to @phmongeau's approach.

Agreed, so I'm closing this. A proper implementation would likely be quite painful (especially once extensions in #30 come into play: The entire tab API would need to handle the case of there not being a widget available).

[coiby]

Actually it double-surprised me. First, I didn't know there is such syntax as 2r in qutebrowser.

That's using a keybinding/command with a count. For most commands, it just multiples an action (e.g. 10j will scroll down 10 times as much as a single j), but with some commands where it makes sense, a count can be used to select a tab instead.

Thank you for the introduction! This is a handy feature!

1. showEvent is triggered for each tab even no tab swiching has been done when starting qutebrowser. So WebEngineView widget has been created after qutebrowser is fully started.

I'm guessing tabs are sometimes shown when they shouldn't be. There are various workarounds to that effect (for QtWebEngine issues, especially in older Qt versions). Also possibly related: #4923, #5377.

Yes, I noticed when loading a session, setCurrentWidget(tab) is applied to each inactive tab as long as background=False which is why showEvent is triggered for all tabs.

# qutebrowser/mainwindow/tabbedbrowser.py
    def tabopen(
            self, url: QUrl = None,
            background: bool = None,
            related: bool = True,
            idx: int = None,
            from_session: bool = False,
    ) -> browsertab.AbstractTab:
        if background:
            # Make sure the background tab has the correct initial size.
            # With a foreground tab, it's going to be resized correctly by the
            # layout anyways.
            tab.resize(self.widget.currentWidget().size())
            self.widget.tab_index_changed.emit(self.widget.currentIndex(),
                                               self.widget.count())
            # Refocus webview in case we lost it by spawning a bg tab
            self.widget.currentWidget().setFocus()
        else:
            self.widget.setCurrentWidget(tab)
            # WORKAROUND for https://bugreports.qt.io/browse/QTBUG-68076
            # Still seems to be needed with Qt 5.11.1
            tab.setFocus()

1. @phmongeau's work has already achived of the aim of not spawaning QtWebEngineProcess for each tab. Initally I thought if onece WebEngineView object is created, QtWebEngineProcess will be spawned immediately which turned out to be a wrong assumption.

Yep. Confirmed by a small test script, even with older Qt versions those are initialized lazily:

This can also be seen in the QWebEnginePage sources (see the ensureInitialized() calls and the showEvent() override).

It's almost certian this approach of implementing lazy loading is futile since no additional benefits are gained in comparision to @phmongeau's approach.

Agreed, so I'm closing this. A proper implementation would likely be quite painful (especially once extensions in #30 come into play: The entire tab API would need to handle the case of there not being a widget available).

One thing I learn from this experiment is even after a list of AbstractHistoryItems are added to the tab's history in showEvent, QtWebEngineProcess is not spawned. However if we load AbstractHistoryItems directly after creating a browsertab, QtWebEngineProcess will be spawned. So if there a way to explicitly tell QtWebEngine to stop spawning QtWebEngineProcess after loading AbstractHistoryItems, we may do lazy loading in a much simpler way. I haven't figured out why in showEvent loading AbstractHistoryItems doesn't lead to spawning QtWebEngineProcess. Thank you for pointing me to the QWebEnginePage sources.