Add a fast-path for independent inherited properties

third_party/WebKit/Source/core/dom/Element.cpp

 /*
  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
  *           (C) 2001 Peter Kelly (pmk@post.com)
  *           (C) 2001 Dirk Mueller (mueller@kde.org)
  *           (C) 2007 David Smith (catfish.man@gmail.com)
  * Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2012, 2013 Apple Inc. All rights reserved.
  *           (C) 2007 Eric Seidel (eric@webkit.org)
  *
  * This library is free software; you can redistribute it and/or
@@ skipping 1690 matching lines @@
 void Element::recalcStyle(StyleRecalcChange change, Text* nextTextSibling)
 {
     DCHECK(document().inStyleRecalc());
     DCHECK(!document().lifecycle().inDetach());
     DCHECK(!parentOrShadowHostNode()->needsStyleRecalc());
     DCHECK(inActiveDocument());
 
     if (hasCustomStyleCallbacks())
         willRecalcStyle(change);
 
-    if (change >= Inherit || needsStyleRecalc()) {
+    if (change >= IndependentInherit || needsStyleRecalc()) {
         if (hasRareData()) {
             ElementRareData* data = elementRareData();
-            data->clearComputedStyle();
+            if (change != IndependentInherit)
+                data->clearComputedStyle();
 
-            if (change >= Inherit) {
+            if (change >= IndependentInherit) {
                 if (ElementAnimations* elementAnimations = data->elementAnimations())
                     elementAnimations->setAnimationStyleChange(false);
             }
         }
         if (parentComputedStyle())
             change = recalcOwnStyle(change);
         clearNeedsStyleRecalc();
     }
 
     // If we reattached we don't need to recalc the style of our descendants anymore.
@@ skipping 23 matching lines @@
         clearChildNeedsStyleRecalc();
     }
 
     if (hasCustomStyleCallbacks())
         didRecalcStyle(change);
 
     if (change == Reattach)
         reattachWhitespaceSiblingsIfNeeded(nextTextSibling);
 }
 
+bool Element::canPropagateInheritedProperties(StyleRecalcChange change)
+{
+    return change == IndependentInherit

[esprehn, 2016/07/19 03:46:26]

if (change != ...)
  return false;
if (!parentComputedStyle())
  return false;
if (hasAnimations())
  return false;
ComputedStyle* style = ...;
return !style->animations() && !style->transitions();

That style is easier to read and doesn't recompute the same stuff repeatedly. :)

[sashab, 2016/07/19 06:32:38]

Nice :) Thanks, done

+        && !needsStyleRecalc()
+        && computedStyle()

[esprehn, 2016/07/19 03:46:26]

computedStyle() contains a bunch of branches, you don't want to call it
repeatedly like this

[sashab, 2016/07/19 06:32:38]

Yup nice, thanks

+        && parentComputedStyle()
+        && !computedStyle()->animations()
+        && !computedStyle()->transitions()
+        && !hasAnimations();
+}
+
 StyleRecalcChange Element::recalcOwnStyle(StyleRecalcChange change)
 {
     DCHECK(document().inStyleRecalc());
     DCHECK(!parentOrShadowHostNode()->needsStyleRecalc());
-    DCHECK(change >= Inherit || needsStyleRecalc());
+    DCHECK(change >= IndependentInherit || needsStyleRecalc());
     DCHECK(parentComputedStyle());
 
     RefPtr<ComputedStyle> oldStyle = mutableComputedStyle();
-    RefPtr<ComputedStyle> newStyle = styleForLayoutObject();
+    RefPtr<ComputedStyle> newStyle;
+
+    // When propagating inherited changes, we don't need to do a full style recalc
+    // if the only changed properties are independent. In this case, we can simply
+    // set these directly on the ComputedStyle object.
+    if (canPropagateInheritedProperties(change)) {
+        newStyle = ComputedStyle::clone(*computedStyle());

[Timothy Loh, 2016/07/19 01:29:32]

clone(oldStyle)?

[sashab, 2016/07/19 03:31:35]

Done

[esprehn, 2016/07/19 03:46:26]

yeah, don't call computedStyle() repeatedly it's got a bunch of branches in it
:)

+        ComputedStyle::propagateIndependentInheritedProperties(*parentComputedStyle(), *newStyle);

[esprehn, 2016/07/19 03:46:26]

parentComputedStyle() isn't free, I think you want to merge this and the
canPropagateInheritedProperties into a function like:

bool propagateInheritedProperties(change)

that has the early returns and avoids calling it multiple times.

then do:

if (!propagateInheritedProperties(change))
  ...

[sashab, 2016/07/19 06:32:38]

Nice!! Done.

Had to pass newStyle because its used later in the function. Could technically
get rid of the double-call from oldStyle = mutableComputedStyle() but probably
not worth it. :)

+        INCREMENT_STYLE_STATS_COUNTER(document().styleEngine(), independentInheritedStylesPropagated, 1);
+    } else {
+        newStyle = styleForLayoutObject();
+    }
     DCHECK(newStyle);
 
     StyleRecalcChange localChange = ComputedStyle::stylePropagationDiff(oldStyle.get(), newStyle.get());

[esprehn, 2016/07/19 03:46:26]

do we even need to run any of this now if we took the propagation path above?

[sashab, 2016/07/19 06:32:38]

That's a good question. I've left it this way now so that:
1) The stats counter gets incremented
2) Callback selectors get updated (what is that even?)

but that's it. The rest is skipped. If you think neither of those things are
worth it then we can early return.

     if (localChange == NoChange) {
         INCREMENT_STYLE_STATS_COUNTER(document().styleEngine(), stylesUnchanged, 1);
     } else {
         INCREMENT_STYLE_STATS_COUNTER(document().styleEngine(), stylesChanged, 1);
     }
 
     if (localChange == Reattach) {
         // TODO(nainar): Remove the style parameter being passed into buildOwnLayout().
         // ComputedStyle will now be stored on Node and accessed in buildOwnLayout()
         // using mutableComputedStyle().
@@ skipping 16 matching lines @@
             layoutObject->setStyleInternal(newStyle.get());
         }
     }
 
     if (getStyleChangeType() >= SubtreeStyleChange)
         return Force;
 
     if (change > Inherit || localChange > Inherit)
         return max(localChange, change);
 
-    if (localChange < Inherit) {
+    if (localChange < IndependentInherit) {
         if (oldStyle->hasChildDependentFlags()) {
             if (childNeedsStyleRecalc())
                 return Inherit;
             newStyle->copyChildDependentFlagsFrom(*oldStyle);
         }
         if (oldStyle->hasPseudoElementStyle() || newStyle->hasPseudoElementStyle())
             return UpdatePseudoElements;
     }
 
     return localChange;
@@ skipping 1924 matching lines @@
 
 DEFINE_TRACE_WRAPPERS(Element)
 {
     if (hasRareData()) {
         visitor->traceWrappers(elementRareData());
     }
     ContainerNode::traceWrappers(visitor);
 }
 
 } // namespace blink