From bca82601d0f0637121f67ce1222107c1186fbcb0 Mon Sep 17 00:00:00 2001 From: Terrence Cole Date: Tue, 3 Nov 2015 07:08:05 -0800 Subject: [PATCH] Bug 1220310 - Generalize GC container trace function dispatch as GCPolicy; r=sfink --- js/public/TraceableHashTable.h | 92 ++++++++++++++++++---------------- js/public/TraceableVector.h | 49 +++++++++--------- js/public/TracingAPI.h | 33 ++++++++---- js/src/ds/TraceableFifo.h | 25 ++++----- js/src/gc/Tracer.h | 10 +++- js/src/vm/Debugger.h | 4 +- 6 files changed, 121 insertions(+), 92 deletions(-) diff --git a/js/public/TraceableHashTable.h b/js/public/TraceableHashTable.h index 2993d22fa91..eece6ec4f94 100644 --- a/js/public/TraceableHashTable.h +++ b/js/public/TraceableHashTable.h @@ -13,15 +13,23 @@ namespace js { +// Define a reasonable default GC policy for GC-aware Maps. +template +struct DefaultMapGCPolicy { + using KeyPolicy = DefaultGCPolicy; + using ValuePolicy = DefaultGCPolicy; +}; + // A TraceableHashMap is a HashMap with an additional trace method that knows // how to visit all keys and values in the table. HashMaps that contain GC // pointers that must be traced to be kept alive will generally want to use // this TraceableHashMap specializeation in lieu of HashMap. // // Most types of GC pointers as keys and values can be traced with no extra -// infrastructure. For structs and non-gc-pointer members, ensure that there -// is a specialization of DefaultTracer with an appropriate trace method -// available to handle the custom type. +// infrastructure. For structs and non-gc-pointer members, ensure that there +// is a specialization of DefaultGCPolicy with an appropriate trace method +// available to handle the custom type. Generic helpers can be found in +// js/public/TracingAPI.h. // // Note that although this HashMap's trace will deal correctly with moved keys, // it does not itself know when to barrier or trace keys. To function properly @@ -30,8 +38,7 @@ template , typename AllocPolicy = TempAllocPolicy, - typename KeyTraceFunc = DefaultTracer, - typename ValueTraceFunc = DefaultTracer> + typename GCPolicy = DefaultMapGCPolicy> class TraceableHashMap : public HashMap, public JS::Traceable { @@ -45,9 +52,9 @@ class TraceableHashMap : public HashMap, if (!this->initialized()) return; for (typename Base::Enum e(*this); !e.empty(); e.popFront()) { - ValueTraceFunc::trace(trc, &e.front().value(), "hashmap value"); + GCPolicy::ValuePolicy::trace(trc, &e.front().value(), "hashmap value"); Key key = e.front().key(); - KeyTraceFunc::trace(trc, &key, "hashmap key"); + GCPolicy::KeyPolicy::trace(trc, &key, "hashmap key"); if (key != e.front().key()) e.rekeyFront(key); } @@ -137,40 +144,40 @@ class MutableTraceableHashMapOperations } }; -template -class RootedBase> - : public MutableTraceableHashMapOperations>, A,B,C,D,E,F> +template +class RootedBase> + : public MutableTraceableHashMapOperations>, A,B,C,D,E> {}; -template -class MutableHandleBase> - : public MutableTraceableHashMapOperations>, - A,B,C,D,E,F> +template +class MutableHandleBase> + : public MutableTraceableHashMapOperations>, + A,B,C,D,E> {}; -template -class HandleBase> - : public TraceableHashMapOperations>, A,B,C,D,E,F> +template +class HandleBase> + : public TraceableHashMapOperations>, A,B,C,D,E> {}; // A TraceableHashSet is a HashSet with an additional trace method that knows -// how to visit all set element. HashSets that contain GC pointers that must +// how to visit all set elements. HashSets that contain GC pointers that must // be traced to be kept alive will generally want to use this TraceableHashSet // specializeation in lieu of HashSet. // -// Most types of GC pointers can be traced with no extra infrastructure. For +// Most types of GC pointers can be traced with no extra infrastructure. For // structs and non-gc-pointer members, ensure that there is a specialization of -// DefaultTracer with an appropriate trace method available to handle the -// custom type. +// DefaultGCPolicy with an appropriate trace method available to handle the +// custom type. Generic helpers can be found in js/public/TracingAPI.h. // // Note that although this HashSet's trace will deal correctly with moved -// elements, it does not itself know when to barrier or trace elements. To +// elements, it does not itself know when to barrier or trace elements. To // function properly it must either be used with Rooted or barriered and traced // manually. template , typename AllocPolicy = TempAllocPolicy, - typename ElemTraceFunc = DefaultTracer> + typename GCPolicy = DefaultGCPolicy> class TraceableHashSet : public HashSet, public JS::Traceable { @@ -185,7 +192,7 @@ class TraceableHashSet : public HashSet, return; for (typename Base::Enum e(*this); !e.empty(); e.popFront()) { T elem = e.front(); - ElemTraceFunc::trace(trc, &elem, "hashset element"); + GCPolicy::trace(trc, &elem, "hashset element"); if (elem != e.front()) e.rekeyFront(elem); } @@ -273,41 +280,42 @@ class MutableTraceableHashSetOperations } }; -template -class RootedBase> - : public MutableTraceableHashSetOperations>, T, HP, AP, TF> +template +class RootedBase> + : public MutableTraceableHashSetOperations>, + T, HP, AP, GP> { - using Set = TraceableHashSet; + using Set = TraceableHashSet; - friend class TraceableHashSetOperations, T, HP, AP, TF>; + friend class TraceableHashSetOperations, T, HP, AP, GP>; const Set& extract() const { return *static_cast*>(this)->address(); } - friend class MutableTraceableHashSetOperations, T, HP, AP, TF>; + friend class MutableTraceableHashSetOperations, T, HP, AP, GP>; Set& extract() { return *static_cast*>(this)->address(); } }; -template -class MutableHandleBase> - : public MutableTraceableHashSetOperations>, - T, HP, AP, TF> +template +class MutableHandleBase> + : public MutableTraceableHashSetOperations>, + T, HP, AP, GP> { - using Set = TraceableHashSet; + using Set = TraceableHashSet; - friend class TraceableHashSetOperations, T, HP, AP, TF>; + friend class TraceableHashSetOperations, T, HP, AP, GP>; const Set& extract() const { return *static_cast*>(this)->address(); } - friend class MutableTraceableHashSetOperations, T, HP, AP, TF>; + friend class MutableTraceableHashSetOperations, T, HP, AP, GP>; Set& extract() { return *static_cast*>(this)->address(); } }; -template -class HandleBase> - : public TraceableHashSetOperations>, T, HP, AP, TF> +template +class HandleBase> + : public TraceableHashSetOperations>, T, HP, AP, GP> { - using Set = TraceableHashSet; - friend class TraceableHashSetOperations, T, HP, AP, TF>; + using Set = TraceableHashSet; + friend class TraceableHashSetOperations, T, HP, AP, GP>; const Set& extract() const { return *static_cast*>(this)->address(); } }; diff --git a/js/public/TraceableVector.h b/js/public/TraceableVector.h index 53c66961ba1..7382b08ec04 100644 --- a/js/public/TraceableVector.h +++ b/js/public/TraceableVector.h @@ -22,8 +22,9 @@ namespace js { // // Most types of GC pointers as keys and values can be traced with no extra // infrastructure. For structs and non-gc-pointer members, ensure that there -// is a specialization of DefaultTracer with an appropriate trace method -// available to handle the custom type. +// is a specialization of DefaultGCPolicy with an appropriate trace method +// available to handle the custom type. Generic helpers can be found in +// js/public/TracingAPI.h. // // Note that although this Vector's trace will deal correctly with moved items, // it does not itself know when to barrier or trace items. To function properly @@ -31,12 +32,12 @@ namespace js { template > + typename GCPolicy = DefaultGCPolicy> class TraceableVector : public mozilla::VectorBase>, + TraceableVector>, public JS::Traceable { using Base = mozilla::VectorBase; @@ -51,14 +52,14 @@ class TraceableVector 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"); + GCPolicy::trace(trc, &Base::operator[](i), "vector element"); } }; -template +template class TraceableVectorOperations { - using Vec = TraceableVector; + using Vec = TraceableVector; const Vec& vec() const { return static_cast(this)->get(); } public: @@ -76,11 +77,11 @@ class TraceableVectorOperations } }; -template +template class MutableTraceableVectorOperations - : public TraceableVectorOperations + : public TraceableVectorOperations { - using Vec = TraceableVector; + using Vec = TraceableVector; const Vec& vec() const { return static_cast(this)->get(); } Vec& vec() { return static_cast(this)->get(); } @@ -141,26 +142,26 @@ class MutableTraceableVectorOperations void erase(T* aBegin, T* aEnd) { vec().erase(aBegin, aEnd); } }; -template -class RootedBase> - : public MutableTraceableVectorOperations>, T,N,AP,TP> +template +class RootedBase> + : public MutableTraceableVectorOperations>, T,N,AP,GP> {}; -template -class MutableHandleBase> - : public MutableTraceableVectorOperations>, - T,N,AP,TP> +template +class MutableHandleBase> + : public MutableTraceableVectorOperations>, + T,N,AP,GP> {}; -template -class HandleBase> - : public TraceableVectorOperations>, T,N,AP,TP> +template +class HandleBase> + : public TraceableVectorOperations>, T,N,AP,GP> {}; -template -class PersistentRootedBase> - : public MutableTraceableVectorOperations>, - T,N,AP,TP> +template +class PersistentRootedBase> + : public MutableTraceableVectorOperations>, + T,N,AP,GP> {}; } // namespace js diff --git a/js/public/TracingAPI.h b/js/public/TracingAPI.h index c18d2552914..bdc6cf9ea57 100644 --- a/js/public/TracingAPI.h +++ b/js/public/TracingAPI.h @@ -369,31 +369,44 @@ JS_GetTraceThingInfo(char* buf, size_t bufsize, JSTracer* trc, namespace js { -// Automates static dispatch for tracing for TraceableContainers. -template struct DefaultTracer; +// Automates static dispatch for GC interaction with TraceableContainers. +template +struct DefaultGCPolicy; -// The default for non-pod (e.g. struct) types is to call the trace method. +// This policy dispatches GC methods to a method on the type. template -struct DefaultTracer { +struct StructGCPolicy { static void trace(JSTracer* trc, T* t, const char* name) { + // This is the default GC policy for storing GC things in containers. // If your build is failing here, it means you either need an - // implementation of DefaultTracer for your type or, for container - // and structure types that contain GC pointers, a trace method. + // implementation of DefaultGCPolicy for your type or, if this is + // the right policy for you, your struct or container is missing a + // trace method. t->trace(trc); } }; +// This policy ignores any GC interaction, e.g. for non-GC types. +template +struct IgnoreGCPolicy { + static void trace(JSTracer* trc, uint32_t* id, const char* name) {} +}; + +// The default policy when no other more specific policy fits (e.g. for a +// direct GC pointer), is to assume a struct type that implements the needed +// methods. +template +struct DefaultGCPolicy : public StructGCPolicy {}; + template <> -struct DefaultTracer +struct DefaultGCPolicy { static void trace(JSTracer* trc, jsid* id, const char* name) { JS_CallUnbarrieredIdTracer(trc, id, name); } }; -template <> struct DefaultTracer { - static void trace(JSTracer* trc, uint32_t* id, const char* name) {} -}; +template <> struct DefaultGCPolicy : public IgnoreGCPolicy {}; } // namespace js diff --git a/js/src/ds/TraceableFifo.h b/js/src/ds/TraceableFifo.h index 2ac1390e977..52c70858fde 100644 --- a/js/src/ds/TraceableFifo.h +++ b/js/src/ds/TraceableFifo.h @@ -9,11 +9,10 @@ #include "ds/Fifo.h" #include "js/RootingAPI.h" +#include "js/TracingAPI.h" namespace js { -template struct DefaultTracer; - // A TraceableFifo is a Fifo with an additional trace method that knows how to // visit all of the items stored in the Fifo. For Fifos that contain GC things, // this is usually more convenient than manually iterating and marking the @@ -21,8 +20,10 @@ template struct DefaultTracer; // // Most types of GC pointers as keys and values can be traced with no extra // infrastructure. For structs and non-gc-pointer members, ensure that there is -// a specialization of DefaultTracer with an appropriate trace method -// available to handle the custom type. +// a specialization of DefaultGCPolicy with an appropriate trace method +// available to handle the custom type. Generic helpers can be found in +// js/public/TracingAPI.h. Generic helpers can be found in +// js/public/TracingAPI.h. // // Note that although this Fifo's trace will deal correctly with moved items, it // does not itself know when to barrier or trace items. To function properly it @@ -30,7 +31,7 @@ template struct DefaultTracer; template > + typename GCPolicy = DefaultGCPolicy> class TraceableFifo : public js::Fifo, public JS::Traceable @@ -48,16 +49,16 @@ class TraceableFifo static void trace(TraceableFifo* tf, JSTracer* trc) { for (size_t i = 0; i < tf->front_.length(); ++i) - TraceFunc::trace(trc, &tf->front_[i], "fifo element"); + GCPolicy::trace(trc, &tf->front_[i], "fifo element"); for (size_t i = 0; i < tf->rear_.length(); ++i) - TraceFunc::trace(trc, &tf->rear_[i], "fifo element"); + GCPolicy::trace(trc, &tf->rear_[i], "fifo element"); } }; -template +template class TraceableFifoOperations { - using TF = TraceableFifo; + using TF = TraceableFifo; const TF& fifo() const { return static_cast(this)->extract(); } public: @@ -66,11 +67,11 @@ class TraceableFifoOperations const T& front() const { return fifo().front(); } }; -template +template class MutableTraceableFifoOperations - : public TraceableFifoOperations + : public TraceableFifoOperations { - using TF = TraceableFifo; + using TF = TraceableFifo; TF& fifo() { return static_cast(this)->extract(); } public: diff --git a/js/src/gc/Tracer.h b/js/src/gc/Tracer.h index 81da218b9d3..64976ad25da 100644 --- a/js/src/gc/Tracer.h +++ b/js/src/gc/Tracer.h @@ -138,16 +138,22 @@ TraceCycleCollectorChildren(JS::CallbackTracer* trc, ObjectGroup* group); } // namespace gc +// Define a default Policy for all pointer types. This may fail to link if this +// policy gets used on a non-GC typed pointer by accident. template -struct DefaultTracer +struct DefaultGCPolicy { static void trace(JSTracer* trc, T** t, const char* name) { + // If linking is failing here, it likely means that you need to define + // or use a non-default GC policy for your non-gc-pointer type. TraceManuallyBarrieredEdge(trc, t, name); } }; +// RelocatablePtr is only defined for GC pointer types, so this default policy +// should work in all cases. template -struct DefaultTracer> +struct DefaultGCPolicy> { static void trace(JSTracer* trc, RelocatablePtr t, const char* name) { TraceEdge(trc, t, name); diff --git a/js/src/vm/Debugger.h b/js/src/vm/Debugger.h index bb1d6f7fb78..20484b3b942 100644 --- a/js/src/vm/Debugger.h +++ b/js/src/vm/Debugger.h @@ -943,14 +943,14 @@ class Debugger : private mozilla::LinkedListElement }; template<> -struct DefaultTracer { +struct DefaultGCPolicy { static void trace(JSTracer* trc, Debugger::TenurePromotionsLogEntry* e, const char*) { Debugger::TenurePromotionsLogEntry::trace(e, trc); } }; template<> -struct DefaultTracer { +struct DefaultGCPolicy { static void trace(JSTracer* trc, Debugger::AllocationsLogEntry* e, const char*) { Debugger::AllocationsLogEntry::trace(e, trc); }