1423370 - WebRender is hurt sending scroll data on MotionMark bouncing circles

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, defect, P2)

Description

[Jeff Muizelaar [:jrmuizel]]

24% of display list building time is in IPC::ParamTraits<mozilla::layers::WebRenderScrollData>::Write.

The likely root cause of this is that each bouncing circle has a scroll frame.

Comment 1

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

It doesn't look like each bouncing circle has a scrollframe. I think what's happening is that there's an nsDisplayTransform item for each circle, and so we create a new WebRenderLayerScrollData for each one and it just adds up. The reason we need a new scrolldata for each transform is for the compositor hit-testing to work properly. But we can probably do a better job of squashing these things together, and I think with the new WR-based hit-testing we might not need to send this over the compositor at all.

Comment 2

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

Attachment: Bug 1423370 - Create fewer WebRenderLayerScrollData items for transformed items.

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

Comment 3

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

I made a patch that eliminates the spurious WebRenderLayerScrollData instances for when gfx.webrender.hit-test is enabled. I don't want to land it yet though, I feel like I need to spend some more time figuring out if there's any other architectural improvements we can make, and if there's some way to make it more robust against accidentally regressing things.

Comment 4

[Jeff Muizelaar [:jrmuizel]]

I think we planed on landing this.

Comment 5

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

Yeah, sorry - I looked at this again and was not convinced it was correct. I thought we were only using the transform on the APZ side with non-WR hit-testing, but then I looked again and we were using it in other places for scrollbar-related things, so I think it might affect correctness. We should still be able to optimize this use case, but we need to make sure we don't throw away all the transforms (in particular transforms that are on scrollframes). Leaving the needinfo on me for now.

Comment 6

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

Here's a try push with a version of the patch I'm more confident should be good: https://treeherder.mozilla.org/#/jobs?repo=try&revision=84566196378a0783b09a989e246f4a02f16b979f

Comment 7

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

Looks like there's an R6 failure, I'll have to look into it.

Comment 8

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

https://treeherder.mozilla.org/#/jobs?repo=try&revision=25718f5c9547dc6d0454af77a981e5e9676139cc fixes it for me locally, hopefully it doesn't break anything else...

Comment 9

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

Comment on attachment 8935460 [details]
Bug 1423370 - Create fewer WebRenderLayerScrollData items for transformed items.

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

Comment 10

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8935460 [details]
Bug 1423370 - Create fewer WebRenderLayerScrollData items for transformed items.

https://reviewboard.mozilla.org/r/206358/#review237204

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cf1fdb432d46
Create fewer WebRenderLayerScrollData items for transformed items. r=jrmuizel

Comment 12

[Eliza Balazs [:ebalazs_]]

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