1368386 - Invalidating many elements is slow

Status: RESOLVED FIXED

(Core :: Web Painting, enhancement)

Description

[Markus Stange [:mstange]]

Attachment: dl-test-invalidate-everything.html

This testcase is an augmented version of https://people-mozilla.org/~jmuizelaar/implementation-tests/dl-test.html that invalidates all elements on every frame.

Profile: https://perfht.ml/2rbfPax

We spend a lot of time in ClientTiledPaintedLayer::InvalidateRegion, due to this line:

> mValidRegion.Sub(mValidRegion, mInvalidRegion.GetRegion());

Recomputing mValidRegion on every single call is wasteful. We should find a way to skip this step until mValidRegion is actually needed.

Comment 1

[Markus Stange [:mstange]]

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

Comment 2

[Markus Stange [:mstange]]

Attachment: Bug 1368386 - Make ProgressiveUpdate's aInvalidRegion parameter const to make it a bit easier to reason about.

The caller of RenderHighPrecision and RenderLowPrecision doesn't look at the
variable that it passes as the aInvalidRegion parameter after the call, so any
modifications that would have happened to this variable don't matter.

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

Comment 3

[Markus Stange [:mstange]]

Attachment: Bug 1368386 - Make ProgressiveUpdate's aValidRegion parameter const to make it a bit easier to reason about.

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

Comment 4

[Markus Stange [:mstange]]

Attachment: Bug 1368386 - Add PaintedLayer::InvalidateWholeLayer().

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

Comment 5

[Markus Stange [:mstange]]

Attachment: Bug 1368386 - Make PaintedLayer::mValidRegion private so that its access can be tightly controlled.

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

Comment 6

[Markus Stange [:mstange]]

Attachment: Bug 1368386 - Only call mInvalidRegion.GetRegion() when an up-to-date valid region is requested.

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

Comment 7

[Markus Stange [:mstange]]

Comment on attachment 8878233 [details]
Bug 1368386 - Only call mInvalidRegion.GetRegion() when an up-to-date valid region is requested.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/149580/diff/1-2/

Comment 8

[Matt Woodrow (:mattwoodrow)]

Comment on attachment 8878229 [details]
Bug 1368386 - Make ProgressiveUpdate's aInvalidRegion parameter const to make it a bit easier to reason about.

https://reviewboard.mozilla.org/r/149572/#review154250

Comment 9

[Markus Stange [:mstange]]

Comment on attachment 8878233 [details]
Bug 1368386 - Only call mInvalidRegion.GetRegion() when an up-to-date valid region is requested.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/149580/diff/2-3/

Comment 10

[Matt Woodrow (:mattwoodrow)]

Comment on attachment 8878230 [details]
Bug 1368386 - Make ProgressiveUpdate's aValidRegion parameter const to make it a bit easier to reason about.

https://reviewboard.mozilla.org/r/149574/#review154272

Comment 11

[Matt Woodrow (:mattwoodrow)]

Comment on attachment 8878231 [details]
Bug 1368386 - Add PaintedLayer::InvalidateWholeLayer().

https://reviewboard.mozilla.org/r/149576/#review154276

Comment 12

[Matt Woodrow (:mattwoodrow)]

Comment on attachment 8878232 [details]
Bug 1368386 - Make PaintedLayer::mValidRegion private so that its access can be tightly controlled.

https://reviewboard.mozilla.org/r/149578/#review154632

Comment 13

[Matt Woodrow (:mattwoodrow)]

Comment on attachment 8878233 [details]
Bug 1368386 - Only call mInvalidRegion.GetRegion() when an up-to-date valid region is requested.

https://reviewboard.mozilla.org/r/149580/#review154634

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/983ebee83aed
Make ProgressiveUpdate's aInvalidRegion parameter const to make it a bit easier to reason about. r=mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/02720536f017
Make ProgressiveUpdate's aValidRegion parameter const to make it a bit easier to reason about. r=mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/43cbca8c663e
Add PaintedLayer::InvalidateWholeLayer(). r=mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/11b6836cf3eb
Make PaintedLayer::mValidRegion private so that its access can be tightly controlled. r=mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/b4d533bb18fc
Only call mInvalidRegion.GetRegion() when an up-to-date valid region is requested. r=mattwoodrow

Comment 15

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

https://hg.mozilla.org/mozilla-central/rev/983ebee83aed
https://hg.mozilla.org/mozilla-central/rev/02720536f017
https://hg.mozilla.org/mozilla-central/rev/43cbca8c663e
https://hg.mozilla.org/mozilla-central/rev/11b6836cf3eb
https://hg.mozilla.org/mozilla-central/rev/b4d533bb18fc