1371508 - Lots of time spent maintaining nsDisplayLayerEventRegions on BBC coverage of the UK election

Status: RESOLVED FIXED

(Core :: Graphics, enhancement, P3)

Description

[Boris Zbarsky [:bzbarsky]]

I'm not sure whether the URL will still show anything useful tomorrow...

What I'm seeing going on there is refresh driver ticks thathave a bunch of time under nsLayoutUtils::PaintFrame.

About 1/5 of this code is in pixman_op, adding rects to regions under nsDisplayLayerEventRegions::AddFrame.  Another 1/5 is elsewhere under nsDisplayLayerEventRegions::AddFrame (including under pixman_op); there's malloc/free traffic in there, etc, etc.

I see nothing that would redce the complexity of the regions in question at first glance.  So if we're adding lots of rects to them, we get O(N^2) behavior, right?

Profile is at https://perf-html.io/public/7f7c7b793164acc14d1a73df1fcbd4894bdb2f9b/calltree/?thread=4 and the "Content (3 of 4)" process is the one I'm looking at.

Kats, it looks like you touched this code somewhat recently; do you have time to take a look?

Comment 1

[Kartikaya Gupta (email:[e-mail])]

(In reply to Boris Zbarsky [:bz] (if a patch has no decent message, automatic r-) from comment #0)
> I see nothing that would redce the complexity of the regions in question at
> first glance.  So if we're adding lots of rects to them, we get O(N^2)
> behavior, right?

Basically, yeah. Botond recently added a call to reduce the complexity of the "maybe-hit" regions at [1] but that might not be the one being hit on this page. Botond, do you have cycles to take a look at this one?

The other regions (mHitRegion, mDispatchToContentRegion, etc.) are subject to similar issues as mMaybeHitRegion. While we can simplify the mDispatchToContentRegion outwards without any problem, simplifying the mHitRegion is a little more tricky. For that case we'll have to make sure that we dump the extra area included into the mDispatchToContentRegion so that we don't end up with false-positive results on layer hit tests in the compositor.

[1] http://searchfox.org/mozilla-central/rev/a798ee4fc323f9387b7576dbed177859d29d09b7/layout/painting/nsDisplayList.cpp#4428

Comment 2

[Botond Ballo [:botond]]

I do an SVG map on that page with a bunch of small elements (once for each electoral district in the UK, it seems), and I'm seeing us build dispatch-to-content regions with ~1000 elements. I believe the DTC region, like the maybe-hit region, should be safe to simplify outward.

Comment 3

[Botond Ballo [:botond]]

Attachment: Bug 1371508 - Simplify the dispatch-to-content region in nsDisplayLayerEventRegions::AddFrame() and AddInactiveScrollPort() if it starts to get large.

Review commit: https://reviewboard.mozilla.org/r/149230/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/149230/

Comment 4

[Botond Ballo [:botond]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=04364291d845ff0e64a378088312299ad4066132

Comment 5

[Timothy Nikkel (:tnikkel)]

Comment on attachment 8877813 [details]
Bug 1371508 - Simplify the dispatch-to-content region in nsDisplayLayerEventRegions::AddFrame() and AddInactiveScrollPort() if it starts to get large.

https://reviewboard.mozilla.org/r/149230/#review154688

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cbddc717675d
Simplify the dispatch-to-content region in nsDisplayLayerEventRegions::AddFrame() and AddInactiveScrollPort() if it starts to get large. r=tnikkel

Comment 7

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/cbddc717675d

Comment 8

[Boris Zbarsky [:bzbarsky]]

That helps a good bit.  The time under nsDisplayLayerEventRegions::AddFrame is now 16% self time, where before it was only 1%.

We still spend a ton of CPU painting on this page, but I'm not seeing other quite-as-obvious-to-me smoking guns.