1454529 - tune nglayout.initialpaint.delay setting

Status: RESOLVED DUPLICATE of bug 1536781

(GeckoView :: General, enhancement, P3)

Description

[Kevin Brosnan [Ex-Mozilla]]

Currently Firefox for Android uses 250ms as the value for nglayout.initialpaint.delay. The first option is to use the same settings as desktop 5ms. If we want to spend some time on this we could come up with a custom value that is better than desktop. see bug 1454474 comment 2 for some initial findings.

Comment 1

[Kevin Brosnan [Ex-Mozilla]]

bah was right the first time

Comment 2

[Kevin Brosnan [Ex-Mozilla]]

Attachment: bug 1454529 - set nglayout.initialpaint.delay to 5m globally. Removing the Android ifdef.

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

Comment 3

[Timothy Nikkel (:tnikkel)]

So we're basing this on the load of one site? (bug 1454474 comment 2) Last time we looked at this I believe the extra painting we had to do badly regressed page load speed. Do we have more evidence that this will be a win now?

Comment 4

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8968388 [details]
bug 1454529 - set nglayout.initialpaint.delay to 5m globally. Removing the Android ifdef.

https://reviewboard.mozilla.org/r/237084/#review243392

I don't think we can take this patch without having a lot of data to suggest that it will improve things. I spent a lot of time on paint suppression a few years ago because painting during page load can dramatically affect performance. If we paint earlier, we'll have more paints, and likely slower load times.

Comment 5

[Timothy Nikkel (:tnikkel)]

Comment on attachment 8968388 [details]
bug 1454529 - set nglayout.initialpaint.delay to 5m globally. Removing the Android ifdef.

https://reviewboard.mozilla.org/r/237084/#review243616

I'd be happy to enable trying some new things out in this area but I think we need more data right now to say that this will be a benefit.

Comment 6

[Chris Peterson [:cpeterson]]

We may want to run a mobile experiment testing different values of nglayout.initialpaint.delay. Desktop default is 5 ms and Android default is 250 ms.

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

I'm testing a patch where the whole ancient paint suppression is removed, but it is on top of a 
patch where painting is slower during page load (after first non-blank paint).

remote: View your change here:
remote:   https://hg.mozilla.org/try/rev/32400c88a4741e998aa913d43f70388120f846e9
remote: 
remote: Follow the progress of your build on Treeherder:
remote:   https://treeherder.mozilla.org/#/jobs?repo=try&revision=32400c88a4741e998aa913d43f70388120f846e9
remote: 
remote: It looks like this try push has talos jobs. Compare performance against a baseline revision:
remote:   https://treeherder.mozilla.org/perf.html#/comparechooser?newProject=try&newRevision=32400c88a4741e998aa913d43f70388120f846e9
remote: recorded changegroup in replication log in 0.027s

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

I've tried several different approaches to remove Paint Suppression altogether.
remote: Follow the progress of your build on Treeherder:
remote:   https://treeherder.mozilla.org/#/jobs?repo=try&revision=95fcfec03a1388563c44eba0e74e0c0c0d220a60
remote: recorded changegroup in replication log in 0.026s
is the latest variant and closest to pass test. But there is at least one weird wpt failure. That isn't an issue in paint suppression code itself, but something racy, as far as I see.

Comment 9

[Olli Pettay [:smaug][[e-mail]]]

Attachment: paint_suppress_runnable_for_toplevel.diff

tests + talos
https://treeherder.mozilla.org/#/jobs?repo=try&revision=9595757b552191b40520f81a5acc58436f70a50f

raptor
https://hg.mozilla.org/try/rev/6cbd4b694a75fa6c53f0e1a211d7f50e448a8bdb

Comment 10

[Olli Pettay [:smaug][[e-mail]]]

er, raptor https://treeherder.mozilla.org/#/jobs?repo=try&revision=6cbd4b694a75fa6c53f0e1a211d7f50e448a8bdb

Comment 11

[Olli Pettay [:smaug][[e-mail]]]

I think that because of bug 1524806, this doesn't actually need to be p1 anymore, but I'll investigate a bit more.