We currently handle a class="" change on an element by doing an eRestyle_Subtree of that element. In some cases, we have many descendants that won't change which selectors they match, but some that do. We could instead post a restyle eRestyle_Self more selectively to the descendants that might change. See bug 1136893 for a proof of concept patch and some comments.

> by doing an eRestyle_Subtree of that element Only if the selectors that match that class have a ' ' or '>' combinator after them, right? A rightmost selector matching the class should only trigger eRestyleSelf. Anyway, it sounds like you're proposing making precisely the case when we're followed by ' ' faster, right?

(In reply to Boris Zbarsky [:bz] from comment #1) > Only if the selectors that match that class have a ' ' or '>' combinator > after them, right? A rightmost selector matching the class should only > trigger eRestyleSelf. Yes! > Anyway, it sounds like you're proposing making precisely the case when we're > followed by ' ' faster, right? Yep. '>' should already be faster due to the bug 931668 work.

(In reply to Cameron McCormack (:heycam) from comment #2) > > Anyway, it sounds like you're proposing making precisely the case when we're > > followed by ' ' faster, right? > > Yep. '>' should already be faster due to the bug 931668 work. I was wrong, bug 931668 doesn't help with that at all. '>' is handled just like ' ' for attribute changes, AFAICT. We return eRestyle_Subtree for it. I think it would be good to make '>' changes faster too. I am not sure yet whether the solution in this bug will catch that case too.

So at the moment, whenever we call HasAttributeDependStyle and look up the rule cascade for the class that was added or removed, and we get a selector that has a ' ' or '>' combinator on the right, then we return eRestyle_Subtree. This means we have to traverse the whole subtree and re-run selector matching on all elements in it. This is bad for two reasons. First is that we can do selector matching on elements that have no chance of changing what they matched. For example with: <style> div.x p { opacity: 0.5; } </style> <div> <blockquote> <p> <span> <em> if we toggle the x class on the div, it's wasteful to restyle the div and the blockquote. Second is that we can't avail ourselves of the bug 931668 optimizations. Since the p changes its opacity, we ought to be able to avoid restyling its descendants. (With bug 931668, we still must restyle the span, but bug 1180120 should let us stop directly after the p.) So it would be good to turn that into an eRestyle_Self on the p. We don't know which specific descendants of the element with a modified class will need to be restyled, but we can at least limit it to elements that match the selector to the right of the last combinator, in this case, |p|. We can make HasAttributeDependentStyle be able to return a list of selectors to apply an eRestyle_Self restyle hint to and the element that roots the subtree in which to look for those selector-matching elements (the subtree root being the element that had the class change), and somehow track these appropriately in the RestyleTracker. Then, once we come to process restyles in RestyleTracker::DoProcessRestyles, we can do a pass over the subtrees that we recorded and do the equivalent of AddPendingRestyle(..., eRestyle_Self, NS_STYLE_HINT_NONE) on the elements that match. We might not want to call AddPendingRestyle exactly, since that does a lot of traversal up the tree looking for restyle root bits, which we won't want to do each time. There is a risk here that the traversal of the subtrees with this restyle-descendants-that-match-this-selector hint will take longer than the eRestyle_Subtree processing that we currently do. We should be able to ensure that we visit each subtree element at most once. I'm hoping that for big subtrees, where we are spending a lot of time running the eRestyle_Subtree processing, we end up needing to restyle only a small proportion of those descendants.

(In reply to Cameron McCormack (:heycam) from comment #4) > There is a risk here that the traversal of the subtrees with this > restyle-descendants-that-match-this-selector hint will take longer than > the eRestyle_Subtree processing that we currently do. We should be > able to ensure that we visit each subtree element at most once. I'm > hoping that for big subtrees, where we are spending a lot of time > running the eRestyle_Subtree processing, we end up needing to restyle > only a small proportion of those descendants. Also of note is that with this plan we will end up matching the selector to the rhs of the last combinator against the matching element twice: first to determine whether we should call AddPendingRestyle for it, and second when we get around to restyling it. Still, matching a single selector like that shouldn't be that costly.

I'm going to try this out. Boris/David, do you have any comments on this plan?

Adding a new eRestyle_SomeDescendants hint for this and storing the selector in the mPendingRestyles table entry might be a better way to handle this. Then instead of explicitly traversing the subtree to find the elements that match the selector and calling AddPendingRestyle for the matching elements just before we process restyles, we can instead handle it as part of RestyleManager's work as it goes down the tree.

To simplify things, we should limit the types of selectors that can be used with eRestyle_SomeDescendants. I'm thinking for the moment that if the right-most selector has any pseudo-class or pseudo-element, then we should just fall back to returning eRestyle_Subtree. Many of the pseudo-classes would be easy to test for without having to accumulate state in ElementRestyler, but let's think about relaxing the restrictions later. Also if there is a ~ or + combinator we should continue returning eRestyle_LaterSiblings.

> Still, matching a single selector like that shouldn't be that costly. As long as we just do a SelectorMatches without following combinators, should be pretty fast, yes. Even more so if we restrict what selectors can end up in here. In general this does sound at least worth trying, esp. if we then measure it on both what we think are best and worst cases. One thing to worry about: selectorsa are not refcounted. So if you just store references to them in some long-lived data structure you'll need to make sure you never examine those after the sheets for the document have changed, or any other operation that can cause selectors to be freed.

(In reply to Boris Zbarsky [:bz] from comment #9) > One thing to worry about: selectorsa are not refcounted. So if you just > store references to them in some long-lived data structure you'll need to > make sure you never examine those after the sheets for the document have > changed, or any other operation that can cause selectors to be freed. To avoid making them refcounted or otherwise long lived, maybe we can just set a boolean on the RestyleManager if we clear rule cascades. If the boolean is set, then when restyling we don't look at the selectors and just treat eRestyle_SomeDescendants as eRestyle_Subtree.

I have some WIP patches, and these are the timings I'm getting on the bug 1172239 restyle testcase (attachment 8629189 [details]) on my machine: no patches applied: 2870ms bug 1180118 patches applied: 2226ms " plus a hack to mimic bug 1184842's effect: 1803ms I still haven't addressed the issue mentioned in bug 1136893 comment 12 where we're spending time looking upwards for a restyle root under AddPendingRestyle, so I reckon I can make more savings.

(In reply to Cameron McCormack (:heycam) from comment #11) > I still haven't addressed the issue mentioned in bug 1136893 comment 12 > where we're spending time looking upwards for a restyle root under > AddPendingRestyle, so I reckon I can make more savings. Doing that didn't help any.

A few test failures: https://treeherder.mozilla.org/#/jobs?repo=try&revision=fd3f0b08b200

Tests passing (apart from test_additional_sheets.html timeouts, which is just due to the expensive style context destructor assertions being turned on): https://treeherder.mozilla.org/#/jobs?repo=try&revision=9be1d12d87c2

Not sure about the name "SomeDescendants". Let me know if you think of something better.

Here's a simple test to show how the performance changes in a best case and worst case scenario. The test creates a binary tree of span elements 16 levels deep. In the best case, the style sheet has a rule that matches a single span element at level 8. In the worst case, that rule matches every span element. Without this bug's patches, the number of elements that we restyle is the same for both the best and worst case, although the worst case still takes longer as we end up doing more work for each element restyled. With these patches, the best case will only restyle the one element that matches the selector, plus its two children. In the worst case, we do the same work as without the patches, plus the additional overhead of passing around the selectors and converting the eRestyle_SomeDescendants into eRestyle_Selfs for each span. On my machine, I get these times: without patches best case ~900ms worst case ~6000ms with patches best case ~9ms worst case ~6070ms

Comment on attachment 8640241 [details] [diff] [review] Part 1: Add a method to match a single nsCSSSelector (without pseudos) against an Element. I don't understand why the negations and pseudo-classes restrictions are there. The code as written should work fine even if those are present, no? In particular, if that restriction is removed, you could call this method from nsCSSRuleProcessor::SelectorListMatches I would think, right? r=me with either the restriction removed or _very_ clearly explained.

Comment on attachment 8640243 [details] [diff] [review] Part 2: Add eRestyle_SomeDescendants restyle hint and pass associated restyle hint data into restyle methods. This looks ok, but it did leave me wondering why we can't just have a RestyleHintData argument and move the nsRestyleHint into that struct, since they're always supposed to go together. r=me either way, I guess, but I think I'd prefer we combine the two arguments, at least in a followup. Also, I'm assuming the lifetime management of these selector pointers will be handled in later patches somehow.

Also, should this thing be in the mozilla::css namespace or the mozilla namespace?

Comment on attachment 8640244 [details] [diff] [review] Part 3: Convert eRestyle_SomeDescendants into eRestyle_Self for elements that match selectors. >+ElementRestyler::AddPendingRestylesForDescendantsMatchingSelectors(Element* aElement) This needs some better documentation about why the mRestyleTracker.HasRestyleData(aElement) case doesn't recur on the kids and just modifies the existing restyle data instead (presumably because we're about to process that restyle data anyway). >@@ -2880,16 +2957,28 @@ ElementRestyler::Restyle(nsRestyleHint aRestyleHint) Hmm. I have to admit to being a bit lost here (perhaps better comments about what mSelectorsForDescendants would help?). How does this not end up recursively going over the descendants over and over again each step down the tree? >- else if (!(aRestyleHint & (eRestyle_Self | eRestyle_Subtree))) { A diff -w would sure have been nice here. ;) >+ const bool restyleSelf = This is computed in multiple places, and the computation is not quite trivial. Maybe factor it out? >+ * Returns true iff a selector in mSelectors matches aElement. This mSelectorsForDescendants, right? r=me with the above fixed.

Comment on attachment 8640245 [details] [diff] [review] Part 4: Store pointer to the first selector for class, ID and attribute selectors in the rule cascade. I think s/first/rightmost/ would make this a lot clearer... because the rightmost thing is not usually considered "first". Another option would be to call it the "selector matching subject" or "subject-matching selector" or some such, since that's the part we _really_ care about, right? r=me with a better name here and much better documentation on SelectorPair as to what it's storing: the first thing is a selector that might start or stop matching and the second one is a filter on the possible nodes that could have their style affected as a result, right? Should the second item be null if it's otherwise == to the first item?

Comment on attachment 8640246 [details] [diff] [review] Part 5: Add a RestyleHintData outparam to HasAttributeDependentStyle for use with eRestyle_SomeDescendants. r=me

> Should the second item be null if it's otherwise == to the first item? Ah, looks line no, null means something different in part 6. Please clearly document on SelectorPair what null means.

And perhaps add asserts that the first selector is non-null and if the two selectors are equal the first has no combinator?

Comment on attachment 8640247 [details] [diff] [review] Part 6: Return eRestyle_SomeDescendants from HasAttributeDependentStyle where appropriate. I still don't quite understand this no-pseudoclasses restriction. Why is it really there? It's not intrinsic to RestrictedSelectorMatches per previous review comments, so is it intrinsic to this optimization somehow? Whatever the answer is we should get it into the comments here. > // If enumData->change already includes all the bits of possibleChange, don't > // bother calling SelectorMatches Please fix this comment to describe the new setup and in particular why we want to call SelectorMatches even if enumData->change already includes eRestyle_SomeDescendants. >+ if (possibleChange == eRestyle_SomeDescendants) { Bit-and would make more sense here, since this is in theory a bitfield, right? r=me with that fixed.

Comment on attachment 8640248 [details] [diff] [review] Part 7: Split out FindClosestRestyleRoot and allow passing in a pre-computed restyle root to AddPendingRestyle. Why not just make the argument Element*, with null representing "call FindClosestRestyleRoot"? Is there a use case for passing in explicit null? If so, document, please. r=me

Comment on attachment 8640249 [details] [diff] [review] Part 8: Keep track of the closest restyle root in AddPendingRestylesForDescendantsMatchingSelectors. r=me. I guess this can end up passing null as restyle root if FindClosestRestyleRoot() returns null in ElementRestyler::Restyle?

Comment on attachment 8640250 [details] [diff] [review] Part 9: Clear nsCSSSelector pointers in the pending restyle tracker if they might be stale. r=me

Comment on attachment 8640251 [details] [diff] [review] Part 10: Logging. >+ nsAutoString buf; >+ aElement->NodeInfo()->NameAtom()->ToString(buf); nsDependentAtomString buf(aElement->NodeInfo()->NameAtom()); Not that this is terribly performance-sensitive code, but it's a good habit. ;) r=me

Comment on attachment 8640945 [details] [diff] [review] Part 11: Use ReparentStyleContext even if eRestyle_SomeDescendants is used. >@@ -3621,34 +3622,36 @@ ElementRestyler::RestyleSelf(nsIFrame* a Hmm. The logic in this chunk didn't use to consider aRestyleHint at all. Why do we need to consider it now? Or was that a preexisting bug? >+ if (!(aRestyleHint & ~(eRestyle_Force | eRestyle_ForceDescendants | >+ eRestyle_SomeDescendants)) && >+ !styleSet->IsInRuleTreeReconstruct()) { This condition is duplicated several times. Factor it out? >- // XXX Should the above condition ignore eRestyle_Force(Descendants) Seems to me like this part of the change is a new optimization we didn't have before, right? It may be worth doing that in a separate bug just to mitigate regression risk... r=me with that.

(In reply to On vacation August 4-25. Please mail manually if really needed. from comment #30) > Comment on attachment 8640241 [details] [diff] [review] > Part 1: Add a method to match a single nsCSSSelector (without pseudos) > against an Element. > > I don't understand why the negations and pseudo-classes restrictions are > there. The code as written should work fine even if those are present, no? I thought :not() and _moz-any() allowed selectors with combinators inside it, but they don't. My original thinking was that since they could involve expensive checking up the tree of the nested selector, you might not want to do this for every element in the subtree, especially since you need to evaluate the selector twice (first while traversing down the tree processing the eRestyle_SomeDescendants, and second when proceeding to restyle the element if it matched). If we allow all pseudo-classes I'll have to ensure that NodeMatchContext is set up correctly before matching.

(In reply to On vacation August 4-25. Please mail manually if really needed. from comment #31) > Comment on attachment 8640243 [details] [diff] [review] > Part 2: Add eRestyle_SomeDescendants restyle hint and pass associated > restyle hint data into restyle methods. > > This looks ok, but it did leave me wondering why we can't just have a > RestyleHintData argument and move the nsRestyleHint into that struct, since > they're always supposed to go together. > > r=me either way, I guess, but I think I'd prefer we combine the two > arguments, at least in a followup. Yeah, I almost did that, but changed my mind and I don't recall why now. I'll file a followup for it.

(In reply to On vacation August 4-25. Please mail manually if really needed. from comment #32) > Also, should this thing be in the mozilla::css namespace or the mozilla > namespace? I think we are trying to avoid mozilla::css at this point and one day it'll be merged down into mozilla.

(In reply to On vacation August 4-25. Please mail manually if really needed. from comment #33) > Comment on attachment 8640244 [details] [diff] [review] > Part 3: Convert eRestyle_SomeDescendants into eRestyle_Self for elements > that match selectors. > > >+ElementRestyler::AddPendingRestylesForDescendantsMatchingSelectors(Element* aElement) > > This needs some better documentation about why the > mRestyleTracker.HasRestyleData(aElement) case doesn't recur on the kids and > just modifies the existing restyle data instead (presumably because we're > about to process that restyle data anyway). OK. That is the reason, yes. > >@@ -2880,16 +2957,28 @@ ElementRestyler::Restyle(nsRestyleHint aRestyleHint) > > Hmm. I have to admit to being a bit lost here (perhaps better comments > about what mSelectorsForDescendants would help?). How does this not end up > recursively going over the descendants over and over again each step down > the tree? At this point we've decided to stop restyling, so we need to recurse down the tree to look at descendants that would otherwise be skipped. That means recursing until we find (a) an element that already has restyle data, which means we'll look at it soon once we get back up to ProcessPendingRestyles, or (b) it matches one of the mSelectorsForDescendants, in which case we need to restyle it. In both cases we stop our descendant traversal, so we shouldn't visit any descendants to check mSelectorseForDescendants against more than once. > >- else if (!(aRestyleHint & (eRestyle_Self | eRestyle_Subtree))) { > > A diff -w would sure have been nice here. ;) > > >+ const bool restyleSelf = > > This is computed in multiple places, and the computation is not quite > trivial. Maybe factor it out? OK. > >+ * Returns true iff a selector in mSelectors matches aElement. This > > mSelectorsForDescendants, right? Yep.

(In reply to On vacation August 4-25. Please mail manually if really needed. from comment #34) > Comment on attachment 8640245 [details] [diff] [review] > Part 4: Store pointer to the first selector for class, ID and attribute > selectors in the rule cascade. > > I think s/first/rightmost/ would make this a lot clearer... because the > rightmost thing is not usually considered "first". > > Another option would be to call it the "selector matching subject" or > "subject-matching selector" or some such, since that's the part we _really_ > care about, right? Yes, thanks, both of those sound clearer than "first". I like "rightmost". > r=me with a better name here and much better documentation on SelectorPair > as to what it's storing: the first thing is a selector that might start or > stop matching and the second one is a filter on the possible nodes that > could have their style affected as a result, right? Yes that's exactly right. I'll add comments.

(In reply to On vacation August 4-25. Please mail manually if really needed. from comment #37) > And perhaps add asserts that the first selector is non-null and if the two > selectors are equal the first has no combinator? Yes that makes sense to assert.

(In reply to On vacation August 4-25. Please mail manually if really needed. from comment #38) > Comment on attachment 8640247 [details] [diff] [review] > Part 6: Return eRestyle_SomeDescendants from HasAttributeDependentStyle > where appropriate. ... > Bit-and would make more sense here, since this is in theory a bitfield, > right? Yes in theory, though RestyleHintForSelectorWithAttributeChange only ever returns one bit. If we ever return more than one bit, then we strictly wouldn't need to append the selector if we got eRestyle_Subtree too, for example. But we do that kind of optimization further up the call stack once we've finished building up the selector array from all the rule processors, so doing a bit check here should be fine.

(In reply to On vacation August 4-25. Please mail manually if really needed. from comment #39) > Comment on attachment 8640248 [details] [diff] [review] > Part 7: Split out FindClosestRestyleRoot and allow passing in a pre-computed > restyle root to AddPendingRestyle. > > Why not just make the argument Element*, with null representing "call > FindClosestRestyleRoot"? Is there a use case for passing in explicit null? > If so, document, please. Yes, explicit null means "there was no restyle root found on all ancestors" and Nothing means "we haven't checked for a restyle root yet". I'll document.

(In reply to On vacation August 4-25. Please mail manually if really needed. from comment #43) > Comment on attachment 8640945 [details] [diff] [review] > Part 11: Use ReparentStyleContext even if eRestyle_SomeDescendants is used. > > >@@ -3621,34 +3622,36 @@ ElementRestyler::RestyleSelf(nsIFrame* a > > Hmm. The logic in this chunk didn't use to consider aRestyleHint at all. > Why do we need to consider it now? Or was that a preexisting bug? I don't believe it's a bug, since it's fine to call ResolveStyleWithReplacement instead of ReparentStyleContext, it just does some unnecessary work. I'll split these changes out into a separate bug since they're not required here. (I just wanted to make the code paths for restyling the primary frame's style context, its extra style contexts, and restyling undisplayed content to be uniform.)

(In reply to On vacation August 4-25. Please mail manually if really needed. from comment #30) > Comment on attachment 8640241 [details] [diff] [review] > Part 1: Add a method to match a single nsCSSSelector (without pseudos) > against an Element. ... > In particular, if that restriction is removed, you could call this method > from nsCSSRuleProcessor::SelectorListMatches I would think, right? I think in the end we can't do that, since RestrictedSelectorMatches will want to ensure :visited links match properly, which SelectorListMatches doesn't.

> I thought :not() and _moz-any() allowed selectors with combinators inside it Ah, they don't yet, but eventually they would, I think. That's a good point. > If we allow all pseudo-classes I'll have to ensure that NodeMatchContext is set > up correctly before matching. This is to get :link and :visited correct (per comment 53)? I agree that that sounds too annoying. Perhaps add comments documenting all this to explain why the restriction is there so someone doesn't try to fix it? > At this point we've decided to stop restyling, so we need to recurse down the > tree to look at descendants that would otherwise be skipped. [etc] This might be good as a code comment. > I don't believe it's a bug, since it's fine to call ResolveStyleWithReplacement > instead of ReparentStyleContext, it just does some unnecessary work. Performance bugs are still bugs. ;) Splitting out makes sense, as does trying to make the various codepaths be uniform.

(In reply to On vacation August 4-25. Please mail manually if really needed. from comment #55) > This is to get :link and :visited correct (per comment 53)? I agree that > that sounds too annoying. Perhaps add comments documenting all this to > explain why the restriction is there so someone doesn't try to fix it? I think we can handle this without too much trouble by making RestrictedSelectorMatches do the matching for both unvisited and visited handling, if nodeContext.mIsRelevantLink (which we just initialized to nsCSSRuleProcessor::IsLink(aElement)) is true. That's probably better than restricting the selector to having no pseudo-classes at all or searching for :visited/:link in mPseodoClasses when deciding whether we can generate an eRestyle_SomeDescendants.

https://hg.mozilla.org/integration/mozilla-inbound/rev/ef165b896cf4 https://hg.mozilla.org/integration/mozilla-inbound/rev/20984dfa4302 https://hg.mozilla.org/integration/mozilla-inbound/rev/4a7b79980353 https://hg.mozilla.org/integration/mozilla-inbound/rev/b99c358a6fea https://hg.mozilla.org/integration/mozilla-inbound/rev/9bcc3233f3c8 https://hg.mozilla.org/integration/mozilla-inbound/rev/cfeeae42d514 https://hg.mozilla.org/integration/mozilla-inbound/rev/b7ec8d4d2d7e https://hg.mozilla.org/integration/mozilla-inbound/rev/37493f6eef20 https://hg.mozilla.org/integration/mozilla-inbound/rev/9b41cd9f2bc5 https://hg.mozilla.org/integration/mozilla-inbound/rev/15ad6049b940 https://hg.mozilla.org/integration/mozilla-inbound/rev/91a3e2205388

(In reply to Cameron McCormack (:heycam) from comment #56) > I think we can handle this without too much trouble by making > RestrictedSelectorMatches do the matching for both unvisited and visited > handling, if nodeContext.mIsRelevantLink (which we just initialized to > nsCSSRuleProcessor::IsLink(aElement)) is true. That's probably better than > restricting the selector to having no pseudo-classes at all or searching for > :visited/:link in mPseodoClasses when deciding whether we can generate an > eRestyle_SomeDescendants. I did this, and added a warning if we detect selectors inside mNegations or mPseudoClasses that have a combinator, so we can re-evaluate what to do with them if we allow that in the future.

sorry had to back this out for crashes like https://treeherder.mozilla.org/logviewer.html#?job_id=12466216&repo=mozilla-inbound

Backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/01576b408ea7

https://hg.mozilla.org/integration/mozilla-inbound/rev/7b8dc7443e3a https://hg.mozilla.org/integration/mozilla-inbound/rev/fc5ff8a35475 https://hg.mozilla.org/integration/mozilla-inbound/rev/d69928567288 https://hg.mozilla.org/integration/mozilla-inbound/rev/cf1a39719f3b https://hg.mozilla.org/integration/mozilla-inbound/rev/2f73362274d6 https://hg.mozilla.org/integration/mozilla-inbound/rev/13729747e3c2 https://hg.mozilla.org/integration/mozilla-inbound/rev/c45ba0178caa https://hg.mozilla.org/integration/mozilla-inbound/rev/b7367a66c33a https://hg.mozilla.org/integration/mozilla-inbound/rev/9f7deabf4068 https://hg.mozilla.org/integration/mozilla-inbound/rev/68bef2d8c8bf https://hg.mozilla.org/integration/mozilla-inbound/rev/6f97ff142c89

https://hg.mozilla.org/mozilla-central/rev/7b8dc7443e3a https://hg.mozilla.org/mozilla-central/rev/fc5ff8a35475 https://hg.mozilla.org/mozilla-central/rev/d69928567288 https://hg.mozilla.org/mozilla-central/rev/cf1a39719f3b https://hg.mozilla.org/mozilla-central/rev/2f73362274d6 https://hg.mozilla.org/mozilla-central/rev/13729747e3c2 https://hg.mozilla.org/mozilla-central/rev/c45ba0178caa https://hg.mozilla.org/mozilla-central/rev/b7367a66c33a https://hg.mozilla.org/mozilla-central/rev/9f7deabf4068 https://hg.mozilla.org/mozilla-central/rev/68bef2d8c8bf https://hg.mozilla.org/mozilla-central/rev/6f97ff142c89

Hey Cameron, do you think it's possible to create a Talos test for this ?

I've spoken previously to avhi and jmaher about adding small performance test cases like this, but I got the impression that it would be difficult. I can't remember the details now. Maybe for Talos a test measuring the time to open the SMS app would be more appropriate? We could add a test to ensure that we don't regress these specific improvements by adding some counters in debug builds for the number of frames that get restyled (for example), and in a mochitest check that the expected number of frames do get restyled.

I'll discuss with other Gaia folks to see how we can test this from Gaia. I know we can detect reflows, maybe we can detect restyles (with the hint !). Thanks !

Release Note Request (optional, but appreciated) [Why is this notable]: less processor churn, (potentially) more battery life [Suggested wording]: Improved performance on interactive websites that trigger a lot of restyles [Links (documentation, blog post, etc)]: (this bug?)

Added to the release notes with Florian's wording. Once more, thanks for your release notes submission, it is really appreciated!

(In reply to Julien Wajsberg [:julienw] from comment #68) > I'll discuss with other Gaia folks to see how we can test this from Gaia. I > know we can detect reflows, maybe we can detect restyles (with the hint !). > Thanks ! FYI I filed bug 1192168 about this.

(In reply to Florian Bender from comment #69) > Release Note Request (optional, but appreciated) > [Why is this notable]: less processor churn, (potentially) more battery life > [Suggested wording]: Improved performance on interactive websites that > trigger a lot of restyles > [Links (documentation, blog post, etc)]: (this bug?) We are looking to mention on going improvements to performance on the Mozilla blog and it sounds like this one in particular will be notable for users. Question - does the improvement translate into faster page loads? Better page scrolling? Wondering if it's OK for us to be specific on how it helps users. Any more detail you can provide on this that we can include in a blog post?

I'm not sure the best way to describe the improvements here. :-) Ultimately the improvement is "restyles resulting from certain kinds of dynamic modifications, such as element class name changes, are processed faster". That could translate to more responsive pages -- for example, performing some interaction on the page that does a class name change could be fast enough now not to miss frames.

Maybe: "eliminates some sources of perceived lag on very dynamic websites".