1394699 - stylo: Stylist is too big

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect, P3)

Description

[Nicholas Nethercote [inactive]]

The Stylist struct -- not the things hanging off it, just the struct itself -- is 34,296 bytes. Here's a breakdown of the major pieces:

> - Stylist 34000
>   - 3x CascadeData 10736
>     - PerPseudoElementMap<SelectorMap<Rule>> 9088
>       - 71x Option<SelectorMap<Rule>> 128
>     - 2x NonCountingBloomFilter 512
>     - InvalidationMap 288
>     - SelectorMap<Rule> 128
>     - SelectorMap<RevalidationSelectorAndHashes> 128

Changing PerPseudoElementMap to use Box<Option<T>> instead of Option<T> would help, assuming that the 71 elements are mostly Nothing.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Why do we have PerPseudoElementMaps for all the anon boxes?  That seems pretty fishy to me.

Gecko only has 25 pseudo-elements, not the 71 that stylo thinks it has.

That said, even with only 25 we'd still be at close to 10K for the stylist...  So Option<Box<T>> might still make sense.

Comment 2

[Bobby Holley (:bholley)]

(In reply to Nicholas Nethercote [:njn] from comment #0)
> Changing PerPseudoElementMap to use Box<Option<T>> instead of Option<T>
> would help, assuming that the 71 elements are mostly Nothing.

I think you mean Option<Box<T>>. Box<Option<T>> would be very inefficient.

Comment 3

[Emilio Cobos Álvarez [:emilio]]

I can take this one, should be easy to fix while waiting for release builds to finish.

Comment 4

[Joel Maher ( :jmaher ) (UTC -8) (PTO back normal Nov 17)]

the PR https://github.com/servo/servo/pull/18293, has a awsy improvement:
== Change summary for alert #9093 (as of August 29 2017 11:54 UTC) ==

Improvements:

  2%  Heap Unclassified summary windows10-64-stylo opt stylo     64,827,500.84 -> 63,286,282.79

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=9093

Comment 5

[Nicholas Nethercote [inactive]]

>   2%  Heap Unclassified summary windows10-64-stylo opt stylo    
> 64,827,500.84 -> 63,286,282.79

~1.5 MiB! That's a lot more than I expected.

Comment 6

[Bobby Holley (:bholley)]

(In reply to Nicholas Nethercote [:njn] from comment #5)
> >   2%  Heap Unclassified summary windows10-64-stylo opt stylo    
> > 64,827,500.84 -> 63,286,282.79
> 
> ~1.5 MiB! That's a lot more than I expected.

Assuming we got about 27k per stylist of savings out of this, that would imply that we have ~55 presshells floating around at the time when AWSY does its measurements. Does that seem accurate?

Anyway, the big win here is landed.

https://hg.mozilla.org/integration/autoland/rev/0e5471fdbb8c

Comment 7

[Boris Zbarsky [:bzbarsky]]

> Does that seem accurate?

Per https://areweslimyet.com/faq.htm awsy loads the tp5 pageset 5 times, round-robin allocating the loads to 30 tabs.  So ignoring leaks and other buildup of stuff it basically corresponds to loading the last 30 pages of the tp5 pageset into tabs, one per tab.

Having 55 live presshells at that point does not seem unreasonable at all: that's <2 per tab, which is actually on the low side of what I would have expected, given ad frames and whatnot.  Chances are we actually have more presshells than that and the savings was a bit less than 27K, since we do in fact have some pseudo-element styles in our UA sheet, as well as anon box styles, etc.

Comment 8

[Bobby Holley (:bholley)]

Makes sense - thanks!