Extremely bad PresShell::Paint performance on http://a816-dohbesp.nyc.gov/IndicatorPublic/dognames/

RESOLVED FIXED in Firefox 55

Status

()

Core
Panning and Zooming
P2
normal
RESOLVED FIXED
2 months ago
a month ago

People

(Reporter: mconley, Assigned: botond)

Tracking

({perf})

55 Branch
mozilla55
Points:
---

Firefox Tracking Flags

(firefox55 fixed)

Details

(Whiteboard: [qf][gfx-noted], URL)

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Reporter)

Description

2 months ago
STR (warning - will kill main thread):

1. Visit http://a816-dohbesp.nyc.gov/IndicatorPublic/dognames/


This absolutely kills the main thread. Here's a profile I captured:

https://perfht.ml/2qOkBNl

It seems like we're spending a lot of time in PresShell::Paint. Like, multiple seconds.

Hey jrmuizel, anything useful in here? Is this a known bug?
Flags: needinfo?(jmuizelaar)
Nice. AccumulateEventRegions is the culprit. We should investigate why the event region is so large.
Component: Graphics → Panning and Zooming
Flags: needinfo?(jmuizelaar)
Flags: needinfo?(botond)
(Assignee)

Comment 2

a month ago
Yikes. We're building a maybe-hit region with 1.26 million rectangles! We probably shouldn't be doing that :)
Flags: needinfo?(botond)
(Assignee)

Comment 3

a month ago
This is very similar to bug 1333195. In that testcase, the maybe-hit region "only" has about 190,000 rects.
See Also: → bug 1333195
(Assignee)

Comment 4

a month ago
We previously added a |mMaybeHitRegion.SimplifyOutward(8)| call to AccumulateEventRegions() in bug 1200158, but then removed it in bug 1201548.

I don't see a correctness reason for removing it in bug 1201548, we just no longer needed it to address a different performance problem, so I think it should be safe to put back.
Comment hidden (mozreview-request)
(Assignee)

Comment 6

a month ago
(I also invesigated using TiledRegion instead, but I didn't see any performance wins compared to SimplifyOutward, and it's more complicated code-wise, so we're going to stick with the SimplifyOutward approach.)
(Assignee)

Updated

a month ago
Assignee: nobody → botond
(Assignee)

Updated

a month ago
Duplicate of this bug: 1333195
status-firefox55: --- → affected
Priority: -- → P2
Whiteboard: [qf] → [qf][gfx-noted]
Version: unspecified → 55 Branch
Keywords: perf

Comment 8

a month ago
mozreview-review
Comment on attachment 8867483 [details]
Bug 1363423 - Prevent the maybe-hit region from growing too large (in terms of number of rectangles) in PaintedLayerData::AccumulateEventRegions().

https://reviewboard.mozilla.org/r/139022/#review143656
Attachment #8867483 - Flags: review?(jmuizelaar) → review+

Comment 9

a month ago
Pushed by bballo@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c5ac37367e2e
Prevent the maybe-hit region from growing too large (in terms of number of rectangles) in PaintedLayerData::AccumulateEventRegions(). r=jrmuizel

Comment 10

a month ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/c5ac37367e2e
Status: NEW → RESOLVED
Last Resolved: a month ago
status-firefox55: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.