908987 - NotifyDidPaintForSubtree is slow when there are lots of open browsing contexts

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

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

The method seems to go through each descendant document and then each child
traverses up to root prescontext (at least if mFireAfterPaintEvents is false).

Do we really need to go through also all the background tabs in case the method
is called on root presshell?

(Saw NotifyDidPaintForSubtree in profiles when looking at Bug 854100 in a browser which
had lots of tabs open)

Comment 1

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

I think bug 1078005 fixed the actual firing of after-paint events, but there's still a lot of time spent in NotifyDidPaintForSubtree, mostly in nsFrameList::FrameAt called from nsIFrame::IsVisibleConsideringAncestors.

Comment 2

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

That call stack is actually (with the tail call optimization removed):

nsFrameList::FrameAt
nsDeckFrame::GetSelectedBox
nsIFrame::IsVisibleConsideringAncestors
PresShell::IsVisible
nsPresContext::NotifyDidPaintForSubtree

Comment 3

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

... I suspect that may be fixable by making PresShell::IsVisible use mIsActive more aggressively, which would be part of the way towards fixing bug 465216.

Comment 4

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

(Note that the nsFrameList::FrameAt calls make the algorithm O(N^2), since we're searching the deck frame's child list for each child in turn.)

Comment 5

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

https://hg.mozilla.org/try/pushloghtml?changeset=7e9ef8eb3933
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=7e9ef8eb3933
https://tbpl.mozilla.org/?tree=Try&rev=7e9ef8eb3933

Comment 6

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

And a try run on top of a good revision, this time:

https://hg.mozilla.org/try/pushloghtml?changeset=dfa8e027d0d7
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=dfa8e027d0d7
https://tbpl.mozilla.org/?tree=Try&rev=dfa8e027d0d7

Comment 7

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

(Note that in some sense this patch is rewriting bug 1096076 more aggressively.)

Comment 8

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

Attachment: Honor mIsActive more aggressively to avoid calling IsVisibleConsideringAncestors

This means that we avoid an O(N^2) calling pattern of IsVisible when we
call IsVisible for all tabs of a tabbrowser, since in a tabbrowser
(which uses an nsDeckFrame), IsVisibleConsideringAncestors is O(N) in
number of earlier tabs.

Comment 9

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

https://hg.mozilla.org/try/pushloghtml?changeset=83d5de58d42a
https://treeherder.mozilla.org/#/jobs?repo=try&revision=83d5de58d42a
https://tbpl.mozilla.org/?tree=Try&rev=83d5de58d42a

Comment 10

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/7d3ef4f3fa8f

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/7d3ef4f3fa8f