Bug 1216431 patch 6 - Back out bug 1209603 patch 8. r=heycam
authorL. David Baron <dbaron@dbaron.org>
Fri, 23 Oct 2015 08:57:36 +0900
changeset 304305 063bf9fb0cd344462c570a4cc05c2f537ecf52c8
parent 304304 3b9f6b8b11a48930e836c786646848230e007a48
child 304306 713de8f859af05f24fdde909629863f667d93e11
push id1001
push userraliiev@mozilla.com
push dateMon, 18 Jan 2016 19:06:03 +0000
treeherdermozilla-release@8b89261f3ac4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersheycam
bugs1216431, 1209603
milestone44.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 1216431 patch 6 - Back out bug 1209603 patch 8. r=heycam This requires a little bit of gymnastics since it has to add the inverse of tests, since the is-a-reset-struct tests originally added in patch 8 were made unconditional in patch 9, and with this backout we now want to execute the code only for inherited structs. This also doesn't back out the cleanup to use NS_STYLE_INHERIT_BIT() for constants rather than nsCachedStyleData::GetBitForSID. This backs out the part of bug 1209603 whose speed I was concerned about.
layout/style/nsRuleNode.cpp
layout/style/nsRuleNode.h
layout/style/nsStyleContext.h
--- a/layout/style/nsRuleNode.cpp
+++ b/layout/style/nsRuleNode.cpp
@@ -2367,22 +2367,21 @@ nsRuleNode::WalkRuleTree(const nsStyleSt
 
   if (detail == eRuleNone && startStruct) {
     // We specified absolutely no rule information, but a parent rule in the tree
     // specified all the rule information.  We set a bit along the branch from our
     // node in the tree to the node that specified the data that tells nodes on that
     // branch that they never need to examine their rules for this particular struct type
     // ever again.
     PropagateDependentBit(aSID, ruleNode, startStruct);
-    // With this same bit set, we do two different things:
-    // For reset structs, record that we have asked for this struct on
-    // this context, but it is not cached on the context.
     // For inherited structs, mark the struct (which will be set on
     // the context by our caller) as not being owned by the context.
-    aContext->AddStyleBit(nsCachedStyleData::GetBitForSID(aSID));
+    if (!isReset) {
+      aContext->AddStyleBit(nsCachedStyleData::GetBitForSID(aSID));
+    }
     return startStruct;
   }
   if ((!startStruct && !isReset &&
        (detail == eRuleNone || detail == eRulePartialInherited)) ||
       detail == eRuleFullInherited) {
     // We specified no non-inherited information and neither did any of
     // our parent rules.
 
@@ -2777,19 +2776,16 @@ nsRuleNode::SetDefaultOnRoot(const nsSty
     }                                                                         \
     NS_ASSERTION(!aHighestNode->mStyleData.mResetData->                       \
                    GetStyleData(eStyleStruct_##type_),                        \
                  "Going to leak style data");                                 \
     aHighestNode->mStyleData.mResetData->                                     \
       SetStyleData(eStyleStruct_##type_, data_);                              \
     /* Propagate the bit down. */                                             \
     PropagateDependentBit(eStyleStruct_##type_, aHighestNode, data_);         \
-    /* Tell the context that we've gotten the data (separate meaning */       \
-    /* of mBits when the cached data pointer is null) */                      \
-    aContext->AddStyleBit(NS_STYLE_INHERIT_BIT(type_));                       \
   } else if (conditions.Cacheable()) {                                        \
     if (!mStyleData.mResetData) {                                             \
       mStyleData.mResetData = new (mPresContext) nsConditionalResetStyleData; \
     }                                                                         \
     mStyleData.mResetData->                                                   \
       SetStyleData(eStyleStruct_##type_, mPresContext, data_, conditions);    \
     /* Tell the style context that it doesn't own the data */                 \
     aContext->AddStyleBit(NS_STYLE_INHERIT_BIT(type_));                       \
@@ -9431,22 +9427,21 @@ nsRuleNode::GetStyleData(nsStyleStructID
 
   const void *data;
 
   // Never use cached data for animated style inside a pseudo-element;
   // see comment on cacheability in AnimValuesStyleRule::MapRuleInfoInto.
   if (!(HasAnimationData() && ParentHasPseudoElementData(aContext))) {
     data = mStyleData.GetStyleData(aSID, aContext, aComputeData);
     if (MOZ_LIKELY(data != nullptr)) {
-      // With this same bit set, we do two different things:
-      // For reset structs, mark the struct as having been retrieved for
-      // this context.
       // For inherited structs, mark the struct (which will be set on
       // the context by our caller) as not being owned by the context.
-      aContext->AddStyleBit(nsCachedStyleData::GetBitForSID(aSID));
+      if (!nsCachedStyleData::IsReset(aSID)) {
+        aContext->AddStyleBit(nsCachedStyleData::GetBitForSID(aSID));
+      }
       return data; // We have a fully specified struct. Just return it.
     }
   }
 
   if (MOZ_UNLIKELY(!aComputeData))
     return nullptr;
 
   // Nothing is cached.  We'll have to delve further and examine our rules.
--- a/layout/style/nsRuleNode.h
+++ b/layout/style/nsRuleNode.h
@@ -927,36 +927,32 @@ public:
                                                                               \
     MOZ_ASSERT(data, "should have aborted on out-of-memory");                 \
     return data;                                                              \
   }
 
   #define STYLE_STRUCT_RESET(name_, checkdata_cb_)                            \
   template<bool aComputeData>                                                 \
   const nsStyle##name_*                                                       \
-  GetStyle##name_(nsStyleContext* aContext, uint64_t& aContextStyleBits)      \
+  GetStyle##name_(nsStyleContext* aContext)                                   \
   {                                                                           \
     NS_ASSERTION(IsUsedDirectly(),                                            \
                  "if we ever call this on rule nodes that aren't used "       \
                  "directly, we should adjust handling of mDependentBits "     \
                  "in some way.");                                             \
     MOZ_ASSERT(!ContextHasCachedData(aContext, eStyleStruct_##name_),         \
                "style context should not have cached data for struct");       \
                                                                               \
     const nsStyle##name_ *data;                                               \
                                                                               \
     /* Never use cached data for animated style inside a pseudo-element; */   \
     /* see comment on cacheability in AnimValuesStyleRule::MapRuleInfoInto */ \
     if (!(HasAnimationData() && ParentHasPseudoElementData(aContext))) {      \
       data = mStyleData.GetStyle##name_(aContext, aComputeData);              \
       if (MOZ_LIKELY(data != nullptr)) {                                      \
-        /* Mark the struct as having been retrieved for this context. */      \
-        /* Normally this would be aContext->AddStyleBit(), but aContext is */ \
-        /* an incomplete type here, so we work around that with a param. */   \
-        aContextStyleBits |= NS_STYLE_INHERIT_BIT(name_);                     \
         return data;                                                          \
       }                                                                       \
     }                                                                         \
                                                                               \
     if (!aComputeData)                                                        \
       return nullptr;                                                         \
                                                                               \
     data = static_cast<const nsStyle##name_ *>                                \
--- a/layout/style/nsStyleContext.h
+++ b/layout/style/nsStyleContext.h
@@ -555,17 +555,17 @@ private:
         const nsStyle##name_ * cachedData =                             \
           static_cast<nsStyle##name_*>(                                 \
             mCachedResetData->mStyleStructs[eStyleStruct_##name_]);     \
         if (cachedData) /* Have it cached already, yay */               \
           return cachedData;                                            \
       }                                                                 \
       /* Have the rulenode deal */                                      \
       AUTO_CHECK_DEPENDENCY(eStyleStruct_##name_);                      \
-      return mRuleNode->GetStyle##name_<aComputeData>(this, mBits);     \
+      return mRuleNode->GetStyle##name_<aComputeData>(this);            \
     }
   #include "nsStyleStructList.h"
   #undef STYLE_STRUCT_RESET
   #undef STYLE_STRUCT_INHERITED
 
   // Helper for ClearCachedInheritedStyleDataOnDescendants.
   void DoClearCachedInheritedStyleDataOnDescendants(uint32_t aStructs);
 
@@ -625,20 +625,16 @@ private:
   // mCachedResetData.
   nsResetStyleData*       mCachedResetData; // Cached reset style data.
   nsInheritedStyleData    mCachedInheritedData; // Cached inherited style data
 
   // mBits stores a number of things:
   //  - For all structs, when they are non-null in the style context's
   //    storage, it records (using the style struct bits) which structs
   //    are inherited from the parent context or owned by mRuleNode.
-  //  - For reset (non-inherited) style structs, when they are null in
-  //    the style context's storage, it records that we have been asked
-  //    for that struct.  (We don't need this for inherited structs
-  //    since we always cache them in mCachedInheritedData.)
   //  - It also stores the additional bits listed at the top of
   //    nsStyleStruct.h.
   uint64_t                mBits;
 
   uint32_t                mRefCnt;
 
 #ifdef DEBUG
   uint32_t                mFrameRefCnt; // number of frames that use this