1370604 - stylo: Consider preloading the style sharing cache with styled siblings during incremental restyle

Status: NEW

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

Description

[Bobby Holley (:bholley)]

During today's call, bz pointed out that the style sharing cache only works within a single traversal, so sharing will progressively decrease (and memory usage will progressively increase) over successive restyle generations.

An obvious solution to this is just to insert styled siblings into the cache when we determine that they don't need any other processing. The key point is that our reliance on flags set by rust-selectors to determine whether to put something in the cache is gone after [1]. There's still AFFECTED_BY_PSEUDO_ELEMENTS, but we can determine whether a style has pseudos by just examining the ElementStyes, and that check will go away anyway in bug 1369952. So this means that the cost of insertion is just the refcounting overhead of sticking the ComputedStyle in the cache.

We'd want to be careful to only insert elements with a different ComputedValues identity from anything already in-cache (so that we don't insert 50 entries for 50 siblings that all shared styles). And even then, we'd want to measure how this impacts traversal time over elements that don't need styling (i.e. what happens if we post a RESTYLE_SELF to some single leaf element deep in a tree with 10k elements).

[1] https://github.com/servo/servo/pull/17055

Comment 1

[Bobby Holley (:bholley)]

(In reply to Bobby Holley (:bholley) (busy with Stylo) from comment #0)
> We'd want to be careful to only insert elements with a different
> ComputedValues identity from anything already in-cache (so that we don't
> insert 50 entries for 50 siblings that all shared styles).

Oh, and we can implement this cheaply by storing a raw/opaque ComputedValues pointer (for comparison purposes only) in the style sharing cache alongside the element. This allows us to check against previous values without either borrowing their element data or storing strong ComputedValues pointers in the cache (either of which would cost an atomic operation).

Comment 2

[Bobby Holley (:bholley)]

Cameron did some measurements yesterday to determine whether we need this. One thing we discovered is that we really ought to share styles during recascade, which I've done in bug 1397976. Boris is going to measure on top of those patches and try to get a sense of how important this is.

Emilo's comments by email:
> I sincerely don't think that bug is a problem in practice. If the
> concern is that we may miss sharing that we otherwise got while, for
> example, hovering over tweets, that's basically true, but it only
> affects the root of the restyle.
>
> And since you're more likely to style the children in sequential mode,
> you may get more sharing for them.
>
> That being said, when the restyle is due to a class change on an
> ancestor or something like that, we restyle all the siblings that could
> change, so we get sharing for them.

>> Also, what does Blink do here?
> Blink removed sharing. WebKit looks at the DOM directly.

Comment 3

[Bobby Holley (:bholley)]

I'm going to assume that we're not doing this for 57. Let me know if your investigation turns up anything to the contrary.

Comment 4

[Chris Peterson [:cpeterson]]

status-firefox57=wontfix unless someone thinks this bug should block 57

Comment 5

[Boris Zbarsky [:bzbarsky]]

Not getting to this.