1494124 - Large spike in GetWindowDraggingRegion hangs on Sept. 5th

Status: RESOLVED FIXED

(Core :: Web Painting, defect, P1)

Description

[Alex Thayer [:alexical] (she/her)]

This represents a substantial (~11%) increase in overall Main Thread hangs.

See here: https://arewesmoothyet.com/?category=all&durationSpec=128_512_test&mode=explore&payloadID=34f51bbb1d2d4e9badb9c8a40351af91&search=GetWindowDraggingRegion&thread=2

And the corresponding spike in the tracker view here: https://arewesmoothyet.com/?mode=track&trackedStat=All%20Hangs

(Check "Show seven-day average" or uncheck all the categories other than "paint")

Comment 1

[Mike Conley (:mconley) (:⚙️)]

Here's the changelog between Sept 4 and Sept 5, 2018:

https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=bade35bc346f3bc1f5e8b59d3d49c7a1ba28166c&tochange=c4c125ee2556a8d0e4252de29064bbdc130a0040

Comment 2

[Miko Mynttinen]

This is very likely caused by bug 1413546 that enabled retained display lists for parent process. Managing the window dragging region with retained display list is more complicated, because we need to be able to remove frames that contributed to the region in case the frames are modified or removed, instead of just keeping one region around[1].

This might be the culprit behind bug 1489184. Awesome catch Doug!

[1]: https://searchfox.org/mozilla-central/rev/881a3c5664ede5e08ee986d76433bc5c4b5680e6/layout/painting/nsDisplayList.cpp#2016

Comment 3

[Alex Thayer [:alexical] (she/her)]

Per Matt Woodrow in IRC: a likely culprit is mRetainedWindowDraggingRegion (+mRetainedWindowNoDraggingRegion?) accumulating values on each display list build without being cleared.

Comment 4

[Sylvestre Ledru [:Sylvestre]]

Which versions have/are been impacted?

Comment 5

[Miko Mynttinen]

(In reply to Sylvestre Ledru [:sylvestre] from comment #4)
> Which versions have/are been impacted?

This pref (layout.display-list.retain.chrome) is only enabled on 64.

Comment 6

[Miko Mynttinen]

Attachment: Bug 1494124 - Allow only one rect per frame in WeakFrameRegion

Comment 7

[Matt Woodrow (:mattwoodrow)]

Comment on attachment 9013348 [details]
Bug 1494124 - Allow only one rect per frame in WeakFrameRegion

Matt Woodrow (:mattwoodrow) has approved the revision.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/260007b95d67
Allow only one rect per frame in WeakFrameRegion r=mattwoodrow

Comment 9

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/260007b95d67