Open Bug 1373430 Opened 7 years ago Updated 2 years ago

stylo: Too much time spent in the post-traversal

Categories

(Core :: CSS Parsing and Computation, enhancement, P4)

enhancement

Tracking

()

Tracking Status
firefox57 --- wontfix

People

(Reporter: bholley, Unassigned)

References

(Depends on 1 open bug)

Details

See the profile in bug 1373416 comment 0. We're currently spending 35ms in the post-traversal on obama wikipedia, which is way too much. Manish's fusing work should help a lot here.
So I took a profile [1] where we spent 43ms in the post-traversal, and did a bit of a breakdown of where we're spending time [2]. The results are kind of all of the place, but a few things stand out: * We spend a lot of time dropping the ComputedValues. Bug 1367904 should help with that, though some of the overhead will just move into the servo traversal. We should remeasure at some point and continue thinking about arenas (bug 1347852). * We spend quite a lot of time computing style for text nodes, pseudo elements, and anonymous boxes. We should fix bug 1368290 and bug 1368291. * We're doing some dumb twiddling of the style contexts (SetStyleBits, EnsureSameStructsCached) that I think we can eliminate after bug 1367904 lands. Once those various dependencies, we should remeasure, and figure out how much other work we can move out of the post-traversal into the the parallel traversal. DidSetStyleContext looks pretty daunting to make threadsafe, but it's possible we can 80/20 it somehow. Or maybe the post-traversal overhead will be small enough by that point that it won't matter. [1] https://perfht.ml/2t8qaEA [2] https://docs.google.com/spreadsheets/d/1T06-Z99R2-mEaN7d9LF3BuJci1zIAs1d0Cor-bh56Vw/edit?usp=sharing
Depends on: 1347852
Depends on: 1368290, 1368291
Depends on: 1373512
Depends on: 1376655
We think we can get rid of a lot of the overhead by hooking up a fast-path into the allocator that doesn't do the dumb stuff to support osx (checking whether pointers were really allocated by jemalloc, and zeroing things that shouldn't need to be zeroed) - glandium is going to take a look at that stuff.
Depends on: 1356701
Overhead is down, but still significant. I see about 25ms on obama now (with about 15ms doing the actual styling). Hopefully the text/anonymous box sharing will help a lot, will remeasure once that's fixed. Marking p3 because we don't have a specific target, we'll need to re-evaluate once the dependencies land and see how much more we need to do.
Priority: -- → P3
Priority: P3 → P4

Is this still an issue?

Flags: needinfo?(cam)

I do see the post-traversal still show up in profiles, so I think it's worth doing some measurement again and seeing what work we can move out of e.g. DidSetStyleContext. (One thought I had a few weeks ago is that we should be able to get rid of DidSetStyleContext calls for text frames, since we generally should handle all of the work it could do on the parent element.)

Flags: needinfo?(cam)

I think another significant chunk of the post-traversal I see often in profiles is just freeing styles. It may be worth seeing if we can change something in our allocator setup to mitigate that.

Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.