Bug 1047120 - PurpleBuffer doesn't actually need to use Heap<T>; r=mccr8,jonco
authorTerrence Cole <terrence@mozilla.com>
Thu, 31 Jul 2014 14:43:45 -0700
changeset 198366 05c37bb1fc0354189c6477456d01cf9913973d1b
parent 198365 7cc3f4f2a4e80da073437e4f419a91310e732106
child 198367 5c73b82d77235f7395696af6fb0b64702178a445
push id27268
push userryanvm@gmail.com
push dateThu, 07 Aug 2014 21:20:21 +0000
treeherdermozilla-central@9242bc34ac5c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmccr8, jonco
bugs1047120
milestone34.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 1047120 - PurpleBuffer doesn't actually need to use Heap<T>; r=mccr8,jonco
js/public/RootingAPI.h
js/public/Value.h
xpcom/base/nsCycleCollector.cpp
--- a/js/public/RootingAPI.h
+++ b/js/public/RootingAPI.h
@@ -3,16 +3,17 @@
  * This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #ifndef js_RootingAPI_h
 #define js_RootingAPI_h
 
 #include "mozilla/Attributes.h"
+#include "mozilla/DebugOnly.h"
 #include "mozilla/GuardObjects.h"
 #include "mozilla/LinkedList.h"
 #include "mozilla/NullPtr.h"
 #include "mozilla/TypeTraits.h"
 
 #include "jspubtd.h"
 
 #include "js/GCAPI.h"
@@ -161,16 +162,33 @@ JS_FRIEND_API(bool) isGCEnabled();
  *   foo(JS::NullPtr());
  * which avoids creating a Rooted<JSObject*> just to pass nullptr.
  */
 struct JS_PUBLIC_API(NullPtr)
 {
     static void * const constNullValue;
 };
 
+#ifdef JSGC_GENERATIONAL
+JS_FRIEND_API(void) HeapCellPostBarrier(js::gc::Cell **cellp);
+JS_FRIEND_API(void) HeapCellRelocate(js::gc::Cell **cellp);
+#endif
+
+#ifdef JS_DEBUG
+/*
+ * For generational GC, assert that an object is in the tenured generation as
+ * opposed to being in the nursery.
+ */
+extern JS_FRIEND_API(void)
+AssertGCThingMustBeTenured(JSObject* obj);
+#else
+inline void
+AssertGCThingMustBeTenured(JSObject *obj) {}
+#endif
+
 /*
  * The Heap<T> class is a heap-stored reference to a JS GC thing. All members of
  * heap classes that refer to GC things should use Heap<T> (or possibly
  * TenuredHeap<T>, described below).
  *
  * Heap<T> is an abstraction that hides some of the complexity required to
  * maintain GC invariants for the contained reference. It uses operator
  * overloading to provide a normal pointer interface, but notifies the GC every
@@ -280,28 +298,16 @@ class Heap : public js::HeapBase<T>
 
     enum {
         crashOnTouchPointer = 1
     };
 
     T ptr;
 };
 
-#ifdef JS_DEBUG
-/*
- * For generational GC, assert that an object is in the tenured generation as
- * opposed to being in the nursery.
- */
-extern JS_FRIEND_API(void)
-AssertGCThingMustBeTenured(JSObject* obj);
-#else
-inline void
-AssertGCThingMustBeTenured(JSObject *obj) {}
-#endif
-
 /*
  * The TenuredHeap<T> class is similar to the Heap<T> class above in that it
  * encapsulates the GC concerns of an on-heap reference to a JS object. However,
  * it has two important differences:
  *
  *  1) Pointers which are statically known to only reference "tenured" objects
  *     can avoid the extra overhead of SpiderMonkey's write barriers.
  *
@@ -555,21 +561,16 @@ class MOZ_STACK_CLASS MutableHandle : pu
     MutableHandle() {}
 
     T *ptr;
 
     template <typename S> void operator=(S v) MOZ_DELETE;
     void operator=(MutableHandle other) MOZ_DELETE;
 };
 
-#ifdef JSGC_GENERATIONAL
-JS_FRIEND_API(void) HeapCellPostBarrier(js::gc::Cell **cellp);
-JS_FRIEND_API(void) HeapCellRelocate(js::gc::Cell **cellp);
-#endif
-
 } /* namespace JS */
 
 namespace js {
 
 /*
  * InternalHandle is a handle to an internal pointer into a gcthing. Use
  * InternalHandle when you have a pointer to a direct field of a gcthing, or
  * when you need a parameter type for something that *may* be a pointer to a
@@ -661,16 +662,22 @@ struct GCMethods<T *>
 #endif
 };
 
 template <>
 struct GCMethods<JSObject *>
 {
     static JSObject *initial() { return nullptr; }
     static bool poisoned(JSObject *v) { return JS::IsPoisonedPtr(v); }
+    static gc::Cell *asGCThingOrNull(JSObject *v) {
+        if (!v)
+            return nullptr;
+        JS_ASSERT(uintptr_t(v) > 32);
+        return reinterpret_cast<gc::Cell *>(v);
+    }
     static bool needsPostBarrier(JSObject *v) {
         return v != nullptr && gc::IsInsideNursery(reinterpret_cast<gc::Cell *>(v));
     }
 #ifdef JSGC_GENERATIONAL
     static void postBarrier(JSObject **vp) {
         JS::HeapCellPostBarrier(reinterpret_cast<js::gc::Cell **>(vp));
     }
     static void relocate(JSObject **vp) {
@@ -1241,11 +1248,29 @@ class CompilerRootNode
 
   public:
     CompilerRootNode *next;
 
   protected:
     js::gc::Cell *ptr_;
 };
 
-}  /* namespace js */
+namespace gc {
+
+template <typename T, typename TraceCallbacks>
+void
+CallTraceCallbackOnNonHeap(T *v, const TraceCallbacks &aCallbacks, const char *aName, void *aClosure)
+{
+    static_assert(sizeof(T) == sizeof(JS::Heap<T>), "T and Heap<T> must be compatible.");
+    MOZ_ASSERT(v);
+    mozilla::DebugOnly<Cell *> cell = GCMethods<T>::asGCThingOrNull(*v);
+    MOZ_ASSERT(cell);
+    MOZ_ASSERT(!IsInsideNursery(cell));
+    JS::Heap<T> *asHeapT = reinterpret_cast<JS::Heap<T>*>(v);
+    aCallbacks.Trace(asHeapT, aName, aClosure);
+    MOZ_ASSERT(GCMethods<T>::asGCThingOrNull(*v) == cell);
+}
+
+} /* namespace gc */
+
+} /* namespace js */
 
 #endif  /* js_RootingAPI_h */
--- a/js/public/Value.h
+++ b/js/public/Value.h
@@ -1632,16 +1632,19 @@ template <> struct GCMethods<const JS::V
 };
 
 template <> struct GCMethods<JS::Value>
 {
     static JS::Value initial() { return JS::UndefinedValue(); }
     static bool poisoned(const JS::Value &v) {
         return v.isMarkable() && JS::IsPoisonedPtr(v.toGCThing());
     }
+    static gc::Cell *asGCThingOrNull(const JS::Value &v) {
+        return v.isMarkable() ? v.toGCThing() : nullptr;
+    }
     static bool needsPostBarrier(const JS::Value &v) {
         return v.isObject() && gc::IsInsideNursery(reinterpret_cast<gc::Cell*>(&v.toObject()));
     }
 #ifdef JSGC_GENERATIONAL
     static void postBarrier(JS::Value *v) { JS::HeapValuePostBarrier(v); }
     static void relocate(JS::Value *v) { JS::HeapValueRelocate(v); }
 #endif
 };
--- a/xpcom/base/nsCycleCollector.cpp
+++ b/xpcom/base/nsCycleCollector.cpp
@@ -2516,73 +2516,75 @@ private:
 // If GC happens before CC, references to GCthings and the self reference are
 // removed.
 class JSPurpleBuffer
 {
   ~JSPurpleBuffer()
   {
     MOZ_ASSERT(mValues.IsEmpty());
     MOZ_ASSERT(mObjects.IsEmpty());
-    MOZ_ASSERT(mTenuredObjects.IsEmpty());
   }
 
 public:
   explicit JSPurpleBuffer(JSPurpleBuffer*& aReferenceToThis)
     : mReferenceToThis(aReferenceToThis)
   {
     mReferenceToThis = this;
     NS_ADDREF_THIS();
     mozilla::HoldJSObjects(this);
   }
 
   void Destroy()
   {
     mReferenceToThis = nullptr;
     mValues.Clear();
     mObjects.Clear();
-    mTenuredObjects.Clear();
     mozilla::DropJSObjects(this);
     NS_RELEASE_THIS();
   }
 
   NS_INLINE_DECL_CYCLE_COLLECTING_NATIVE_REFCOUNTING(JSPurpleBuffer)
   NS_DECL_CYCLE_COLLECTION_SCRIPT_HOLDER_NATIVE_CLASS(JSPurpleBuffer)
 
   JSPurpleBuffer*& mReferenceToThis;
-  SegmentedArray<JS::Heap<JS::Value>> mValues;
-  SegmentedArray<JS::Heap<JSObject*>> mObjects;
-  SegmentedArray<JS::TenuredHeap<JSObject*>> mTenuredObjects;
+
+  // These are raw pointers instead of Heap<T> because we only need Heap<T> for
+  // pointers which may point into the nursery. The purple buffer never contains
+  // pointers to the nursery because nursery gcthings can never be gray and only
+  // gray things can be inserted into the purple buffer.
+  SegmentedArray<JS::Value> mValues;
+  SegmentedArray<JSObject*> mObjects;
 };
 
 NS_IMPL_CYCLE_COLLECTION_CLASS(JSPurpleBuffer)
 
 NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(JSPurpleBuffer)
   tmp->Destroy();
 NS_IMPL_CYCLE_COLLECTION_UNLINK_END
 
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(JSPurpleBuffer)
   CycleCollectionNoteChild(cb, tmp, "self");
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE_SCRIPT_OBJECTS
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
 
-#define NS_TRACE_SEGMENTED_ARRAY(_field)                                       \
-    {                                                                          \
-        auto segment = tmp->_field.GetFirstSegment();                          \
-        while (segment) {                                                      \
-            for (uint32_t i = segment->Length(); i > 0;) {                     \
-                aCallbacks.Trace(&segment->ElementAt(--i), #_field, aClosure); \
-            }                                                                  \
-            segment = segment->getNext();                                      \
-        }                                                                      \
-    }
+#define NS_TRACE_SEGMENTED_ARRAY(_field, _type)                                \
+  {                                                                            \
+    auto segment = tmp->_field.GetFirstSegment();                              \
+    while (segment) {                                                          \
+      for (uint32_t i = segment->Length(); i > 0;) {                           \
+        js::gc::CallTraceCallbackOnNonHeap<_type, TraceCallbacks>(             \
+            &segment->ElementAt(--i), aCallbacks, #_field, aClosure);          \
+      }                                                                        \
+      segment = segment->getNext();                                            \
+    }                                                                          \
+  }
 
 NS_IMPL_CYCLE_COLLECTION_TRACE_BEGIN(JSPurpleBuffer)
-  NS_TRACE_SEGMENTED_ARRAY(mValues)
-  NS_TRACE_SEGMENTED_ARRAY(mObjects)
-  NS_TRACE_SEGMENTED_ARRAY(mTenuredObjects)
+  NS_TRACE_SEGMENTED_ARRAY(mValues, JS::Value)
+  NS_TRACE_SEGMENTED_ARRAY(mObjects, JSObject*)
 NS_IMPL_CYCLE_COLLECTION_TRACE_END
 
 NS_IMPL_CYCLE_COLLECTION_ROOT_NATIVE(JSPurpleBuffer, AddRef)
 NS_IMPL_CYCLE_COLLECTION_UNROOT_NATIVE(JSPurpleBuffer, Release)
 
 struct SnowWhiteObject
 {
   void* mPointer;
@@ -2639,43 +2641,49 @@ public:
   bool HasSnowWhiteObjects() const
   {
     return mObjects.Length() > 0;
   }
 
   virtual void Trace(JS::Heap<JS::Value>* aValue, const char* aName,
                      void* aClosure) const
   {
-    if (aValue->isMarkable()) {
-      void* thing = aValue->toGCThing();
+    JS::Value val = *aValue;
+    if (val.isMarkable()) {
+      void* thing = val.toGCThing();
       if (thing && xpc_GCThingIsGrayCCThing(thing)) {
-        mCollector->GetJSPurpleBuffer()->mValues.AppendElement(*aValue);
+        MOZ_ASSERT(!js::gc::IsInsideNursery((js::gc::Cell *)thing));
+        mCollector->GetJSPurpleBuffer()->mValues.AppendElement(val);
       }
     }
   }
 
   virtual void Trace(JS::Heap<jsid>* aId, const char* aName,
                      void* aClosure) const
   {
   }
 
+  void AppendJSObjectToPurpleBuffer(JSObject *obj) const
+  {
+    if (obj && xpc_GCThingIsGrayCCThing(obj)) {
+      MOZ_ASSERT(!js::gc::IsInsideNursery(JS::AsCell(obj)));
+      mCollector->GetJSPurpleBuffer()->mObjects.AppendElement(obj);
+    }
+  }
+
   virtual void Trace(JS::Heap<JSObject*>* aObject, const char* aName,
                      void* aClosure) const
   {
-    if (*aObject && xpc_GCThingIsGrayCCThing(*aObject)) {
-      mCollector->GetJSPurpleBuffer()->mObjects.AppendElement(*aObject);
-    }
+    AppendJSObjectToPurpleBuffer(*aObject);
   }
 
   virtual void Trace(JS::TenuredHeap<JSObject*>* aObject, const char* aName,
                      void* aClosure) const
   {
-    if (*aObject && xpc_GCThingIsGrayCCThing(*aObject)) {
-      mCollector->GetJSPurpleBuffer()->mTenuredObjects.AppendElement(*aObject);
-    }
+    AppendJSObjectToPurpleBuffer(*aObject);
   }
 
   virtual void Trace(JS::Heap<JSString*>* aString, const char* aName,
                      void* aClosure) const
   {
   }
 
   virtual void Trace(JS::Heap<JSScript*>* aScript, const char* aName,
@@ -3812,16 +3820,18 @@ nsCycleCollector::SizeOfIncludingThis(mo
   // - mJSRuntime: because it's non-owning and measured by JS reporters.
   // - mParams: because it only contains scalars.
 }
 
 JSPurpleBuffer*
 nsCycleCollector::GetJSPurpleBuffer()
 {
   if (!mJSPurpleBuffer) {
+    // The Release call here confuses the GC analysis.
+    JS::AutoSuppressGCAnalysis nogc;
     // JSPurpleBuffer keeps itself alive, but we need to create it in such way
     // that it ends up in the normal purple buffer. That happens when
     // nsRefPtr goes out of the scope and calls Release.
     nsRefPtr<JSPurpleBuffer> pb = new JSPurpleBuffer(mJSPurpleBuffer);
   }
   return mJSPurpleBuffer;
 }