Bug 1302054 - Part 1: Avoid computing style differences when we just want to ensure structs are cached on the new context. r=dbaron
authorCameron McCormack <cam@mcc.id.au>
Tue, 21 Mar 2017 16:33:05 +0800
changeset 348668 6a196410f140cb1186508f9c218ff8f9651505e9
parent 348667 70914315e02b97090945edfe996b7c8604c55d00
child 348669 a60081798b342f5d18128648ea1578fdf98e5757
push id31533
push userkwierso@gmail.com
push dateTue, 21 Mar 2017 23:08:53 +0000
treeherdermozilla-central@8744e9f8eb99 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdbaron
bugs1302054
milestone55.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1302054 - Part 1: Avoid computing style differences when we just want to ensure structs are cached on the new context. r=dbaron MozReview-Commit-ID: DLhHcCD4GQS
layout/base/GeckoRestyleManager.cpp
layout/base/ServoRestyleManager.cpp
layout/style/nsStyleContext.cpp
layout/style/nsStyleContext.h
--- a/layout/base/GeckoRestyleManager.cpp
+++ b/layout/base/GeckoRestyleManager.cpp
@@ -962,31 +962,20 @@ GeckoRestyleManager::ReparentStyleContex
       // nsTransitionManager::ConsiderInitiatingTransition.
 #if 0
       if (!copyFromContinuation) {
         TryInitiatingTransition(mPresContext, aFrame->GetContent(),
                                 oldContext, &newContext);
       }
 #endif
 
-      // Make sure to call CalcStyleDifference so that the new context ends
-      // up resolving all the structs the old context resolved.
+      // Ensure the new context ends up resolving all the structs the old
+      // context resolved.
       if (!copyFromContinuation) {
-        uint32_t equalStructs;
-        uint32_t samePointerStructs;
-        DebugOnly<nsChangeHint> styleChange =
-          oldContext->CalcStyleDifference(newContext, nsChangeHint(0),
-                                          &equalStructs,
-                                          &samePointerStructs);
-        // The style change is always 0 because we have the same rulenode and
-        // CalcStyleDifference optimizes us away.  That's OK, though:
-        // reparenting should never trigger a frame reconstruct, and whenever
-        // it's happening we already plan to reflow and repaint the frames.
-        NS_ASSERTION(!(styleChange & nsChangeHint_ReconstructFrame),
-                     "Our frame tree is likely to be bogus!");
+        newContext->EnsureSameStructsCached(oldContext);
       }
 
       aFrame->SetStyleContext(newContext);
 
       nsIFrame::ChildListIterator lists(aFrame);
       for (; !lists.IsDone(); lists.Next()) {
         for (nsIFrame* child : lists.CurrentList()) {
           // only do frames that are in flow
@@ -1028,33 +1017,19 @@ GeckoRestyleManager::ReparentStyleContex
            (oldExtraContext = aFrame->GetAdditionalStyleContext(contextIndex));
            ++contextIndex) {
         RefPtr<nsStyleContext> newExtraContext;
         newExtraContext = StyleSet()->
                             ReparentStyleContext(oldExtraContext,
                                                  newContext, nullptr);
         if (newExtraContext) {
           if (newExtraContext != oldExtraContext) {
-            // Make sure to call CalcStyleDifference so that the new
-            // context ends up resolving all the structs the old context
-            // resolved.
-            uint32_t equalStructs;
-            uint32_t samePointerStructs;
-            DebugOnly<nsChangeHint> styleChange =
-              oldExtraContext->CalcStyleDifference(newExtraContext,
-                                                   nsChangeHint(0),
-                                                   &equalStructs,
-                                                   &samePointerStructs);
-            // The style change is always 0 because we have the same
-            // rulenode and CalcStyleDifference optimizes us away.  That's
-            // OK, though: reparenting should never trigger a frame
-            // reconstruct, and whenever it's happening we already plan to
-            // reflow and repaint the frames.
-            NS_ASSERTION(!(styleChange & nsChangeHint_ReconstructFrame),
-                         "Our frame tree is likely to be bogus!");
+            // Ensure the new context ends up resolving all the structs the old
+            // context resolved.
+            newContext->EnsureSameStructsCached(oldContext);
           }
 
           aFrame->SetAdditionalStyleContext(contextIndex, newExtraContext);
         }
       }
 #ifdef DEBUG
       DebugVerifyStyleTree(aFrame);
 #endif
--- a/layout/base/ServoRestyleManager.cpp
+++ b/layout/base/ServoRestyleManager.cpp
@@ -208,17 +208,17 @@ ServoRestyleManager::ProcessPostTraversa
 
   RefPtr<nsStyleContext> newContext = nullptr;
   if (recreateContext) {
     MOZ_ASSERT(styleFrame || displayContentsNode);
     newContext =
       aStyleSet->GetContext(computedValues.forget(), aParentContext, nullptr,
                             CSSPseudoElementType::NotPseudo, aElement);
 
-    newContext->EnsureStructsForServo(oldStyleContext);
+    newContext->EnsureSameStructsCached(oldStyleContext);
 
     // XXX This could not always work as expected: there are kinds of content
     // with the first split and the last sharing style, but others not. We
     // should handle those properly.
     // XXXbz I think the UpdateStyleOfOwnedAnonBoxes call below handles _that_
     // right, but not other cases where we happen to have different styles on
     // different continuations... (e.g. first-line).
     for (nsIFrame* f = styleFrame; f;
--- a/layout/style/nsStyleContext.cpp
+++ b/layout/style/nsStyleContext.cpp
@@ -174,46 +174,16 @@ nsStyleContext::FinishConstruction(bool 
   }
 
   #define eStyleStruct_LastItem (nsStyleStructID_Length - 1)
   NS_ASSERTION(NS_STYLE_INHERIT_MASK & NS_STYLE_INHERIT_BIT(LastItem),
                "NS_STYLE_INHERIT_MASK must be bigger, and other bits shifted");
   #undef eStyleStruct_LastItem
 }
 
-void
-nsStyleContext::EnsureStructsForServo(const nsStyleContext* aOldContext)
-{
-  MOZ_ASSERT(aOldContext);
-  MOZ_ASSERT(mSource.IsServoComputedValues() &&
-             aOldContext->mSource.IsServoComputedValues());
-
-  // NOTE(emilio): We could do better here. We only call Style##name_() because
-  // we need to run FinishStyle, but otherwise this is only a bitwise or.
-  //
-  // We could reduce the FFI traffic we do only doing it for structs that have
-  // non-trivial FinishStyle.
-#define STYLE_STRUCT(name_, checkdata_cb)                                      \
-  if (aOldContext->mBits & NS_STYLE_INHERIT_BIT(name_)) {                      \
-    mozilla::Unused << Style##name_();                                         \
-  }
-
-#include "nsStyleStructList.h"
-
-#undef STYLE_STRUCT
-
-#ifdef DEBUG
-  auto oldMask = aOldContext->mBits & NS_STYLE_INHERIT_MASK;
-  auto newMask = mBits & NS_STYLE_INHERIT_MASK;
-  MOZ_ASSERT((oldMask & newMask) == oldMask,
-             "Should have at least as many structs computed as the "
-             "old context!");
-#endif
-}
-
 nsStyleContext::~nsStyleContext()
 {
   NS_ASSERTION((nullptr == mChild) && (nullptr == mEmptyChild), "destructing context with children");
   MOZ_ASSERT_IF(mSource.IsServoComputedValues(), !mCachedResetData);
 
 #ifdef DEBUG
   if (mSource.IsServoComputedValues()) {
     MOZ_ASSERT(!mCachedResetData,
@@ -1326,16 +1296,44 @@ nsStyleContext::CalcStyleDifference(cons
 {
   FakeStyleContext newContext(aNewComputedValues);
   return CalcStyleDifferenceInternal(&newContext,
                                      aParentHintsNotHandledForDescendants,
                                      aEqualStructs,
                                      aSamePointerStructs);
 }
 
+void
+nsStyleContext::EnsureSameStructsCached(nsStyleContext* aOldContext)
+{
+  // NOTE(emilio): We could do better here for stylo, where we only call
+  // Style##name_() because we need to run FinishStyle, but otherwise this
+  // is only a bitwise or.
+  //
+  // We could reduce the FFI traffic we do only doing it for structs that have
+  // non-trivial FinishStyle.
+
+#define STYLE_STRUCT(name_, checkdata_cb_)                                    \
+  if (aOldContext->PeekStyle##name_()) {                                      \
+    Style##name_();                                                           \
+  }
+#include "nsStyleStructList.h"
+#undef STYLE_STRUCT
+
+#ifdef DEBUG
+  if (mSource.IsServoComputedValues()) {
+    auto oldMask = aOldContext->mBits & NS_STYLE_INHERIT_MASK;
+    auto newMask = mBits & NS_STYLE_INHERIT_MASK;
+    MOZ_ASSERT((oldMask & newMask) == oldMask,
+               "Should have at least as many structs computed as the "
+               "old context!");
+  }
+#endif
+}
+
 #ifdef DEBUG
 void nsStyleContext::List(FILE* out, int32_t aIndent, bool aListDescendants)
 {
   nsAutoCString str;
   // Indent
   int32_t ix;
   for (ix = aIndent; --ix >= 0; ) {
     str.AppendLiteral("  ");
--- a/layout/style/nsStyleContext.h
+++ b/layout/style/nsStyleContext.h
@@ -382,24 +382,16 @@ public:
   #define STYLE_STRUCT(name_, checkdata_cb_)              \
     const nsStyle##name_ * PeekStyle##name_() {           \
       return DoGetStyle##name_<false>();                  \
     }
   #include "nsStyleStructList.h"
   #undef STYLE_STRUCT
 
   /**
-   * Ensures that this context's computed struct list is at least the old
-   * context's.
-   *
-   * aOldContext must not be null.
-   */
-  void EnsureStructsForServo(const nsStyleContext* aOldContext);
-
-  /**
    * Compute the style changes needed during restyling when this style
    * context is being replaced by aNewContext.  (This is nonsymmetric since
    * we optimize by skipping comparison for styles that have never been
    * requested.)
    *
    * This method returns a change hint (see nsChangeHint.h).  All change
    * hints apply to the frame and its later continuations or ib-split
    * siblings.  Most (all of those except the "NotHandledForDescendants"
@@ -431,16 +423,22 @@ private:
   template<class StyleContextLike>
   nsChangeHint CalcStyleDifferenceInternal(StyleContextLike* aNewContext,
                                            nsChangeHint aParentHintsNotHandledForDescendants,
                                            uint32_t* aEqualStructs,
                                            uint32_t* aSamePointerStructs);
 
 public:
   /**
+   * Ensures the same structs are cached on this style context as would be
+   * done if we called aOther->CalcDifference(this).
+   */
+  void EnsureSameStructsCached(nsStyleContext* aOldContext);
+
+  /**
    * Get a color that depends on link-visitedness using this and
    * this->GetStyleIfVisited().
    *
    * @param aField A pointer to a member variable in a style struct.
    *               The member variable and its style struct must have
    *               been listed in nsCSSVisitedDependentPropList.h.
    */
   template<typename T, typename S>