862274 - copy style structs from old style contexts to new ones during style change handling

Status: RESOLVED FIXED

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

Description

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

In bug 780341 comment 38 I wrote:
> Another random thought that just occurred to me.  It's not quite a valid
> optimization (because of cross-struct dependencies), but it might be close
> enough that we could make it one:
> 
>   When we're replacing one style context with another, old->GetRuleNode() ==
> new->GetRuleNode(), and we want to compute a struct that's not cached on the
> rule node, then if the old->GetParent()->GetStyle##struct() ==
> new->GetParent()->GetStyleStruct(), just share the struct from old to new
> (and set a bit on old saying it no longer owns it).  This sharing could
> propagate down the tree.  Then we could benefit from pointer equality checks.

In bug 780341 comment 40 I wrote:
> (In reply to Boris Zbarsky (:bz) from bug 780341 comment 39)
> > > Another random thought that just occurred to me.
> > 
> > That's an interesting idea.  We could at least do that for a subset of
> > structs, perhaps?
> 
> The idea would basically be:
> 
>  * replace the bool &aCanStoreInRuleTree with a reference to a simple struct
> with a few booleans in it, and some helper methods, so we can simultaneously
> record both "can store in rule tree" and record the common cases of
> cross-struct dependence (within style context)
>  * on inherited structs, include in the struct a bitfield (encapsulated in a
> struct, maybe the same one as above, or maybe not) that stores those common
> cases of cross-struct dependence (maybe one bit for every struct, or maybe
> fewer so it can fit in a char)
>  * give the struct that encapsulates that bitfield a method that takes two
> style contexts and returns whether the struct can just be copied
>  * have DO_STRUCT_DIFFERENCE call a function that uses that method

I still need to think more about whether this is practical, but I want to give it its own bug.

Comment 1

[Kevin Grandon :kgrandon]

Would like to have, but I suppose this is not a requirement for 2.0.

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

heycam ended up having to do this as part of bug 931668, I think.

Comment 3

[Cameron McCormack (:heycam)]

Yes, bug 931668 covered this.