Bug 1274443 - Always cache reset structs for servo. r=heycam
authorBobby Holley <bobbyholley@gmail.com>
Thu, 19 May 2016 21:38:52 -0700
changeset 337873 15df40db9c161ae5702dcfdbde4e6b358db76be8
parent 337872 0b5d2ad721da892b90faa8e973be30a2dff270c8
child 337874 d67e5ce76bf59714d57fe921107c5f82e162a72c
push id6249
push userjlund@mozilla.com
push dateMon, 01 Aug 2016 13:59:36 +0000
treeherdermozilla-beta@bad9d4f5bf7e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersheycam
bugs1274443
milestone49.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 1274443 - Always cache reset structs for servo. r=heycam The shared code always caches inherited structs, but not reset structs. Without this change we will always do an FFI call to get the struct.
layout/style/nsStyleContext.h
--- a/layout/style/nsStyleContext.h
+++ b/layout/style/nsStyleContext.h
@@ -618,18 +618,32 @@ private:
       AUTO_CHECK_DEPENDENCY(eStyleStruct_##name_);                      \
       const nsStyle##name_ * newData;                                   \
       if (mSource.IsGeckoRuleNode()) {                                  \
         newData = mSource.AsGeckoRuleNode()->                           \
           GetStyle##name_<aComputeData>(this);                          \
       } else {                                                          \
         newData =                                                       \
           Servo_GetStyle##name_(mSource.AsServoComputedValues());       \
-        /* the Servo-backed StyleContextSource owns the struct */       \
+        /* The Servo-backed StyleContextSource owns the struct.         \
+         *                                                              \
+         * XXXbholley: Unconditionally caching reset structs here       \
+         * defeats the memory optimization where we lazily allocate     \
+         * mCachedResetData, so that we can avoid performing an FFI     \
+         * call each time we want to get the style structs. We should   \
+         * measure the tradeoffs at some point. If the FFI overhead is  \
+         * low and the memory win significant, we should consider       \
+         * _always_ grabbing the struct over FFI, and potentially       \
+         * giving mCachedInheritedData the same treatment.              \
+         *                                                              \
+         * Note that there is a similar comment in StyleData().         \
+         */                                                             \
         AddStyleBit(NS_STYLE_INHERIT_BIT(name_));                       \
+        SetStyle(eStyleStruct_##name_,                                  \
+                 const_cast<nsStyle##name_*>(newData));                 \
       }                                                                 \
       return newData;                                                   \
     }
   #include "nsStyleStructList.h"
   #undef STYLE_STRUCT_RESET
   #undef STYLE_STRUCT_INHERITED
 
   // Helper for ClearCachedInheritedStyleDataOnDescendants.