908626 - Cache some DisplayItem in reference frame to avoid rebuilding from scratch

Status: NEW

(Core :: Layout, defect, P3)

Description

[Kan-Ru Chen [:kanru] (UTC+9)]

Currently we rebuild the entire DisplayList every time before we redraw. In some benchmarks this contributed about 20% CPU time but the actual layout change is small.

See https://groups.google.com/d/msg/mozilla.dev.tech.layout/BL3BNQu4sSA/CMHWMEHp9x0J

Comment 1

[Thinker Li [:sinker]]

Attachment: WIP - keep nsDisplayItems in a cache and with a separated builder.

Comment 2

[Thinker Li [:sinker]]

Attachment: WIP - keep nsDisplayItems in a cache and with a separated builder, v2

|gDoDisplayCache = true| to enable cache, but destroy the cache every time.  Set |gDoDisplayCacheKeep = true| to enable cache and destroy the cache only for necessary.

Comment 3

[Thinker Li [:sinker]]

If |gDoDisplayCache = true| and |gDoDisplayCacheKeep = true|, it makes dangling pointer error for nsDisplayItems.

Comment 4

[Thinker Li [:sinker]]

Attachment: WIP - keep nsDisplayItems in a cache and with a separated builder, v3

Fix some minor bug!  The cache is already partially work.  But, only nsDisplayItems of frames in the dirty area of the cache building time will be created and be put into the cache.  That means frames out of the dirty area would never be showed, even they are in the dirty area later.

Comment 5

[Thinker Li [:sinker]]

Attachment: WIP - keep nsDisplayItems in a cache and with a separated builder, v4

Flatten |nsDisplayList|s before moving to a cache.  Without flattening lists here, the items in the lists would be flattened by |ComputeVisibilityForRoot()| later, destroying instances of |nsDisplayWrapList| and its sub-classes, and making the cache invalid.

Comment 6

[Thinker Li [:sinker]]

(In reply to Thinker Li [:sinker] from comment #5)
> Created attachment 807721 [details] [diff] [review]
> WIP - keep nsDisplayItems in a cache and with a separated builder, v4
This patch is rendering contents correctly, but crash for some unknown reason.

Comment 7

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I don't see how this works. Who calls ChildIsDirty?

How are you planning to address the problem that when scrolling, offsets in display items need to change, unless we change display items have their scrolling ancestor as their reference frame?

Comment 8

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Thanks for working on this, though.

Comment 9

[Thinker Li [:sinker]]

PresShell::FrameNeedsReflow() and some others will call ChildIsDirty() along the path from leaves to the root.  With ChildIsDirty() invalidating cache works good so far, but I will look it again later to make sure I don't miss anything.

The offsets are recomputed for every reusing for now, see |BeforeReuseCache|, but I would like to use scrolling ancestor as the reference frame as you mentioned.  It means to apply additional transforms for scrolling frames.  But, changing reference frame may break the code of checking dirty area, I will check it later.

Another possible improvement of recomputing offsets is just shift the offsets of items.  It should be more fast, but it must deal the transforms applied by frames.  I am not sure which one is better, so I will do some experiments and let numbers talk.

Comment 10

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to Thinker Li [:sinker] from comment #9)
> The offsets are recomputed for every reusing for now, see
> |BeforeReuseCache|,

OK, that's probably a good approach, but you need to be careful because in some cases we cache data in the display item that takes ToReferenceFrame() into account, e.g. nsDisplayBackgroundImage::mBounds. In some cases we can make those relative to mToReferenceFrame instead so they don't need to be adjusted, but in other cases we might need to override BeforeReuseCache to clear or recompute the data.

Comment 11

[Thinker Li [:sinker]]

Thanks for your reminding, I just fixed the issue of nsDisplayBackgroundImage::mBounds the day before yesterday.  There is some known issues of real pages like what you had mentioned, I am fixing these issues.

Comment 12

[Thinker Li [:sinker]]

Attachment: WIP - keep nsDisplayItems in a cache and with a separated builder, v5

Fix some bug, no more crash for reloading content.  I have tried it on unagi, |BuildDisplayList| drop to about 8% of CPU time of RefreshDriver.

Comment 13

[Thinker Li [:sinker]]

Forget the number of 8% mentioned above.  It quite inaccurate after some study.

Comment 14

[Thinker Li [:sinker]]

Attachment: 810406: WIP - keep nsDisplayItems in a cache and with a separated builder, v6

No more fix ToReferenceFrame() for items before reusing.  All items from the cache are under an nsDisplayTransform, to apply a transform that match the scrolling position.

Comment 15

[Sylvestre Ledru [:Sylvestre]]

Moving to p3 because no activity for at least 1 year(s).
See https://github.com/mozilla/bug-handling/blob/master/policy/triage-bugzilla.md#how-do-you-triage for more information