1422032 - We spend an appreciable amount of time in nsIFrame::ClearInvalidationStateBits during facebook-refresh.thml

Status: NEW

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

Description

[Jeff Muizelaar [:jrmuizel]]

It would be nice if we could avoid this:
https://perfht.ml/2ApmBjH

Comment 1

[Ethan Lin[:ethlin]]

I think we don't need to clear invalidation state bits for the whole frame tree. We just need to clear the bits for fallback items.

Comment 2

[Ethan Lin[:ethlin]]

Attachment: Bug 1422032 - Clear the invalidation state bits for fallback items only but not for all frames.

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

Comment 3

[Ethan Lin[:ethlin]]

I can't reproduce comment 1, but I think this way should be correct. Jeff, can you check if the patch helps the performance?

Comment 4

[Ethan Lin[:ethlin]]

Comment on attachment 8933601 [details]
Bug 1422032 - Clear the invalidation state bits for fallback items only but not for all frames.

Ah..we also check the flag in nsIFrame::InvalidateFrameWithRect. I should find another way to erase the flag efficiently.

Comment 5

[Ethan Lin[:ethlin]]

For WR, we only need to clear the bits for fallback items and all parent frames of the fallback items. So we need a special clear function for WR.

Comment 6

[Ethan Lin[:ethlin]]

I did some test and studies. It looks like we should clear the flag for either non-fallback and fallback frames since frame tree also checks this flag. So we shouldn't partially clear the flag. Matt, can you give some suggestions if we want to optimize this part?

Comment 7

[Jeff Muizelaar [:jrmuizel]]

We talked about this in Austin. In the facebook-refresh case we're setting a flag on the body element and we need to then propagate this down to all descendants. We could have, but don't currently have the infrastructure to avoid doing this.

The style change could propagate down into a blob image so we basically need to have this for blob image invalidation to work properly.

Because we set the flag on all of the descendants we end up having to traverse the whole frame tree to clear the flag.

Comment 8

[Jeff Muizelaar [:jrmuizel]]

This isn't needed for the initial release.