756313 - Don't load homepage URI before first paint

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[Vladan Djeric (:vladan)]

We often start loading the first URI before we've done the first paint, delaying any visual indicator that the browser is starting. As an example, below you can see the startup milestones from my last browser start:

  20            main
1856            createTopLevelWindow
2806            firstLoadURI
4025            delayedStartupStarted
4011            firstPaint
4060            sessionRestoreInitialized
4062            sessionRestoreRestoring
4604            delayedStartupFinished
6461            sessionRestored

It looks like loading the homepage ("about:home") might have delayed startup by a few seconds, although to be fair that gap could also have been caused by other operations.

Comment 1

[Jesse Ruderman]

On the other hand, firing network requests in parallel with UI startup can mean less total time waiting for the home page to be displayed.  This needs both measurement and consensus on the tradeoff.

It does seem silly if the home page is set to "about:home", though!

Comment 2

[Justin Dolske [:Dolske]]

(In reply to Vladan Djeric (:vladan) from comment #0)
> We often start loading the first URI before we've done the first paint,
> delaying any visual indicator that the browser is starting.

Is this the usual paint-suppression used to prevent FOUC in web pages? If it is, seems like that should only be enabled for content-driven loads. (Or, perhaps more simply, don't do that if the chrome window itself hasn't painted).

If it isn't, what's causing firstpaint to take so long? Is something actually explicitly delaying that paint? Or are we just busily thrashing around and just unable to get around to painting?

Comment 3

[(dormant account)]

(In reply to Justin Dolske [:Dolske] from comment #2)

> If it isn't, what's causing firstpaint to take so long? Is something
> actually explicitly delaying that paint? Or are we just busily thrashing
> around and just unable to get around to painting?

For example: about:home uses localstorage, which can be painfully slow, but our disk cache can also be slow to initialize...and yes, thrashing around keeps ui from popping up

Comment 4

[Marco Bonardo [:mak]]

fwiw, I think with some brief changes we may delay our localStorage usage in about:home, for example we may pass basic info to the page from the load handler in browser.js and "executeSoon" snippets loading (That would be the only remaining LS usage). I suggest trying to do this in bug 749477.

Comment 5

[(dormant account)]

This might affect WebRuntime, not sure if it's possible to set an app as a homepage in firefox.

Comment 6

[Lawrence Mandel [:lmandel] (use needinfo)]

k9o- In triage was discussed that general performance improvements not tied to a specific k9o use case will not block k9o.

Comment 7

[Dão Gottwald [:dao]]

Attachment: patch

I was getting a "leaked window property: top" failure following the first browser chrome test. I'm not sure why this is triggered by my changes, but the failure is bogus anyway since 'top' is a native window property similar to 'navigator' and 'constructor'.

Comment 8

[:Gavin Sharp [email: [e-mail]]]

This is probably going to drastically change all of our Ts numbers (which rely on a page loading to report the end time, IIRC). Can you run it through try to get an idea of how much? I'm not suggesting we avoid optimizing perceived responsiveness just because it affects the test, but it's worth being aware that comparing Ts numbers across this patch's landing will be more difficult.

Comment 9

[Neil Deakin]

Comment on attachment 677380 [details] [diff] [review]
patch

The code itself looks fine though.

Comment 10

[(dormant account)]

(In reply to :Gavin Sharp (use [e-mail] for email) from comment #8)
> This is probably going to drastically change all of our Ts numbers (which
> rely on a page loading to report the end time, IIRC). Can you run it through
> try to get an idea of how much? I'm not suggesting we avoid optimizing
> perceived responsiveness just because it affects the test, but it's worth
> being aware that comparing Ts numbers across this patch's landing will be
> more difficult.

We could merge this by itself on mc to isolate changes in ts.

Comment 11

[Dão Gottwald [:dao]]

(In reply to :Gavin Sharp (use [e-mail] for email) from comment #8)
> This is probably going to drastically change all of our Ts numbers (which
> rely on a page loading to report the end time, IIRC). Can you run it through
> try to get an idea of how much? I'm not suggesting we avoid optimizing
> perceived responsiveness just because it affects the test, but it's worth
> being aware that comparing Ts numbers across this patch's landing will be
> more difficult.

http://perf.snarkfest.net/compare-talos/index.html?oldRevs=c3cb054eaeae&newRev=aa1afdc1fe3d&tests=ts_paint&submit=true

Comment 12

[Dão Gottwald [:dao]]

https://hg.mozilla.org/mozilla-central/rev/e35f252ca573

Comment 13

[Ryan VanderMeulen [:RyanVM]]

This was backed out due to mochitest-other failures.
https://hg.mozilla.org/mozilla-central/rev/c5fbfc1b1a94

https://tbpl.mozilla.org/php/getParsedLog.php?id=16670913&tree=Firefox

2697 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/a11y/accessible/bounds/test_zoom.html | Test timed out.
2698 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/a11y/accessible/bounds/test_zoom.html | [SimpleTest.finish()] waitForFocus() was called a different number of times from the number of callbacks run.  Maybe the test terminated prematurely -- be sure to use SimpleTest.waitForExplicitFinish(). - got 1, expected 0
3591 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/a11y/accessible/events/test_docload.html | Different amount of expected children of [' no node info ', role: app root, name: 'Firefox', address: [xpconnect wrapped nsIAccessible]]. - got 3, expected 2
3598 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/a11y/accessible/events/test_docload.html | Different amount of expected children of [' no node info ', role: app root, name: 'Firefox', address: [xpconnect wrapped nsIAccessible]]. - got 3, expected 2
4136 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/a11y/accessible/events/test_focus_browserui.xul | Test timed out.
4137 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/a11y/accessible/events/test_focus_browserui.xul | [SimpleTest.finish()] waitForFocus() was called a different number of times from the number of callbacks run.  Maybe the test terminated prematurely -- be sure to use SimpleTest.waitForExplicitFinish(). - got 1, expected 0

Comment 14

[Dão Gottwald [:dao]]

It's fascinating how a11y tests messing with browser windows are a source of endless pain...

Comment 15

[Dão Gottwald [:dao]]

https://hg.mozilla.org/mozilla-central/rev/00c925c90f86

Comment 16

[:Gavin Sharp [email: [e-mail]]]

It's possible that this will result in perceived performance being worse, because this will result in more time between the window being drawn and the home page loading. It's not clear to me that that's better than letting the load delay drawing of the window, assuming the user's intent is to interact with the home page on startup.

Comment 17

[(dormant account)]

(In reply to :Gavin Sharp (use [e-mail] for email) from comment #16)
> It's possible that this will result in perceived performance being worse,
> because this will result in more time between the window being drawn and the
> home page loading. It's not clear to me that that's better than letting the
> load delay drawing of the window, assuming the user's intent is to interact
> with the home page on startup.

it's better. People perceive chrome startup to be a lot better than ours. They show the window faster yet they take longer to be able to render pages.

Comment 18

[(no longer active)]

Regression  Ts Paint, MAX Dirty Profile increase 2.48% on Linux x64 Firefox-Non-PGO
-------------------------------------------------------------------------------------
    Previous: avg 648.635 stddev 5.204 of 30 runs up to revision 556b9cfb269f
    New     : avg 664.716 stddev 0.891 of 5 runs since revision 00c925c90f86
    Change  : +16.081 (2.48% / z=3.090)
    Graph   : http://mzl.la/SFiFPe

Changeset range: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=556b9cfb269f&tochange=00c925c90f86

Changesets:
  * http://hg.mozilla.org/mozilla-central/rev/00c925c90f86
    : D?o Gottwald <[e-mail]> - Bug 756313 - Don't load the homepage before the first paint. r=enn
    : http://bugzilla.mozilla.org/show_bug.cgi?id=756313

Comment 19

[Dão Gottwald [:dao]]

(In reply to Ehsan Akhgari [:ehsan] from comment #18)
> Regression  Ts Paint, MAX Dirty Profile increase 2.48% on Linux x64

This was expected.