Bug 1188197 - Allow PersistentRooted to store DynamicTraceable; r=sfink
☠☠ backed out by 35e191320db8 ☠ ☠
authorTerrence Cole <terrence@mozilla.com>
Fri, 24 Jul 2015 15:09:28 -0700
changeset 287001 54a082b0117453c81bb53c118576546a26178e4d
parent 287000 6444888e596cf5c8c7036a150cbbdc85494e7cde
child 287002 a5c748f78e97f11a3354ce7142bad4a11c02fafc
push id5067
push userraliiev@mozilla.com
push dateMon, 21 Sep 2015 14:04:52 +0000
treeherdermozilla-beta@14221ffe5b2f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssfink
bugs1188197
milestone42.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 1188197 - Allow PersistentRooted to store DynamicTraceable; r=sfink
js/public/RootingAPI.h
js/src/gc/RootMarking.cpp
js/src/jsapi-tests/testGCExactRooting.cpp
js/src/jsgc.cpp
--- a/js/public/RootingAPI.h
+++ b/js/public/RootingAPI.h
@@ -992,16 +992,20 @@ MutableHandle<T>::MutableHandle(Persiste
  * TenuredHeap<T> would be better types. It's up to the implementor of the type
  * containing Heap<T> or TenuredHeap<T> members to make sure their referents get
  * marked when the object itself is marked.
  */
 template<typename T>
 class PersistentRooted : public js::PersistentRootedBase<T>,
                          private mozilla::LinkedListElement<PersistentRooted<T>>
 {
+    static_assert(!mozilla::IsConvertible<T, StaticTraceable*>::value &&
+                  !mozilla::IsConvertible<T, DynamicTraceable*>::value,
+                  "Rooted takes pointer or Traceable types but not Traceable* type");
+
     typedef mozilla::LinkedListElement<PersistentRooted<T>> ListBase;
 
     friend class mozilla::LinkedList<PersistentRooted>;
     friend class mozilla::LinkedListElement<PersistentRooted>;
 
     friend struct js::gc::PersistentRootedMarker<T>;
 
     friend void js::gc::FinishPersistentRootedChains(js::RootLists&);
@@ -1011,17 +1015,18 @@ class PersistentRooted : public js::Pers
         js::ThingRootKind kind = js::RootKind<T>::rootKind();
         roots.heapRoots_[kind].insertBack(reinterpret_cast<JS::PersistentRooted<void*>*>(this));
         // Until marking and destruction support the full set, we assert that
         // we don't try to add any unsupported types.
         MOZ_ASSERT(kind == js::THING_ROOT_OBJECT ||
                    kind == js::THING_ROOT_SCRIPT ||
                    kind == js::THING_ROOT_STRING ||
                    kind == js::THING_ROOT_ID ||
-                   kind == js::THING_ROOT_VALUE);
+                   kind == js::THING_ROOT_VALUE ||
+                   kind == js::THING_ROOT_DYNAMIC_TRACEABLE);
     }
 
   public:
     PersistentRooted() : ptr(js::GCMethods<T>::initial()) {}
 
     template <typename RootingContext>
     explicit PersistentRooted(const RootingContext& cx)
       : ptr(js::GCMethods<T>::initial())
--- a/js/src/gc/RootMarking.cpp
+++ b/js/src/gc/RootMarking.cpp
@@ -299,23 +299,24 @@ JSPropertyDescriptor::trace(JSTracer* tr
 namespace js {
 namespace gc {
 
 template<typename T>
 struct PersistentRootedMarker
 {
     typedef PersistentRooted<T> Element;
     typedef mozilla::LinkedList<Element> List;
-    typedef void (*MarkFunc)(JSTracer* trc, T* ref, const char* name);
+    using TraceFunc = void (*)(JSTracer* trc, T* ref, const char* name);
 
+    template <TraceFunction<T> TraceFn = TraceNullableRoot>
     static void
     markChain(JSTracer* trc, List& list, const char* name)
     {
         for (Element* r = list.getFirst(); r; r = r->getNext())
-            TraceNullableRoot(trc, r->address(), name);
+            TraceFn(trc, r->address(), name);
     }
 };
 
 } // namespace gc
 } // namespace js
 
 void
 js::gc::MarkPersistentRootedChainsInLists(RootLists& roots, JSTracer* trc)
@@ -326,16 +327,21 @@ js::gc::MarkPersistentRootedChainsInList
                                                  "PersistentRooted<JSScript*>");
     PersistentRootedMarker<JSString*>::markChain(trc, roots.getPersistentRootedList<JSString*>(),
                                                  "PersistentRooted<JSString*>");
 
     PersistentRootedMarker<jsid>::markChain(trc, roots.getPersistentRootedList<jsid>(),
                                             "PersistentRooted<jsid>");
     PersistentRootedMarker<Value>::markChain(trc, roots.getPersistentRootedList<Value>(),
                                              "PersistentRooted<Value>");
+
+    PersistentRootedMarker<ConcreteTraceable>::markChain<MarkDynamicTraceable>(trc,
+        reinterpret_cast<mozilla::LinkedList<JS::PersistentRooted<ConcreteTraceable>>&>(
+            roots.heapRoots_[THING_ROOT_DYNAMIC_TRACEABLE]),
+        "PersistentRooted<Value>");
 }
 
 void
 js::gc::MarkPersistentRootedChains(JSTracer* trc)
 {
     for (ContextIter cx(trc->runtime()); !cx.done(); cx.next())
         MarkPersistentRootedChainsInLists(cx->roots, trc);
     MarkPersistentRootedChainsInLists(trc->runtime()->mainThread.roots, trc);
--- a/js/src/jsapi-tests/testGCExactRooting.cpp
+++ b/js/src/jsapi-tests/testGCExactRooting.cpp
@@ -118,16 +118,25 @@ struct DynamicContainer : public Dynamic
 };
 
 namespace js {
 template <>
 struct RootedBase<DynamicContainer> {
     RelocatablePtrObject& obj() { return static_cast<Rooted<DynamicContainer>*>(this)->get().obj; }
     RelocatablePtrString& str() { return static_cast<Rooted<DynamicContainer>*>(this)->get().str; }
 };
+template <>
+struct PersistentRootedBase<DynamicContainer> {
+    RelocatablePtrObject& obj() {
+        return static_cast<PersistentRooted<DynamicContainer>*>(this)->get().obj;
+    }
+    RelocatablePtrString& str() {
+        return static_cast<PersistentRooted<DynamicContainer>*>(this)->get().str;
+    }
+};
 } // namespace js
 
 BEGIN_TEST(testGCRootedDynamicStructInternalStackStorage)
 {
     JS::Rooted<DynamicContainer> container(cx);
     container.get().obj = JS_NewObject(cx, nullptr);
     container.get().str = JS_NewStringCopyZ(cx, "Hello");
 
@@ -148,16 +157,50 @@ BEGIN_TEST(testGCRootedDynamicStructInte
     container.str() = JS_NewStringCopyZ(cx, "Hello");
 
     JS_GC(cx->runtime());
     JS_GC(cx->runtime());
 
     JS::RootedObject obj(cx, container.obj());
     JS::RootedValue val(cx, StringValue(container.str()));
     CHECK(JS_SetProperty(cx, obj, "foo", val));
+    obj = nullptr;
+    val = UndefinedValue();
+
+    {
+        JS::RootedString actual(cx);
+        bool same;
+
+        // Automatic move from stack to heap.
+        JS::PersistentRooted<DynamicContainer> heap(cx, container);
+
+        // clear prior rooting.
+        container.obj() = nullptr;
+        container.str() = nullptr;
+
+        obj = heap.obj();
+        CHECK(JS_GetProperty(cx, obj, "foo", &val));
+        actual = val.toString();
+        CHECK(JS_StringEqualsAscii(cx, actual, "Hello", &same));
+        CHECK(same);
+        obj = nullptr;
+        actual = nullptr;
+
+        JS_GC(cx->runtime());
+        JS_GC(cx->runtime());
+
+        obj = heap.obj();
+        CHECK(JS_GetProperty(cx, obj, "foo", &val));
+        actual = val.toString();
+        CHECK(JS_StringEqualsAscii(cx, actual, "Hello", &same));
+        CHECK(same);
+        obj = nullptr;
+        actual = nullptr;
+    }
+
     return true;
 }
 END_TEST(testGCRootedDynamicStructInternalStackStorageAugmented)
 
 using MyHashMap = js::TraceableHashMap<js::Shape*, JSObject*>;
 
 BEGIN_TEST(testGCRootedHashMap)
 {
@@ -227,16 +270,26 @@ BEGIN_TEST(testGCHandleHashMap)
 
     CHECK(FillMyHashMap(cx, &map));
 
     JS_GC(rt);
     JS_GC(rt);
 
     CHECK(CheckMyHashMap(cx, map));
 
+    // Unfortunately, the type of get() needs to be non-const ref, so
+    // we need to explicitly Move the storage.
+    JS::PersistentRooted<MyHashMap> heapMap(cx, mozilla::Move(map.get()));
+    CHECK(CheckMyHashMap(cx, heapMap));
+
+    JS_GC(rt);
+    JS_GC(rt);
+
+    CHECK(CheckMyHashMap(cx, heapMap));
+
     return true;
 }
 END_TEST(testGCHandleHashMap)
 
 BEGIN_TEST(testGCRootedVector)
 {
     using ShapeVec = TraceableVector<Shape*>;
     JS::Rooted<ShapeVec> shapes(cx, ShapeVec(cx));
--- a/js/src/jsgc.cpp
+++ b/js/src/jsgc.cpp
@@ -1366,16 +1366,17 @@ FinishPersistentRootedChain(mozilla::Lin
 void
 js::gc::FinishPersistentRootedChains(RootLists& roots)
 {
     FinishPersistentRootedChain(roots.getPersistentRootedList<JSObject*>());
     FinishPersistentRootedChain(roots.getPersistentRootedList<JSScript*>());
     FinishPersistentRootedChain(roots.getPersistentRootedList<JSString*>());
     FinishPersistentRootedChain(roots.getPersistentRootedList<jsid>());
     FinishPersistentRootedChain(roots.getPersistentRootedList<Value>());
+    FinishPersistentRootedChain(roots.heapRoots_[THING_ROOT_DYNAMIC_TRACEABLE]);
 }
 
 void
 GCRuntime::finishRoots()
 {
     if (rootsHash.initialized())
         rootsHash.clear();