Bug 1189809 - Remove the ill-fated DynamicTraceable; r=jonco
authorTerrence Cole <terrence@mozilla.com>
Fri, 31 Jul 2015 08:27:12 -0700
changeset 287603 b05f6533036ef582590878548755d9b8753da24d
parent 287602 8a2ab7f7e7379aaf946c284c6f3e66f5f1faa06f
child 287604 0aed5c00a7354bf60deaf8f11a6e0f05c9c51bc8
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)
reviewersjonco
bugs1189809
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 1189809 - Remove the ill-fated DynamicTraceable; r=jonco
js/public/RootingAPI.h
js/public/TraceableHashTable.h
js/public/TraceableVector.h
js/src/gc/RootMarking.cpp
js/src/jsapi-tests/testGCExactRooting.cpp
js/src/jsapi.h
js/src/jspubtd.h
js/src/jsscript.h
js/src/vm/Stack.h
js/src/vm/TypeInference.h
--- a/js/public/RootingAPI.h
+++ b/js/public/RootingAPI.h
@@ -562,47 +562,36 @@ struct GCMethods<JSFunction*>
                                   reinterpret_cast<JSObject*>(next));
     }
 };
 
 } /* namespace js */
 
 namespace JS {
 
-// If a class containing GC pointers has (or can gain) a vtable, then it can be
-// trivially used with Rooted/Handle/MutableHandle by deriving from
-// DynamicTraceable and overriding |void trace(JSTracer*)|.
-class DynamicTraceable
+// Non pointer types -- structs or classes that contain GC pointers, either as
+// a member or in a more complex container layout -- can also be stored in a
+// [Persistent]Rooted if it derives from JS::Traceable. A JS::Traceable stored
+// in a [Persistent]Rooted must implement the method:
+//     |static void trace(T*, JSTracer*)|
+class Traceable
 {
   public:
-    static js::ThingRootKind rootKind() { return js::THING_ROOT_DYNAMIC_TRACEABLE; }
-
-    virtual ~DynamicTraceable() {}
-    virtual void trace(JSTracer* trc) = 0;
-};
-
-// To use a static class or struct (e.g. not containing a vtable) as part of a
-// Rooted/Handle/MutableHandle, ensure that it derives from StaticTraceable
-// (i.e. so that automatic upcast via calls works) and ensure that a method
-// |static void trace(T*, JSTracer*)| exists on the class.
-class StaticTraceable
-{
-  public:
-    static js::ThingRootKind rootKind() { return js::THING_ROOT_STATIC_TRACEABLE; }
+    static js::ThingRootKind rootKind() { return js::THING_ROOT_TRACEABLE; }
 };
 
 } /* namespace JS */
 
 namespace js {
 
 template <typename T>
 class DispatchWrapper
 {
-    static_assert(mozilla::IsBaseOf<JS::StaticTraceable, T>::value,
-                  "DispatchWrapper is intended only for usage with a StaticTraceable");
+    static_assert(mozilla::IsBaseOf<JS::Traceable, T>::value,
+                  "DispatchWrapper is intended only for usage with a Traceable");
 
     using TraceFn = void (*)(T*, JSTracer*);
     TraceFn tracer;
 #if JS_BITS_PER_WORD == 32
     uint32_t padding; // Ensure the storage fields have CellSize alignment.
 #endif
     T storage;
 
@@ -615,17 +604,17 @@ class DispatchWrapper
     { }
     T* operator &() { return &storage; }
     const T* operator &() const { return &storage; }
     operator T&() { return storage; }
     operator const T&() const { return storage; }
 
     // Trace the contained storage (of unknown type) using the trace function
     // we set aside when we did know the type.
-    static void TraceWrapped(JSTracer* trc, JS::StaticTraceable* thingp, const char* name) {
+    static void TraceWrapped(JSTracer* trc, JS::Traceable* thingp, const char* name) {
         auto wrapper = reinterpret_cast<DispatchWrapper*>(
                            uintptr_t(thingp) - offsetof(DispatchWrapper, storage));
         wrapper->tracer(&wrapper->storage, trc);
     }
 };
 
 inline RootLists&
 RootListsForRootingContext(JSContext* cx)
@@ -661,18 +650,17 @@ namespace JS {
  * function that requires a handle, e.g. Foo(Root<T>(cx, x)).
  *
  * If you want to add additional methods to Rooted for a specific
  * specialization, define a RootedBase<T> specialization containing them.
  */
 template <typename T>
 class MOZ_STACK_CLASS Rooted : public js::RootedBase<T>
 {
-    static_assert(!mozilla::IsConvertible<T, StaticTraceable*>::value &&
-                  !mozilla::IsConvertible<T, DynamicTraceable*>::value,
+    static_assert(!mozilla::IsConvertible<T, Traceable*>::value,
                   "Rooted takes pointer or Traceable types but not Traceable* type");
 
     /* Note: CX is a subclass of either ContextFriendFields or PerThreadDataFriendFields. */
     void registerWithRootLists(js::RootLists& roots) {
         js::ThingRootKind kind = js::RootKind<T>::rootKind();
         this->stack = &roots.stackRoots_[kind];
         this->prev = *stack;
         *stack = reinterpret_cast<Rooted<void*>*>(this);
@@ -725,23 +713,23 @@ class MOZ_STACK_CLASS Rooted : public js
      * stack head pointer for different classes.
      */
     Rooted<void*>** stack;
     Rooted<void*>* prev;
 
     /*
      * For pointer types, the TraceKind for tracing is based on the list it is
      * in (selected via rootKind), so no additional storage is required here.
-     * All StaticTraceable, however, share the same list, so the function to
+     * All Traceable, however, share the same list, so the function to
      * call for tracing is stored adjacent to the struct. Since C++ cannot
      * templatize on storage class, this is implemented via the wrapper class
      * DispatchWrapper.
      */
     using MaybeWrapped = typename mozilla::Conditional<
-        mozilla::IsBaseOf<StaticTraceable, T>::value,
+        mozilla::IsBaseOf<Traceable, T>::value,
         js::DispatchWrapper<T>,
         T>::Type;
     MaybeWrapped ptr;
 
     MOZ_DECL_USE_GUARD_OBJECT_NOTIFIER
 
     Rooted(const Rooted&) = delete;
 };
--- a/js/public/TraceableHashTable.h
+++ b/js/public/TraceableHashTable.h
@@ -29,24 +29,25 @@ template <typename> struct DefaultTracer
 // it must either be used with Rooted, or barriered and traced manually.
 template <typename Key,
           typename Value,
           typename HashPolicy = DefaultHasher<Key>,
           typename AllocPolicy = TempAllocPolicy,
           typename KeyTraceFunc = DefaultTracer<Key>,
           typename ValueTraceFunc = DefaultTracer<Value>>
 class TraceableHashMap : public HashMap<Key, Value, HashPolicy, AllocPolicy>,
-                         public JS::DynamicTraceable
+                         public JS::Traceable
 {
     using Base = HashMap<Key, Value, HashPolicy, AllocPolicy>;
 
   public:
     explicit TraceableHashMap(AllocPolicy a = AllocPolicy()) : Base(a)  {}
 
-    void trace(JSTracer* trc) override {
+    static void trace(TraceableHashMap* map, JSTracer* trc) { map->trace(trc); }
+    void trace(JSTracer* trc) {
         if (!this->initialized())
             return;
         for (typename Base::Enum e(*this); !e.empty(); e.popFront()) {
             ValueTraceFunc::trace(trc, &e.front().value(), "hashmap value");
             Key key = e.front().key();
             KeyTraceFunc::trace(trc, &key, "hashmap key");
             if (key != e.front().key())
                 e.rekeyFront(key);
--- a/js/public/TraceableVector.h
+++ b/js/public/TraceableVector.h
@@ -32,28 +32,29 @@ template <typename T,
           size_t MinInlineCapacity = 0,
           typename AllocPolicy = TempAllocPolicy,
           typename TraceFunc = DefaultTracer<T>>
 class TraceableVector
   : public mozilla::VectorBase<T,
                                MinInlineCapacity,
                                AllocPolicy,
                                TraceableVector<T, MinInlineCapacity, AllocPolicy, TraceFunc>>,
-    public JS::DynamicTraceable
+    public JS::Traceable
 {
     using Base = mozilla::VectorBase<T, MinInlineCapacity, AllocPolicy, TraceableVector>;
 
   public:
     explicit TraceableVector(AllocPolicy alloc = AllocPolicy()) : Base(alloc) {}
     TraceableVector(TraceableVector&& vec) : Base(mozilla::Forward<TraceableVector>(vec)) {}
     TraceableVector& operator=(TraceableVector&& vec) {
         return Base::operator=(mozilla::Forward<TraceableVector>(vec));
     }
 
-    void trace(JSTracer* trc) override {
+    static void trace(TraceableVector* vec, JSTracer* trc) { vec->trace(trc); }
+    void trace(JSTracer* trc) {
         for (size_t i = 0; i < this->length(); ++i)
             TraceFunc::trace(trc, &Base::operator[](i), "vector element");
     }
 };
 
 template <typename Outer, typename T, size_t Capacity, typename AllocPolicy, typename TraceFunc>
 class TraceableVectorOperations
 {
--- a/js/src/gc/RootMarking.cpp
+++ b/js/src/gc/RootMarking.cpp
@@ -35,30 +35,16 @@ using namespace js::gc;
 using mozilla::ArrayEnd;
 
 using JS::AutoGCRooter;
 
 typedef RootedValueMap::Range RootRange;
 typedef RootedValueMap::Entry RootEntry;
 typedef RootedValueMap::Enum RootEnum;
 
-// We cannot instantiate (even indirectly) the abstract JS::DynamicTraceable.
-// Instead we cast to a ConcreteTraceable, then upcast before calling trace so
-// that we get the implementation defined dynamically in the vtable.
-struct ConcreteTraceable : public JS::DynamicTraceable
-{
-    void trace(JSTracer* trc) override {}
-};
-
-static void
-MarkDynamicTraceable(JSTracer* trc, ConcreteTraceable* t, const char* name)
-{
-    static_cast<JS::DynamicTraceable*>(t)->trace(trc);
-}
-
 template <typename T>
 using TraceFunction = void (*)(JSTracer* trc, T* ref, const char* name);
 
 template <class T, TraceFunction<T> TraceFn = TraceNullableRoot, class Source>
 static inline void
 MarkExactStackRootList(JSTracer* trc, Source* s, const char* name)
 {
     Rooted<T>* rooter = s->roots.template gcRooters<T>();
@@ -71,30 +57,26 @@ MarkExactStackRootList(JSTracer* trc, So
 
 template<class T>
 static void
 MarkExactStackRootsAcrossTypes(T context, JSTracer* trc)
 {
     MarkExactStackRootList<JSObject*>(trc, context, "exact-object");
     MarkExactStackRootList<Shape*>(trc, context, "exact-shape");
     MarkExactStackRootList<BaseShape*>(trc, context, "exact-baseshape");
-    MarkExactStackRootList<ObjectGroup*>(
-        trc, context, "exact-objectgroup");
+    MarkExactStackRootList<ObjectGroup*>(trc, context, "exact-objectgroup");
     MarkExactStackRootList<JSString*>(trc, context, "exact-string");
     MarkExactStackRootList<JS::Symbol*>(trc, context, "exact-symbol");
     MarkExactStackRootList<jit::JitCode*>(trc, context, "exact-jitcode");
     MarkExactStackRootList<JSScript*>(trc, context, "exact-script");
     MarkExactStackRootList<LazyScript*>(trc, context, "exact-lazy-script");
     MarkExactStackRootList<jsid>(trc, context, "exact-id");
     MarkExactStackRootList<Value>(trc, context, "exact-value");
-    MarkExactStackRootList<JS::StaticTraceable,
-                           js::DispatchWrapper<JS::StaticTraceable>::TraceWrapped>(
-        trc, context, "StaticTraceable");
-    MarkExactStackRootList<ConcreteTraceable, MarkDynamicTraceable>(
-        trc, context, "DynamicTraceable");
+    MarkExactStackRootList<JS::Traceable, js::DispatchWrapper<JS::Traceable>::TraceWrapped>(
+        trc, context, "Traceable");
 }
 
 static void
 MarkExactStackRoots(JSRuntime* rt, JSTracer* trc)
 {
     for (ContextIter cx(rt); !cx.done(); cx.next())
         MarkExactStackRootsAcrossTypes<JSContext*>(cx.get(), trc);
     MarkExactStackRootsAcrossTypes<PerThreadData*>(&rt->mainThread, trc);
--- a/js/src/jsapi-tests/testGCExactRooting.cpp
+++ b/js/src/jsapi-tests/testGCExactRooting.cpp
@@ -35,17 +35,17 @@ BEGIN_TEST(testGCSuppressions)
     JS::AutoAssertOnGC nogcRt(cx->runtime());
     JS::AutoCheckCannotGC checkgcRt(cx->runtime());
     JS::AutoSuppressGCAnalysis noanalysisRt(cx->runtime());
 
     return true;
 }
 END_TEST(testGCSuppressions)
 
-struct MyContainer : public JS::StaticTraceable
+struct MyContainer : public JS::Traceable
 {
     RelocatablePtrObject obj;
     RelocatablePtrString str;
 
     MyContainer() : obj(nullptr), str(nullptr) {}
     static void trace(MyContainer* self, JSTracer* trc) {
         if (self->obj)
             js::TraceEdge(trc, &self->obj, "test container");
@@ -57,111 +57,32 @@ struct MyContainer : public JS::StaticTr
 namespace js {
 template <>
 struct RootedBase<MyContainer> {
     RelocatablePtrObject& obj() { return static_cast<Rooted<MyContainer>*>(this)->get().obj; }
     RelocatablePtrString& str() { return static_cast<Rooted<MyContainer>*>(this)->get().str; }
 };
 } // namespace js
 
-BEGIN_TEST(testGCRootedStaticStructInternalStackStorage)
-{
-    JS::Rooted<MyContainer> container(cx);
-    container.get().obj = JS_NewObject(cx, nullptr);
-    container.get().str = JS_NewStringCopyZ(cx, "Hello");
-
-    JS_GC(cx->runtime());
-    JS_GC(cx->runtime());
-
-    JS::RootedObject obj(cx, container.get().obj);
-    JS::RootedValue val(cx, StringValue(container.get().str));
-    CHECK(JS_SetProperty(cx, obj, "foo", val));
-    return true;
-}
-END_TEST(testGCRootedStaticStructInternalStackStorage)
-
 BEGIN_TEST(testGCRootedStaticStructInternalStackStorageAugmented)
 {
     JS::Rooted<MyContainer> container(cx);
     container.obj() = JS_NewObject(cx, nullptr);
     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));
     return true;
 }
 END_TEST(testGCRootedStaticStructInternalStackStorageAugmented)
 
-struct DynamicBase : public JS::DynamicTraceable
-{
-    RelocatablePtrObject obj;
-    DynamicBase() : obj(nullptr) {}
-
-    void trace(JSTracer* trc) override {
-        if (obj)
-            js::TraceEdge(trc, &obj, "test container");
-    }
-};
-
-struct DynamicContainer : public DynamicBase
-{
-    RelocatablePtrString str;
-    DynamicContainer() : str(nullptr) {}
-
-    void trace(JSTracer* trc) override {
-        this->DynamicBase::trace(trc);
-        if (str)
-            js::TraceEdge(trc, &str, "test container");
-    }
-};
-
-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; }
-};
-} // namespace js
-
-BEGIN_TEST(testGCRootedDynamicStructInternalStackStorage)
-{
-    JS::Rooted<DynamicContainer> container(cx);
-    container.get().obj = JS_NewObject(cx, nullptr);
-    container.get().str = JS_NewStringCopyZ(cx, "Hello");
-
-    JS_GC(cx->runtime());
-    JS_GC(cx->runtime());
-
-    JS::RootedObject obj(cx, container.get().obj);
-    JS::RootedValue val(cx, StringValue(container.get().str));
-    CHECK(JS_SetProperty(cx, obj, "foo", val));
-    return true;
-}
-END_TEST(testGCRootedDynamicStructInternalStackStorage)
-
-BEGIN_TEST(testGCRootedDynamicStructInternalStackStorageAugmented)
-{
-    JS::Rooted<DynamicContainer> container(cx);
-    container.obj() = JS_NewObject(cx, nullptr);
-    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));
-    return true;
-}
-END_TEST(testGCRootedDynamicStructInternalStackStorageAugmented)
-
 using MyHashMap = js::TraceableHashMap<js::Shape*, JSObject*>;
 
 BEGIN_TEST(testGCRootedHashMap)
 {
     JS::Rooted<MyHashMap> map(cx, MyHashMap(cx));
     CHECK(map.init(15));
     CHECK(map.initialized());
 
@@ -231,19 +152,20 @@ BEGIN_TEST(testGCHandleHashMap)
     JS_GC(rt);
 
     CHECK(CheckMyHashMap(cx, map));
 
     return true;
 }
 END_TEST(testGCHandleHashMap)
 
+using ShapeVec = TraceableVector<Shape*>;
+
 BEGIN_TEST(testGCRootedVector)
 {
-    using ShapeVec = TraceableVector<Shape*>;
     JS::Rooted<ShapeVec> shapes(cx, ShapeVec(cx));
 
     for (size_t i = 0; i < 10; ++i) {
         RootedObject obj(cx, JS_NewObject(cx, nullptr));
         RootedValue val(cx, UndefinedValue());
         // Construct a unique property name to ensure that the object creates a
         // new shape.
         char buffer[2];
@@ -306,18 +228,16 @@ receiveMutableHandleToShapeVector(JS::Mu
     // Ensure range enumeration works through the handle.
     for (auto shape : handle) {
         CHECK(shape);
     }
     return true;
 }
 END_TEST(testGCRootedVector)
 
-using ShapeVec = TraceableVector<Shape*>;
-
 static bool
 FillVector(JSContext* cx, MutableHandle<ShapeVec> shapes)
 {
     for (size_t i = 0; i < 10; ++i) {
         RootedObject obj(cx, JS_NewObject(cx, nullptr));
         RootedValue val(cx, UndefinedValue());
         // Construct a unique property name to ensure that the object creates a
         // new shape.
--- a/js/src/jsapi.h
+++ b/js/src/jsapi.h
@@ -2502,17 +2502,17 @@ extern JS_PUBLIC_API(bool)
 JS_PreventExtensions(JSContext* cx, JS::HandleObject obj, JS::ObjectOpResult& result);
 
 extern JS_PUBLIC_API(JSObject*)
 JS_New(JSContext* cx, JS::HandleObject ctor, const JS::HandleValueArray& args);
 
 
 /*** Property descriptors ************************************************************************/
 
-struct JSPropertyDescriptor : public JS::StaticTraceable {
+struct JSPropertyDescriptor : public JS::Traceable {
     JSObject* obj;
     unsigned attrs;
     JSGetterOp getter;
     JSSetterOp setter;
     JS::Value value;
 
     JSPropertyDescriptor()
       : obj(nullptr), attrs(0), getter(nullptr), setter(nullptr), value(JS::UndefinedValue())
--- a/js/src/jspubtd.h
+++ b/js/src/jspubtd.h
@@ -280,18 +280,17 @@ enum ThingRootKind
     THING_ROOT_OBJECT_GROUP,
     THING_ROOT_STRING,
     THING_ROOT_SYMBOL,
     THING_ROOT_JIT_CODE,
     THING_ROOT_SCRIPT,
     THING_ROOT_LAZY_SCRIPT,
     THING_ROOT_ID,
     THING_ROOT_VALUE,
-    THING_ROOT_STATIC_TRACEABLE,
-    THING_ROOT_DYNAMIC_TRACEABLE,
+    THING_ROOT_TRACEABLE,
     THING_ROOT_LIMIT
 };
 
 template <typename T>
 struct RootKind;
 
 /*
  * Specifically mark the ThingRootKind of externally visible types, so that
--- a/js/src/jsscript.h
+++ b/js/src/jsscript.h
@@ -201,17 +201,17 @@ class Binding
 JS_STATIC_ASSERT(sizeof(Binding) == sizeof(uintptr_t));
 
 /*
  * Formal parameters and local variables are stored in a shape tree
  * path encapsulated within this class.  This class represents bindings for
  * both function and top-level scripts (the latter is needed to track names in
  * strict mode eval code, to give such code its own lexical environment).
  */
-class Bindings : public JS::StaticTraceable
+class Bindings : public JS::Traceable
 {
     friend class BindingIter;
     friend class AliasedFormalIter;
     template <typename Outer> friend class BindingsOperations;
     template <typename Outer> friend class MutableBindingsOperations;
 
     RelocatablePtrShape callObjShape_;
     uintptr_t bindingArrayAndFlag_;
--- a/js/src/vm/Stack.h
+++ b/js/src/vm/Stack.h
@@ -1168,17 +1168,17 @@ struct DefaultHasher<AbstractFramePtr> {
 // and that T does not have an entry in the frame cache because its bit was
 // not set. Therefore, we have found entry E[j-1] and the subsequent entries
 // are stale and should be purged from the frame cache.
 //
 // We have a LiveSavedFrameCache for each activation to minimize the number of
 // entries that must be scanned through, and to avoid the headaches of
 // maintaining a cache for each compartment and invalidating stale cache entries
 // in the presence of cross-compartment calls.
-class LiveSavedFrameCache : public JS::StaticTraceable
+class LiveSavedFrameCache : public JS::Traceable
 {
   public:
     using FramePtr = mozilla::Variant<AbstractFramePtr, jit::CommonFrameLayout*>;
 
   private:
     struct Entry
     {
         FramePtr                    framePtr;
--- a/js/src/vm/TypeInference.h
+++ b/js/src/vm/TypeInference.h
@@ -267,17 +267,17 @@ class TypeSet
         void ensureTrackedProperty(JSContext* cx, jsid id);
 
         ObjectGroup* maybeGroup();
     };
 
     // Information about a single concrete type. We pack this into one word,
     // where small values are particular primitive or other singleton types and
     // larger values are either specific JS objects or object groups.
-    class Type : public JS::StaticTraceable
+    class Type : public JS::Traceable
     {
         friend class TypeSet;
 
         uintptr_t data;
         explicit Type(uintptr_t data) : data(data) {}
 
       public: