Closed Bug 1530511 Opened 5 years ago Closed 4 years ago

Categories

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

defect

Tracking

()

RESOLVED WONTFIX

People

(Reporter: bholley, Unassigned)

References

(Blocks 2 open bugs)

Details

Attachments

(1 file)

The page ends up with ~24k primitives, which is a ton. Picture caching seems to generally work, but sometimes the entire viewport gets invalidated, which might be related to the scrolling jank.

Not necessarily high priority, but an interesting testcase.

Here's a profile: https://perfht.ml/2T8B8Wf. We spend a lot of time display list building. Thoughts Miko?

Blocks: wr-perf
Flags: needinfo?(mikokm)
Priority: -- → P3

After a quick look at the code and the website, I am guessing that Gecko creates a frame tree for this site that includes inline frames with a lot of siblings, and InlineBackgroundData::GetContinuousRect() iterates through all those with O(n^2) complexity. This seems to be very old code added in bug 412093.

This seems quite painful to fix, since this function is used in quite a few places. Emilio: you worked with this code as part of borders, any ideas?

Flags: needinfo?(mikokm) → needinfo?(emilio)
See Also: → 1509286

So, quick question, is BiDi really enabled in that page? That seems the only way to make the function slow... Right?

Or are we hitting the slow path in SetFrame(..) where we throw away the cache because the inline has a different continuation to our previous frame?

https://searchfox.org/mozilla-central/rev/f1c7ba91fad60bfea184006f3728dd6ac48c8e56/layout/painting/nsCSSRendering.cpp#277

That seems a bit plausible, depending on in which order we build the display-list... Depending on which of the two problems we're hitting, it may be plausible to:

  • Somehow mark lines where bidi is reordering stuff to only process those specially.
  • Refactor that class so that the caching is a bit more explicit, and maintained as we traverse the frame tree.
  • Add a "stack" of frames to that class to handle nested inlines.

I'm not particularly familiar with bidi resolution, but it seems unlikely it affects that. Depending on the order we're traversing the frame tree in, 3 may be an easy win.

Flags: needinfo?(emilio)
Attached file continuation-log.txt

I added some logging for usage of the inline data cache.

Some observations:

  • No Bidi involved here.
  • The outermost inline has 3147 continuations, so recomputing it hurts.
  • We did not hit the cache once in the entire DL build.
  • We're calling GetBorderContinuousRect, which calls GetContinuousRect for both the current frame, and the first continuation (which also sets the cache back to that frame, unhelpful).
  • The inlines are nested 8 deep, and each time we recurse we clobber the cache.

Making GetBorderContinuousRect smarter, as well as having some sort of stack to prevent nested inline from clobbering results would make a massive difference.

I wonder how often the current cache helps in the general case...

The site doesn't seem to exist anymore.

Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: