1357242 - Make initial browser remote by default in XUL markup

Status: RESOLVED WORKSFORME

(Firefox :: General, enhancement, P4)

Description

[Mike Conley (:mconley) (:⚙️)]

I suspect part of our start-up overhead is caused by a hack I introduced in bug 1261842, where when a new browser window opens, we wait on DOMContentLoaded, and swap out the initial non-remote browser for a remote one if gMultiprocessBrowser is true.

That hack was originally introduced to mitigate a tpaint regression where the DestroyWindow runnable for the initial <xul:browser> would delay the parent from being able to answer messages from the content process.

If I remember correctly, it's not as simple as just putting remote="true" on that initial browser, but I can't remember the complication. We should revisit this and find out.

Comment 1

[Mike Conley (:mconley) (:⚙️)]

At least part of the problem appears to be that if a <browser> is remote by default when the XUL document is parsed, when the XBL constructor is called, the TabParent doesn't appear to be available yet, so we fail out when we try to get at the loadContext here:

http://searchfox.org/mozilla-central/rev/214345204f1e7d97abb571b7992b6deedb5ff98f/toolkit/content/widgets/remote-browser.xml#376-380

Comment 2

[Mike Conley (:mconley) (:⚙️)]

According to this profile: https://perf-html.io/public/75cea1de7333eb66c69f39778d9c21d21e3c2b67/calltree/?callTreeFilters=prefix-012345KryWmyWnyWoFQuyL6EI1yL8yL9yLaAYrEN1EDczAjzAkzAlzAmzAnEArzO0zO1zO2zOezOfzOgADvxGgxGhAB2AB3AB4AOkAOlAOmAOnAOoAOpDY2EVcAOsDSnENrAOvzB9OrxWnJkAP0AP1&thread=0

on my high-powered MBP, the updateBrowserRemoteness call on the initial window costs about 57ms.

Comment 3

[Mike Conley (:mconley) (:⚙️)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=28bb0c49a2ba3fc89298abe7cd4e9c2caafa296d

Comment 4

[Mike Conley (:mconley) (:⚙️)]

Attachment: Bug 1357242 - Make initial browser tab remote by default instead of flipping it in DOMContentLoaded.

Review commit: https://reviewboard.mozilla.org/r/141930/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/141930/

Comment 5

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8870502 [details]
Bug 1357242 - Make initial browser tab remote by default instead of flipping it in DOMContentLoaded.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/141930/diff/1-2/

Comment 6

[Mike Conley (:mconley) (:⚙️)]

I've got this working in the e10s case. Unsurprisingly, it's blowing up the non-e10s case.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=1b833aafc8953c623a39ae3e785d364d5d72f58c

Comment 7

[Mike Conley (:mconley) (:⚙️)]

Preliminary results suggest that this gives a nice win on the tpaint talos benchmark. I don't think it moves the ts_paint one much.

Comment 8

[Mike Conley (:mconley) (:⚙️)]

I'll do a comparison push to try for more certainty on that last comment. If it turns out that this doesn't affect ts_paint, we can probably remove this as blocking photon-startup.

Comment 9

[Mike Conley (:mconley) (:⚙️)]

Baseline: https://treeherder.mozilla.org/#/jobs?repo=try&revision=acfeb20706c125c1c207f3d5d994a504405a56d1
Patch: https://treeherder.mozilla.org/#/jobs?repo=try&revision=a13194cc40f8b7952199b78b847baa24bff2842b

Comparison: https://treeherder.mozilla.org/perf.html#/compare?originalProject=try&originalRevision=acfeb20706c125c1c207f3d5d994a504405a56d1&newProject=try&newRevision=a13194cc40f8b7952199b78b847baa24bff2842b&framework=1&showOnlyImportant=0

Comment 10

[Mike Conley (:mconley) (:⚙️)]

The comparison shows a nice win on Linux64 and Windows for tpaint (window opening), but not for ts_paint (initial window opening). In fact, I see a small loss for Linux64 on that particular test with this patch.

I don't think this belongs under photon-startup anymore. We probably want to do this at some point, but I don't think it's going to help our start-up time right just yet.

Comment 11

[Florian Quèze [:florian]]

The non-remote browser constructor is more expensive than I would like; it triggers an about:blank load (bug 1446957). I think this bug still makes a lot of sense, but I would probably wait until bug 1446161 is fixed so that creating a remote browser doesn't block the main thread for the duration of the content process creation.

Comment 12

[:Gijs (he/him)]

We've done this work in bug 1506608 and friends, and although it's not in markup, it is now remote as soon as it gets inserted into the DOM, so gonna mark this WFM.