1424041 - Large main thread rasterization times with OMTP

Status: RESOLVED WORKSFORME

(Core :: Graphics, defect, P3)

Description

[Jeff Muizelaar [:jrmuizel]]

Running https://jrmuizel.github.io/webrender/facebook-refresh.html on the reference hardware I get main thread rasterization times on the order of 30+ ms. It seems likely mostly lock contention.

https://perfht.ml/2nG1qov

Comment 1

[Bas Schouten (:bas.schouten)]

I wonder why we're getting so many, this is a good one though, reinforces my idea that we may want to keep a couple of PathBuilders handy. Can you get a profile which includes the PaintThread easily on that reference machines, a profile on my machine seems to look a little different than over there, I'd like to know what the paint thread is doing during this time.

Comment 2

[Jeff Muizelaar [:jrmuizel]]

w/ PaintThread https://perfht.ml/2k8svLW

Comment 3

[Bas Schouten (:bas.schouten)]

Alright, this is clipping silliness. We are using a lot of complex clips, the creating of the vertex buffers for the complex clip paths is contending with the creation of the patch builders themselves on the main thread. We may be able to fix the main thread work and prevent the contention (I will do this), but the large amount of clipping remains a concern, I'll see if there's something I can do there as well I suppose, but it will be very tricky.

Comment 4

[Bas Schouten (:bas.schouten)]

So although my ratios are a little different (probably because my paint thread is much less busy and I'm running at 60fps), I can reproduce this issue. Note that I instrumented this test case and it makes a lot of RoundedRectangle paths, over a thousand per second, so if it's running at 60fps that's over 16 rounded rectangles per frame. I'm going to try and see if D2D's dedicated API for creating rounded rectangles with all equal corners avoids contention. I will explore preventing path creation on the main thread in a more complete way as well.

Comment 5

[Bas Schouten (:bas.schouten)]

Attachment: Create PathBuilderD2D objects on the paint thread for use on the main thread

So, this page causes us to generate an insane amount of paths, even pre-allocating 100 per frame, frames still regularly ran out of paths. With this patch the locking overhead almost completely goes away, and rasterization takes a much more reasonable portion of the paint time.

David, this basically does what I discussed with you, I might make it a little smarter to keep the cache the size of the 'max of the last 5 frames' or something like that.

How do you feel about the code structure of how I added the cache and mark frame ends.. This seemed like the easiest way, but you may favor something prettier.

Jeff: There's a try build here, could you try it on the reference machine and see if the problem goes away for you, at least mostly.?

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

Comment 6

[Bas Schouten (:bas.schouten)]

Actually Jeff, try this one, I've added the suggesting I had above of using past frames to determine how much to cache, this makes the last tiny bits of this issue go away for me:

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

Comment 7

[Bas Schouten (:bas.schouten)]

Actually, there is another updated build, the older ones could crash on startup under certain circumstances:

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

Comment 8

[Jeff Muizelaar [:jrmuizel]]

It didn't seem to help much https://perfht.ml/2Ao41t3

Comment 9

[Bas Schouten (:bas.schouten)]

Fresh build at: https://treeherder.mozilla.org/#/jobs?repo=try&revision=3ebab25e96dfa213c2224b04318619d195b38273

Comment 10

[Bas Schouten (:bas.schouten)]

I went a little crazy this time, if this still has contention I'm baffled:

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

Comment 11

[David Anderson [:dvander] - inactive, e-mail if emergency]

Comment on attachment 8935628 [details] [diff] [review]
Create PathBuilderD2D objects on the paint thread for use on the main thread

I didn't have time to try this out, but the approach seems fine, and I think we'll need some kind of solution to make filter-intensive cases fast.

Comment 12

[Bas Schouten (:bas.schouten)]

No longer relevant with WR.