Last Comment Bug 756313 - Don't load homepage URI before first paint
: Don't load homepage URI before first paint
Status: RESOLVED FIXED
[Snappy:p1]
: perf
Product: Firefox
Classification: Client Software
Component: General (show other bugs)
: Trunk
: All All
: -- normal with 2 votes (vote)
: Firefox 19
Assigned To: Dão Gottwald [:dao]
:
:
Mentors:
Depends on: 808986 715402 810741
Blocks: 756330 808104 462807 806180
  Show dependency treegraph
 
Reported: 2012-05-17 16:53 PDT by Vladan Djeric (:vladan)
Modified: 2013-03-11 08:31 PDT (History)
21 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
-


Attachments
patch (7.28 KB, patch)
2012-11-01 06:32 PDT, Dão Gottwald [:dao]
enndeakin: review+
Details | Diff | Splinter Review

Description Vladan Djeric (:vladan) 2012-05-17 16:53:22 PDT
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 2012-05-18 05:22:26 PDT
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] 2012-05-22 18:43:17 PDT
(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) 2012-05-22 19:27:38 PDT
(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] 2012-05-23 02:41:17 PDT
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) 2012-05-23 14:12:18 PDT
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) 2012-05-30 13:35:18 PDT
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] 2012-11-01 06:32:59 PDT
Created attachment 677380 [details] [diff] [review]
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: gavin@gavinsharp.com] 2012-11-01 10:34:28 PDT
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 2012-11-01 12:20:02 PDT
Comment on attachment 677380 [details] [diff] [review]
patch

The code itself looks fine though.
Comment 10 (dormant account) 2012-11-01 13:00:31 PDT
(In reply to :Gavin Sharp (use gavin@gavinsharp.com 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] 2012-11-01 13:51:35 PDT
(In reply to :Gavin Sharp (use gavin@gavinsharp.com 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] 2012-11-01 14:02:07 PDT
https://hg.mozilla.org/mozilla-central/rev/e35f252ca573
Comment 13 Ryan VanderMeulen [:RyanVM] 2012-11-01 16:34:38 PDT
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] 2012-11-01 19:44:32 PDT
It's fascinating how a11y tests messing with browser windows are a source of endless pain...
Comment 15 Dão Gottwald [:dao] 2012-11-02 01:59:11 PDT
https://hg.mozilla.org/mozilla-central/rev/00c925c90f86
Comment 16 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-11-02 10:12:22 PDT
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) 2012-11-02 10:15:20 PDT
(In reply to :Gavin Sharp (use gavin@gavinsharp.com 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 :Ehsan Akhgari 2012-11-05 11:41:47 PST
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 <dao@mozilla.com> - 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] 2012-11-06 02:24:54 PST
(In reply to Ehsan Akhgari [:ehsan] from comment #18)
> Regression  Ts Paint, MAX Dirty Profile increase 2.48% on Linux x64

This was expected.

Note You need to log in before you can comment on or make changes to this bug.