1078005 - Firefox window with lots of tabs becomes sluggish due to floods of afterpaint events

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I have a Firefox session with a window that has a large and gradually increasing number of tabs.  It's been getting more and more sluggish as the number of tabs increases.  I profiled, and a significant percentage of the time seems to be spent firing afterpaint events (e.g., 24.5% in DelayedFireDOMPaintEvent::Run, 6.1% in nsPresContext::NotifyDidPaintForSubtree).

For example, at this point I have enough tabs that loading any article on nytimes.com (or the homepage) pegs the CPU at 100% while the article is visible.

The problem mainly seems bad when the visible tab is doing any painting.

But it seems like the fundamental problem here is that for every paint in the visible tab, it looks like we're firing an afterpaint event at **every single tab**.

It's not clear to me how much of this is the fault of the Firefox frontend code, and how much of it is Gecko's fault, but it at least seem odd to me that we'd fire afterpaint events at non-visible tabs.

I haven't yet debugged this very closely, but I could certainly debug further if needed.

(While I admit I've been seeing this for a while, and have dealt with it by closing tabs, I think the performance here is basically unacceptable.)

Comment 1

[Tim Taubert [:ttaubert] (inactive)]

It seems that NotifyDidPaintForSubtree() is called on every PresContext in the subtree. I see MozAfterPaint events dispatched to every window when I only hover the tab-close button because the browser window's PresContext seems to forward that to its subtree. Is that the expected behavior?

Comment 2

[Tim Taubert [:ttaubert] (inactive)]

But even scrolling a content document dispatches MozAfterPaint events to every tab. Not sure why - do we paint something in the browser window like scrollbars here?

Comment 3

[Tim Taubert [:ttaubert] (inactive)]

Hmm this seems to be bug 829330?

Comment 4

[Tim Taubert [:ttaubert] (inactive)]

So should we really be sending all those MozAfterPaint events? nsPresContext::FireDOMPaintEvent() re-targets events from non-chrome documents to the parent target. That is totally fine as long as the unprivileged document itself was painted.

If however the paint notification trickled down from e.g. the browser window's document, even then FireDOMPaintEvent() re-targets and sends an event to the parent target. Should we in that case maybe only send events if (!mUndeliveredInvalidateRequestsBeforeLastPaint.IsEmpty())?

If that's a valid approach we should check that before even creating a ton of runnables I think.

Comment 5

[Matt Woodrow (:mattwoodrow)]

We can't skip sending MozAfterPaint events just because mUndeliveredInvalidateRequestsBeforeLastPaint is empty, because we don't collect invalid regions for subdocuments (excluding the root content one). That change would stop us sending MozAfterPaint to iframes.

We might be able to skip sending them to hidden pres shell that haven't explicitly requested an event (mFireAfterPaintEvents). I'll give that a try.

Comment 6

[Matt Woodrow (:mattwoodrow)]

Attachment: Skip MozAfterPaint for hidden PresShells

Does this help at all?

Approximately how many tabs do you have open for this to become an issue? My session has ~100, and I can't get more then 0.5% of the profile to be spent during firing MozAfterPaint events.

Comment 7

[Matt Woodrow (:mattwoodrow)]

Try push to avoid needing to build locally: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=9e2ebd742c3b

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

(In reply to Matt Woodrow (:mattwoodrow) from comment #6)
> Approximately how many tabs do you have open for this to become an issue? My
> session has ~100, and I can't get more then 0.5% of the profile to be spent
> during firing MozAfterPaint events.

The main window in my current session has 409 tabs.  Probably won't be able to test this until tomorrow.

Comment 9

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Sorry to take so long to get to this; the last 3 days of meetings were busier than I'd expected.

In any case:

The patch does help, in that the substantial parts of the profile that I filed this bug about have entirely disappeared - the 24.5% in DelayedFireDOMPaintEvent::Run is gone, as is the 6.1% in nsPresContext::NotifyDidPaintForSubtree.

Articles on nytimes.com are *still* pegging the CPU, though, but it's now predominantly painting.  I was hoping it would make more of a difference (i.e., that a bunch of the other time spent would be related to this bug as well), but that doesn't seem to be the case.

Comment 10

[Matt Woodrow (:mattwoodrow)]

(In reply to David Baron [:dbaron] (UTC-7, busy Oct 7-9) (needinfo? for questions) from comment #9)

> Articles on nytimes.com are *still* pegging the CPU, though, but it's now
> predominantly painting.  I was hoping it would make more of a difference
> (i.e., that a bunch of the other time spent would be related to this bug as
> well), but that doesn't seem to be the case.

If you get a profile for this (ideally gecko profiler), then I can take a look at why that is.

Comment 11

[Matt Woodrow (:mattwoodrow)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/0f92f54185d9

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/0f92f54185d9