Bug 860573 - Part 1 - Store wrapper cache flags separately to the object pointer r=smaug
authorJon Coppeard <jcoppeard@mozilla.com>
Sat, 08 Jun 2013 09:53:21 +0100
changeset 145940 bbb8169d421673bfcd8d419c8c9711be0d656349
parent 145939 697190293f4e1d4692a42055696ba11534b31476
child 145941 4cecde6e32b0b9c1821e07902b9247f15e326f37
push id2697
push userbbajaj@mozilla.com
push dateMon, 05 Aug 2013 18:49:53 +0000
treeherdermozilla-beta@dfec938c7b63 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs860573
milestone24.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 860573 - Part 1 - Store wrapper cache flags separately to the object pointer r=smaug
dom/base/nsWrapperCache.h
dom/base/nsWrapperCacheInlines.h
dom/workers/DOMBindingBase.cpp
dom/workers/DOMBindingBase.h
--- a/dom/base/nsWrapperCache.h
+++ b/dom/base/nsWrapperCache.h
@@ -60,17 +60,17 @@ class DOMBindingBase;
  */
 class nsWrapperCache
 {
   friend class mozilla::dom::workers::DOMBindingBase;
 
 public:
   NS_DECLARE_STATIC_IID_ACCESSOR(NS_WRAPPERCACHE_IID)
 
-  nsWrapperCache() : mWrapperPtrBits(0)
+  nsWrapperCache() : mWrapper(nullptr), mFlags(0)
   {
   }
   ~nsWrapperCache()
   {
     MOZ_ASSERT(!PreservingWrapper(),
                "Destroying cache with a preserved wrapper!");
   }
 
@@ -89,74 +89,74 @@ public:
    * object returned is not guaranteed to be kept alive past the next CC.
    *
    * This should only be called if you are certain that the return value won't
    * be passed into a JS API function and that it won't be stored without being
    * rooted (or otherwise signaling the stored value to the CC).
    */
   JSObject* GetWrapperPreserveColor() const
   {
-    return GetJSObjectFromBits();
+    return GetWrapperJSObject();
   }
 
   void SetWrapper(JSObject* aWrapper)
   {
     MOZ_ASSERT(!PreservingWrapper(), "Clearing a preserved wrapper!");
     MOZ_ASSERT(aWrapper, "Use ClearWrapper!");
 
-    SetWrapperBits(aWrapper);
+    SetWrapperJSObject(aWrapper);
   }
 
   /**
    * Clear the wrapper. This should be called from the finalizer for the
    * wrapper.
    */
   void ClearWrapper()
   {
     MOZ_ASSERT(!PreservingWrapper(), "Clearing a preserved wrapper!");
 
-    SetWrapperBits(NULL);
+    SetWrapperJSObject(nullptr);
   }
 
   bool PreservingWrapper()
   {
-    return (mWrapperPtrBits & WRAPPER_BIT_PRESERVED) != 0;
+    return HasWrapperFlag(WRAPPER_BIT_PRESERVED);
   }
 
   void SetIsDOMBinding()
   {
-    MOZ_ASSERT(!mWrapperPtrBits,
+    MOZ_ASSERT(!mWrapper && !GetWrapperFlags(),
                "This flag should be set before creating any wrappers.");
-    mWrapperPtrBits = WRAPPER_IS_DOM_BINDING;
+    SetWrapperFlags(WRAPPER_IS_DOM_BINDING);
   }
 
   bool IsDOMBinding() const
   {
-    return (mWrapperPtrBits & WRAPPER_IS_DOM_BINDING) != 0;
+    return HasWrapperFlag(WRAPPER_IS_DOM_BINDING);
   }
 
   void SetHasSystemOnlyWrapper()
   {
     MOZ_ASSERT(GetWrapperPreserveColor(),
                "This flag should be set after wrapper creation.");
     MOZ_ASSERT(IsDOMBinding(),
                "This flag should only be set for DOM bindings.");
-    mWrapperPtrBits |= WRAPPER_HAS_SOW;
+    SetWrapperFlags(WRAPPER_HAS_SOW);
   }
 
   bool HasSystemOnlyWrapper() const
   {
-    return (mWrapperPtrBits & WRAPPER_HAS_SOW) != 0;
+    return HasWrapperFlag(WRAPPER_HAS_SOW);
   }
 
   /**
    * Wrap the object corresponding to this wrapper cache. If non-null is
    * returned, the object has already been stored in the wrapper cache.
    */
-  virtual JSObject* WrapObject(JSContext *cx, JS::Handle<JSObject*> scope)
+  virtual JSObject* WrapObject(JSContext* cx, JS::Handle<JSObject*> scope)
   {
     MOZ_ASSERT(!IsDOMBinding(), "Someone forgot to override WrapObject");
     return nullptr;
   }
 
   /**
    * Returns true if the object has a non-gray wrapper.
    */
@@ -167,47 +167,94 @@ public:
    * and all the GC things it is keeping alive are black too.
    */
   bool IsBlackAndDoesNotNeedTracing(nsISupports* aThis);
 
   // Only meant to be called by code that preserves a wrapper.
   void SetPreservingWrapper(bool aPreserve)
   {
     if(aPreserve) {
-      mWrapperPtrBits |= WRAPPER_BIT_PRESERVED;
+      SetWrapperFlags(WRAPPER_BIT_PRESERVED);
     }
     else {
-      mWrapperPtrBits &= ~WRAPPER_BIT_PRESERVED;
+      UnsetWrapperFlags(WRAPPER_BIT_PRESERVED);
     }
   }
 
   void TraceWrapper(const TraceCallbacks& aCallbacks, void* aClosure)
   {
-    if (PreservingWrapper()) {
-      JSObject *wrapper = GetWrapperPreserveColor();
-      if (wrapper) {
-        uintptr_t flags = mWrapperPtrBits & kWrapperBitMask;
-        aCallbacks.Trace(&wrapper, "Preserved wrapper", aClosure);
-        mWrapperPtrBits = reinterpret_cast<uintptr_t>(wrapper) | flags;
-      }
+    if (PreservingWrapper() && mWrapper) {
+        aCallbacks.Trace(&mWrapper, "Preserved wrapper", aClosure);
     }
   }
 
+  /* 
+   * The following methods for getting and manipulating flags allow the unused
+   * bits of mFlags to be used by derived classes.
+   */
+
+  uint32_t GetFlags() const
+  {
+    return mFlags & ~kWrapperFlagsMask;
+  }
+
+  bool HasFlag(uint32_t aFlag) const
+  {
+    MOZ_ASSERT((aFlag & kWrapperFlagsMask) == 0, "Bad flag mask");
+    return !!(mFlags & aFlag);
+  }
+
+  void SetFlags(uint32_t aFlagsToSet)
+  {
+    MOZ_ASSERT((aFlagsToSet & kWrapperFlagsMask) == 0, "Bad flag mask");
+    mFlags |= aFlagsToSet;
+  }
+
+  void UnsetFlags(uint32_t aFlagsToUnset)
+  {
+    MOZ_ASSERT((aFlagsToUnset & kWrapperFlagsMask) == 0, "Bad flag mask");
+    mFlags &= ~aFlagsToUnset;
+  }
+
 private:
-  JSObject *GetJSObjectFromBits() const
+  JSObject *GetWrapperJSObject() const
   {
-    return reinterpret_cast<JSObject*>(mWrapperPtrBits & ~kWrapperBitMask);
+    return mWrapper;
   }
-  void SetWrapperBits(void *aWrapper)
+
+  void SetWrapperJSObject(JSObject* aWrapper)
   {
-    mWrapperPtrBits = reinterpret_cast<uintptr_t>(aWrapper) |
-                      (mWrapperPtrBits & WRAPPER_IS_DOM_BINDING);
+    mWrapper = aWrapper;
+    UnsetWrapperFlags(kWrapperFlagsMask & ~WRAPPER_IS_DOM_BINDING);
+  }
+
+  void TraceWrapperJSObject(JSTracer* aTrc, const char* aName);
+
+  uint32_t GetWrapperFlags() const
+  {
+    return mFlags & kWrapperFlagsMask;
   }
 
-  void TraceJSObjectFromBits(JSTracer *aTrc, const char *aName);
+  bool HasWrapperFlag(uint32_t aFlag) const
+  {
+    MOZ_ASSERT((aFlag & ~kWrapperFlagsMask) == 0, "Bad wrapper flag bits");
+    return !!(mFlags & aFlag);
+  }
+
+  void SetWrapperFlags(uint32_t aFlagsToSet)
+  {
+    MOZ_ASSERT((aFlagsToSet & ~kWrapperFlagsMask) == 0, "Bad wrapper flag bits");
+    mFlags |= aFlagsToSet;
+  }
+
+  void UnsetWrapperFlags(uint32_t aFlagsToUnset)
+  {
+    MOZ_ASSERT((aFlagsToUnset & ~kWrapperFlagsMask) == 0, "Bad wrapper flag bits");
+    mFlags &= ~aFlagsToUnset;
+  }
 
   /**
    * If this bit is set then we're preserving the wrapper, which in effect ties
    * the lifetime of the JS object stored in the cache to the lifetime of the
    * native object. We rely on the cycle collector to break the cycle that this
    * causes between the native object and the JS object, so it is important that
    * any native object that supports preserving of its wrapper
    * traces/traverses/unlinks the cached JS object (see
@@ -225,22 +272,25 @@ private:
 
   /**
    * If this bit is set then the wrapper for the native object is a DOM binding
    * (regular JS object or proxy) that has a system only wrapper for same-origin
    * access.
    */
   enum { WRAPPER_HAS_SOW = 1 << 2 };
 
-  enum { kWrapperBitMask = (WRAPPER_BIT_PRESERVED | WRAPPER_IS_DOM_BINDING |
-                            WRAPPER_HAS_SOW) };
+  enum { kWrapperFlagsMask = (WRAPPER_BIT_PRESERVED | WRAPPER_IS_DOM_BINDING |
+                              WRAPPER_HAS_SOW) };
 
-  uintptr_t mWrapperPtrBits;
+  JSObject* mWrapper;
+  uint32_t  mFlags;
 };
 
+enum { WRAPPER_CACHE_FLAGS_BITS_USED = 3 };
+
 NS_DEFINE_STATIC_IID_ACCESSOR(nsWrapperCache, NS_WRAPPERCACHE_IID)
 
 #define NS_WRAPPERCACHE_INTERFACE_MAP_ENTRY                                   \
   if ( aIID.Equals(NS_GET_IID(nsWrapperCache)) ) {                            \
     *aInstancePtr = static_cast<nsWrapperCache*>(this);                       \
     return NS_OK;                                                             \
   }
 
--- a/dom/base/nsWrapperCacheInlines.h
+++ b/dom/base/nsWrapperCacheInlines.h
@@ -5,26 +5,16 @@
 
 #ifndef nsWrapperCacheInline_h___
 #define nsWrapperCacheInline_h___
 
 #include "nsWrapperCache.h"
 #include "xpcpublic.h"
 #include "jsapi.h"
 
-// We want to encode 3 bits into mWrapperPtrBits, so anything we store in it
-// needs to be aligned on 8 byte boundaries.
-// JS arenas are aligned on 4k boundaries and padded so that the array of
-// JSObjects ends on the end of the arena. If the size of a JSObject is a
-// multiple of 8 then the start of every JSObject in an arena should be aligned
-// on 8 byte boundaries.
-MOZ_STATIC_ASSERT(sizeof(js::shadow::Object) % 8 == 0 && sizeof(JS::Value) == 8,
-                  "We want to rely on JSObject being aligned on 8 byte "
-                  "boundaries.");
-
 inline JSObject*
 nsWrapperCache::GetWrapper() const
 {
     JSObject* obj = GetWrapperPreserveColor();
     xpc_UnmarkGrayObject(obj);
     return obj;
 }
 
@@ -53,14 +43,14 @@ nsWrapperCache::IsBlackAndDoesNotNeedTra
     bool hasGrayObjects = false;
     participant->Trace(aThis, TraceCallbackFunc(SearchGray), &hasGrayObjects);
     return !hasGrayObjects;
   }
   return false;
 }
 
 inline void
-nsWrapperCache::TraceJSObjectFromBits(JSTracer* aTrc, const char* aName)
+nsWrapperCache::TraceWrapperJSObject(JSTracer* aTrc, const char* aName)
 {
-  JS_CallMaskedObjectTracer(aTrc, &mWrapperPtrBits, kWrapperBitMask, aName);
+  JS_CallObjectTracer(aTrc, &mWrapper, aName);
 }
 
 #endif /* nsWrapperCache_h___ */
--- a/dom/workers/DOMBindingBase.cpp
+++ b/dom/workers/DOMBindingBase.cpp
@@ -55,26 +55,26 @@ DOMBindingBase::GetJSContext() const {
 }
 
 #ifdef DEBUG
 JSObject*
 DOMBindingBase::GetJSObject() const
 {
   // Make sure that the public method results in the same bits as our private
   // method.
-  MOZ_ASSERT(GetJSObjectFromBits() == GetWrapperPreserveColor());
-  return GetJSObjectFromBits();
+  MOZ_ASSERT(GetWrapperJSObject() == GetWrapperPreserveColor());
+  return GetWrapperJSObject();
 }
 
 void
 DOMBindingBase::SetJSObject(JSObject* aObject)
 {
   // Make sure that the public method results in the same bits as our private
   // method.
   SetWrapper(aObject);
 
-  uintptr_t oldWrapperPtrBits = mWrapperPtrBits;
+  uint8_t oldFlags = mFlags;
 
-  SetWrapperBits(aObject);
+  SetWrapperJSObject(aObject);
 
-  MOZ_ASSERT(oldWrapperPtrBits == mWrapperPtrBits);
+  MOZ_ASSERT(oldFlags == mFlags && aObject == mWrapper);
 }
 #endif
--- a/dom/workers/DOMBindingBase.h
+++ b/dom/workers/DOMBindingBase.h
@@ -44,39 +44,38 @@ public:
   NS_DECL_ISUPPORTS
 
   JSContext*
   GetJSContext() const;
 
   void
   TraceJSObject(JSTracer* aTrc, const char* aName)
   {
-      if (GetJSObject())
-          TraceJSObjectFromBits(aTrc, aName);
+    if (GetJSObject()) {
+      TraceWrapperJSObject(aTrc, aName);
+    }
   }
 
 #ifdef DEBUG
   JSObject*
   GetJSObject() const;
 
   void
   SetJSObject(JSObject* aObject);
 #else
   JSObject*
   GetJSObject() const
   {
-    // Reach in and grab the bits directly.
-    return GetJSObjectFromBits();
+    return GetWrapperJSObject();
   }
 
   void
   SetJSObject(JSObject* aObject)
   {
-    // Set the bits directly.
-    SetWrapperBits(aObject);
+    SetWrapperJSObject(aObject);
   }
 #endif
 };
 
 template <class T>
 class DOMBindingAnchor
 {
   T* mBinding;