Closed
Bug 1454529
Opened 7 years ago
Closed 6 years ago
tune nglayout.initialpaint.delay setting
Categories
(GeckoView :: General, enhancement, P3)
Tracking
(Performance Impact:high)
RESOLVED
DUPLICATE
of bug 1536781
Performance Impact | high |
People
(Reporter: kbrosnan, Assigned: smaug)
References
(Blocks 2 open bugs)
Details
(Keywords: perf:pageload)
Attachments
(2 files)
(deleted),
text/x-review-board-request
|
snorp
:
review-
|
Details |
(deleted),
patch
|
Details | Diff | Splinter Review |
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.
Reporter | ||
Updated•7 years ago
|
Reporter | ||
Comment 1•7 years ago
|
||
bah was right the first time
Comment hidden (mozreview-request) |
Reporter | ||
Updated•7 years ago
|
Attachment #8968388 -
Flags: review?(tnikkel)
Attachment #8968388 -
Flags: review?(snorp)
Comment 3•7 years ago
|
||
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?
Flags: needinfo?(kbrosnan)
Comment 4•7 years ago
|
||
mozreview-review |
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.
Attachment #8968388 -
Flags: review?(snorp) → review-
Comment 5•7 years ago
|
||
mozreview-review |
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.
Attachment #8968388 -
Flags: review?(tnikkel)
Updated•7 years ago
|
Assignee: nobody → kbrosnan
Priority: -- → P3
Comment 6•6 years ago
|
||
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.
Assignee | ||
Updated•6 years ago
|
Whiteboard: [qf] → [qf:investigate]
Assignee | ||
Comment 7•6 years ago
|
||
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
Assignee | ||
Comment 8•6 years ago
|
||
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.
Assignee | ||
Updated•6 years ago
|
Assignee: kbrosnan → bugs
Updated•6 years ago
|
Product: Firefox for Android → GeckoView
Assignee | ||
Comment 9•6 years ago
|
||
Assignee | ||
Comment 10•6 years ago
|
||
Updated•6 years ago
|
Whiteboard: [qf:investigate] → [qf:p1:pageload]
Assignee | ||
Comment 11•6 years ago
|
||
I think that because of bug 1524806, this doesn't actually need to be p1 anymore, but I'll investigate a bit more.
Assignee | ||
Updated•6 years ago
|
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → DUPLICATE
Reporter | ||
Updated•4 years ago
|
Flags: needinfo?(kbrosnan)
Updated•3 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•