Closed Bug 1078005 Opened 5 years ago Closed 5 years ago

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

Categories

(Core :: Layout, defect, major)

x86_64
Linux
defect
Not set
major

Tracking

()

RESOLVED FIXED
mozilla36

People

(Reporter: dbaron, Assigned: mattwoodrow)

References

Details

(Keywords: perf)

Attachments

(1 file)

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.)
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?
Flags: needinfo?(roc)
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?
Hmm this seems to be bug 829330?
Flags: needinfo?(matt.woodrow)
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.
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.
Assignee: nobody → matt.woodrow
Flags: needinfo?(matt.woodrow)
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.
Flags: needinfo?(dbaron)
(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.
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.
Flags: needinfo?(dbaron)
Attachment #8501486 - Flags: review?(roc)
(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.
https://hg.mozilla.org/mozilla-central/rev/0f92f54185d9
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
You need to log in before you can comment on or make changes to this bug.