1276890 - HasAttributeDependentStyle called twice per attribute change

Status: RESOLVED WORKSFORME

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

Description

[Olli Pettay [:smaug][[e-mail]]]

About 20% of attribute add time is spent in HasAttributeDependentStyle.
That ends up being called both during AttributeWillChange and AttributeChanged phases.
Could we possibly optimize one of those out?

I thought there was an old bug open about this, but couldn't find it.

My silly microbenchmark is https://bugzilla.mozilla.org/attachment.cgi?id=8757475
(but DOM benchmarks out there tend to be microbenchmarks)

(I'll optimize some useless refcounting in bug 1276888)

jet, I was wondering if someone perhaps in tw team might want to get used to doing some optimizations for microbenchmarks? That requires usually both running test cases and looking at profiler data (I tend to use Zoom profiler on linux).

Comment 1

[Cameron McCormack (:heycam)]

In bug 1184842 comment 4 bz suggested that we don't always have all the information required at the same time to have a single AttributeWillAndDidChange notification, but if these are just edge cases we might be able to handle the common cases with a single HasAttributeDependentStyle change and still have two calls for these edge cases?

Comment 2

[Jet Villegas (inactive)]

In a post-Stylo world, is this still an issue?

Comment 3

[Cameron McCormack (:heycam)]

ServoRestyleManager still does respond to both AttributeWillChange (for taking the snapshot) and AttributeChanged (to note a restyle, and notify the frame/widget, which I think need the attribute value to habe changed).  It's possible we still see some overhead from calling both virtual functions, so it'd be worth profiling again.

Might be something wcpan would like to check?

Comment 4

[Wei-Cheng Pan [:wcpan] [:wcp] [:legnaleurc] (left Mozilla)]

https://perfht.ml/2Ddfs8D

I think we spent some time in Servo_StyleSet_MightHaveAttributeDependency, not sure whether we can squeeze more time from it.

Comment 5

[Cameron McCormack (:heycam)]

AttributeChanged doesn't turn up in the profile there, so I guess the original issue from comment 0 isn't a big deal with Stylo.  It is however a little surprising how much time we spend in Servo_StyleSet_MightHaveAttributeDependency.  wcpan, what did your test case look like?

Comment 6

[Wei-Cheng Pan [:wcpan] [:wcp] [:legnaleurc] (left Mozilla)]

I used Olly's testcase.

Comment 7

[Daniel Holbert [:dholbert]]

smaug, would you mind retesting to see if this is still an issue?

(Per comment 5 it sounds like things may have improved somewhat? Not sure if there's anything left to do here.)

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

Based on https://share.firefox.dev/3RtnIDY things looks quite reasonable.