Closed Bug 1469403 Opened Last year Closed 10 months ago

We dump display lists from about:newtab a bunch when it's not even visible

Categories

(Core :: Web Painting, defect, P2)

57 Branch
defect

Tracking

()

RESOLVED FIXED
mozilla65
Tracking Status
firefox65 --- fixed

People

(Reporter: kats, Assigned: kats)

Details

(Whiteboard: [qf:p1:f64])

Attachments

(2 files, 1 obsolete file)

Start Firefox with a clean profile and do this:
- in about:preferences, check the "Restore previous session" box
- in about:config, set "layout.display-list.dump-content" to true
- close all tabs except for one, and leave that one tab at about:blank

Then shut down firefox. Restart it, recording stdout to a file:

./mach run | tee output.log

As soon as it's open and the about:blank tab is done loading, close it again. Now look at what display lists got painted by examining the logfile.

In my case, of the 16 paints [1] that happened, 12 of them [2] were for about:home, even though about:home was not visible to the user at any point.

This seems like a huge waste. We seem to also repaint about:home every time you alt-tab to and from Firefox.

[1] grep "Painting --- before optimization" output.log | wc -l
[2] grep "FixedPosition" output.log | wc -l (divide the result by 2, because FixedPosition appears in both the "before optimization" and "after optimization" display lists). I used FixedPosition because it appears once on about:home and zero times on about:blank, but there are other things you can search for as well.
Summary: We paint about:home way a bunch when it's not even visible → We paint about:home a bunch when it's not even visible
Sorry, you also need to capture stderr, that's where most of the output ends up.

./mach run 2>&1 | tee output.log
Whiteboard: [qf] → [qf][fxperf]
Whiteboard: [qf][fxperf] → [qf:p1:f64][fxperf]
Moving to Core, but FF:ActivityStream could also be a fit if this is AS-specific.
Product: Firefox → Core
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #0)
> In my case, of the 16 paints [1] that happened, 12 of them [2] were for
> about:home, even though about:home was not visible to the user at any point.
> 
> This seems like a huge waste. We seem to also repaint about:home every time
> you alt-tab to and from Firefox.

How do we go about figuring out what's causing these paints (honest, naive question)?
Flags: needinfo?(bugmail)
Also, and possibly related, can we write a test for this?
Hmm, I can't reproduce this any more. I tried both on latest nightly and the June 17 nightly using my STR from comment 0 and I don't see the same behaviour. I don't know what happened :(

Let's close this for now, I'll reopen if I figure it out.
Status: NEW → RESOLVED
Closed: Last year
Flags: needinfo?(bugmail)
Resolution: --- → INVALID
Ok I'm seeing this again, on a Linux build from recent m-c. Slightly different STR in that I have two tabs open: one is about:config and the other is https://bug1471578.bmoattachments.org/attachment.cgi?id=8988147 (but a local copy, if that matters). When I start firefox with display-list.dump-content enabled I see the about:home display list show up a bunch of times. I traced it a bit and I think what's happening is that the tab gets a RenderLayers call, and that makes the PuppetWidget "visible" at [1]. This sets the PuppetWidget::mVisible flag to true and that makes the NeedsPaint() function start returning true, and from then onwards at [2] we start going into the code to paint the thing instead of just bouncing out. I don't know what the expected codepaths are for avoiding painting a background tab, but it seems fishy to me that the PuppetWidget thinks its visible when it's clearly not. I'm guessing this is some optimization to pre-paint the layers (tab warming?) that's going awry.

I verified that we're painting the presShell even though mPaintingSuppressed is true on it. I didn't find a codepath that actually aborts painting based on this flag, but it seems wrong that we are painting a suppressed presShell.

/cc mconley who presumably knows more about the RenderLayers stuff, I think that's a thing invoked by the frontend. I'm going to move this bug into Web Painting because it seems like the most appropriate Core component, although this might need to be moved back to the Firefox product.

[1] https://searchfox.org/mozilla-central/rev/14cb8f1c238735ba1abe18ad44e39808983c2572/dom/ipc/TabChild.cpp#2632
[2] https://searchfox.org/mozilla-central/rev/14cb8f1c238735ba1abe18ad44e39808983c2572/view/nsViewManager.cpp#444
Status: RESOLVED → REOPENED
Component: General → Layout: Web Painting
Resolution: INVALID → ---
I notice this too, can be annoying when trying to look at display list dump, and switching focus to the terminal prints a whole new display list for hidden content.

It seems like it might be intentional to load and paint about:home in the background so that opening a new tab becomes much quicker, though I'll wait for mconley to comment on that.

There's also a platform bug here, that we run a paint cycle for all visible tabs whenever we change window focus. That's because some native themed content (usually the background of the tabstrip!) changes it's rendering depending on if the window is focused is not.

Whenever the window state changes, we schedule paints (which would include for actually hidden, but pretending to be visible documents). That paint will build a full display list (no RDL, since the schedule paint marks the root as being changed), build layers, and then run DLBI. Generally for a content process tab, no actual invalidations would happen, since there's no themed content that uses the window state, and then we'd be done. A lot of work for no gain in the tabs.

Ideally we'd do 'normal' invalidation here, where all frames/elements that paint relevant themed content would register as listeners for the content state change event, and on changes we'd mark the actual affected frames as changed. That'd quickly be a no-op in the content process usually. It'd also mean would could do RDL partial rebuilding for when we do have affected frames.

An easier workaround might just be to flag the whole document as having at least one instance of this themed content when applicable (based on the previous display list build), and only schedule a paint if it's set.
Priority: -- → P2
So I worked a bit with kats and IRC, and we've determined that what you're seeing is painting for the preloaded about:newtab page.

mattwoodrow's instincts are spot on: the preloaded about:newtab page is a perceived performance optimization to make opening new tabs feel faster. That preloading occurs the first time a tab finishes opening, per window. That explains why you're seeing it after your session restores - restoring your session with more than 1 tab caused the preloading to kick in.

The preloading loads the about:newtab document. Up until Firefox 61, that's all it did, but then in bug 1408026, we went a step further and had it render and upload its layers to the compositor. This was to avoid a white flash or delay when opening new tabs.

Note that the preloading can be disabled by setting browser.newtab.preload to false.

So I suspect this is a WONTFIX / WORKSFORME unless this is an unacceptable situation - although I'd want to hear alternatives on how we can make the newtab page appear seemingly-instantaneously.
Summary: We paint about:home a bunch when it's not even visible → We paint about:newtab a bunch when it's not even visible
(In reply to Mike Conley (:mconley) (:⚙️) (Catching up on needinfos / reviews) from comment #8)
> So I suspect this is a WONTFIX / WORKSFORME unless this is an unacceptable
> situation - although I'd want to hear alternatives on how we can make the
> newtab page appear seemingly-instantaneously.

Ok, that's fair.

Maybe to deal with the annoyance of display-list-dump pollution we can skip dumping the display list in cases where the presshell has suppressed painting. We can just output a single line instead ("display list suppressed for presShell with url <whatever>").
You can also detect paints from non-foreground tabs by checking to see if their DocShells are active. If not, that's a good signal that we're warming them.
Taking this out of frontend perf triage as it seems the issue isn't really fixable on the frontend side of things.
Whiteboard: [qf:p1:f64][fxperf] → [qf:p1:f64]
Based on comment 8/9 I'm going to close this as WONTFIX. We can file a followup to deal with the display list dumping pollution if needed.
Status: REOPENED → RESOLVED
Closed: Last yearLast year
Resolution: --- → WONTFIX
The display list pollution is still super annoying. I'm going to take this bug to skip the DL dumping for non-foreground tabs.
Assignee: nobody → kats
Status: RESOLVED → REOPENED
Resolution: WONTFIX → ---
Try push is at https://treeherder.mozilla.org/#/jobs?repo=try&group_state=expanded&revision=b310896517f0913d08d1f0268366df77eb55add3 although I accidentally based it on top of a local patch on top of m-c from a few days ago. But still it should catch any obvious bustage.
Attachment #9018369 - Attachment is obsolete: true
Pushed by kgupta@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d9e373554f51
Set a flag on the DL builder to indicate if the docshell is active. r=mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/859e7d50fc03
Skip DL dumps when the DL builder is from an inactive docshell. r=mattwoodrow
https://hg.mozilla.org/mozilla-central/rev/d9e373554f51
https://hg.mozilla.org/mozilla-central/rev/859e7d50fc03
Status: REOPENED → RESOLVED
Closed: Last year10 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla65

(Updating summary to reflect what was actually fixed here.)

Summary: We paint about:newtab a bunch when it's not even visible → We dump display lists from about:newtab a bunch when it's not even visible
You need to log in before you can comment on or make changes to this bug.