Open Bug 1422032 Opened 4 years ago Updated 3 years ago

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

Categories

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

enhancement

Tracking

()

People

(Reporter: jrmuizel, Unassigned)

References

(Blocks 2 open bugs)

Details

(Whiteboard: [wr-reserve])

Attachments

(1 file)

It would be nice if we could avoid this:
https://perfht.ml/2ApmBjH
Flags: needinfo?(ethlin)
Blocks: 1422039
Whiteboard: [wr-mvp] [triage]
Priority: -- → P2
Whiteboard: [wr-mvp] [triage] → [wr-reserve]
Assignee: nobody → ethlin
Flags: needinfo?(ethlin)
Status: NEW → ASSIGNED
Priority: P2 → P1
Whiteboard: [wr-reserve] → [wr-mvp]
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.
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 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.
Attachment #8933601 - Flags: review?(jmuizelaar)
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.
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?
Flags: needinfo?(matt.woodrow)
Whiteboard: [wr-mvp] → [wr-reserve]
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.
Flags: needinfo?(matt.woodrow)
Assignee: ethlin → nobody
Status: ASSIGNED → NEW
Priority: P1 → P3
Summary: We spend an appreciable amount of time in nsIFrame::ClearInvalidationStateBits → We spend an appreciable amount of time in nsIFrame::ClearInvalidationStateBits during facebook-refresh.thml
This isn't needed for the initial release.
Priority: P1 → P2
No longer blocks: stage-wr-nightly
You need to log in before you can comment on or make changes to this bug.