Closed Bug 1368399 Opened 7 years ago Closed 7 years ago

stylo: We have no style sharing for inline styles


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

53 Branch





(Reporter: bzbarsky, Assigned: bholley)




(2 files)

In Gecko, these are explicitly coalesced on the DOM level just so we'll share the style side see bug 760331 for why we did that.  In particular, note bug 760331 comment 11, which notes that the change dropped made style context memory usage from 200+MB to 11KB (not a typo!), on a large changeset page.

Anyway, with stylo we share the _declaration_, but inline styles immediately disable ComputedValues sharing.

I don't see why we need to do that.  As long as two elements have the same exact declaration pointer for their inline styles, why can't we do style sharing for them?

(If this is sounding a lot like bug 1368229, that's because it is.)
> change dropped made style context memory usage

Just "change dropped style context memory usage", of course.
Agreed, we should do this once bug 1368229 lands.
Depends on: 1368229
Assignee: nobody → bobbyholley
This is what I was talking about in that other bug. :-)

MozReview-Commit-ID: 9m2ebknBFSE
Attachment #8872841 - Flags: review?(emilio+bugs)
MozReview-Commit-ID: Jmu7Pie2mBP
Attachment #8872842 - Flags: review?(emilio+bugs)
Priority: -- → P1
AFFECTED_BY_PRESENTATIONAL_HINTS is dead too and can also be removed, I think.
Longer terms we should try to think of a way to avoid the FFI call here to get the style attribute, btw, but it seems a bit complicated.
Comment on attachment 8872841 [details] [diff] [review]
Part 1 - Reduce the number of places where we need to enumerate ValidationData members. v1

Review of attachment 8872841 [details] [diff] [review]:

Not a fan of Default, but fine :)
Attachment #8872841 - Flags: review?(emilio+bugs) → review+
Comment on attachment 8872842 [details] [diff] [review]
Part 2 - Compare style attributes rather than rejecting them from the cache. v1

Review of attachment 8872842 [details] [diff] [review]:

r=me, thanks for doing this :)

::: servo/components/style/sharing/
@@ +52,5 @@
> +{
> +    match (target.style_attribute(), candidate.style_attribute()) {
> +        (None, None) => true,
> +        (Some(_), None) => false,
> +        (None, Some(_)) => false,

nit: You can merge the branches here if you want.
Attachment #8872842 - Flags: review?(emilio+bugs) → review+
(In reply to Boris Zbarsky [:bz] (if a patch has no decent message, automatic r-) from comment #5)
> AFFECTED_BY_PRESENTATIONAL_HINTS is dead too and can also be removed, I
> think.

It can't, because it's still used to communicate between stylist and sharing/ See

However, as soon as your id stuff lands, this information will no longer need to round-trip through rust-selectors, which will be awesome.
Closed: 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.