Bug 1209704 - Part 2: Share storage and mixins between Read and Write barriers; r=jonco
authorTerrence Cole <terrence@mozilla.com>
Thu, 01 Oct 2015 13:32:51 -0700
changeset 265807 b297bcfe050499da98ed6de2bfff66e58fdb48a0
parent 265806 af829895379d1e384fae6e89f01a7b2ae4467b62
child 265808 d36103a859acec5e091d0c2160cd81d3dff27ad4
push id29470
push userphilringnalda@gmail.com
push dateSat, 03 Oct 2015 22:38:52 +0000
treeherdermozilla-central@2e62c35afb2c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjonco
bugs1209704
milestone44.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 1209704 - Part 2: Share storage and mixins between Read and Write barriers; r=jonco
js/src/gc/Barrier.h
js/src/gc/Marking.cpp
js/src/gc/Marking.h
--- a/js/src/gc/Barrier.h
+++ b/js/src/gc/Barrier.h
@@ -130,27 +130,32 @@
  * One additional note: not all object writes need to be pre-barriered. Writes
  * to newly allocated objects do not need a pre-barrier. In these cases, we use
  * the "obj->field.init(value)" method instead of "obj->field = value". We use
  * the init naming idiom in many places to signify that a field is being
  * assigned for the first time.
  *
  * This file implements four classes, illustrated here:
  *
- * BarrieredBase          base class of all barriers
- *  |
- * WriteBarrieredBase     base class which provides common write operations
+ * BarrieredBase             base class of all barriers
+ *  |  |
+ *  | WriteBarrieredBase     base class which provides common write operations
+ *  |  |  |  |  |
+ *  |  |  |  | PreBarriered  provides pre-barriers only
  *  |  |  |  |
- *  |  |  | PreBarriered  provides pre-barriers only
+ *  |  |  | HeapPtr          provides pre- and post-barriers
  *  |  |  |
- *  |  | HeapPtr          provides pre- and post-barriers
+ *  |  | RelocatablePtr      provides pre- and post-barriers and is relocatable
  *  |  |
- *  | RelocatablePtr      provides pre- and post-barriers and is relocatable
+ *  | HeapSlot               similar to HeapPtr, but tailored to slots storage
  *  |
- * HeapSlot               similar to HeapPtr, but tailored to slots storage
+ * ReadBarrieredBase         base class which provides common read operations
+ *  |
+ * ReadBarriered             provides read barriers only
+ *
  *
  * The implementation of the barrier logic is implemented on T::writeBarrier.*,
  * via:
  *
  * WriteBarrieredBase<T>::pre
  *  -> InternalGCMethods<T*>::preBarrier
  *      -> T::writeBarrierPre
  *  -> InternalGCMethods<Value>::preBarrier
@@ -362,20 +367,16 @@ class WriteBarrieredBase : public Barrie
     /* For users who need to manually barrier the raw types. */
     static void writeBarrierPre(const T& v) { InternalGCMethods<T>::preBarrier(v); }
 
   protected:
     void pre() { InternalGCMethods<T>::preBarrier(this->value); }
     void post(T prev, T next) { InternalGCMethods<T>::postBarrier(&this->value, prev, next); }
 };
 
-template <>
-class BarrieredBaseMixins<JS::Value> : public ValueOperations<WriteBarrieredBase<JS::Value>>
-{};
-
 /*
  * PreBarriered only automatically handles pre-barriers. Post-barriers must
  * be manually implemented when using this class. HeapPtr and RelocatablePtr
  * should be used in all cases that do not require explicit low-level control
  * of moving behavior, e.g. for HashMap keys.
  */
 template <class T>
 class PreBarriered : public WriteBarrieredBase<T>
@@ -520,57 +521,74 @@ class RelocatablePtr : public WriteBarri
 
     void postBarrieredSet(const T& v) {
         T tmp = this->value;
         this->value = v;
         this->post(tmp, this->value);
     }
 };
 
-/*
- * Incremental GC requires that weak pointers have read barriers. This is mostly
- * an issue for empty shapes stored in JSCompartment. The problem happens when,
- * during an incremental GC, some JS code stores one of the compartment's empty
- * shapes into an object already marked black. Normally, this would not be a
- * problem, because the empty shape would have been part of the initial snapshot
- * when the GC started. However, since this is a weak pointer, it isn't. So we
- * may collect the empty shape even though a live object points to it. To fix
- * this, we mark these empty shapes black whenever they get read out.
- */
-template <class T>
-class ReadBarriered
+// Base class for barriered pointer types that intercept only reads.
+template <typename T>
+class ReadBarrieredBase : public BarrieredBase<T>
 {
-    T value;
+  protected:
+    // ReadBarrieredBase is not directly instantiable.
+    explicit ReadBarrieredBase(T v) : BarrieredBase<T>(v) {}
 
   public:
-    ReadBarriered() : value(nullptr) {}
-    explicit ReadBarriered(T value) : value(value) {}
-    explicit ReadBarriered(const Rooted<T>& rooted) : value(rooted) {}
+    // For use by the GC.
+    T* unsafeGet() { return &this->value; }
+
+  protected:
+    void read() const { InternalGCMethods<T>::readBarrier(this->value); }
+};
+
+// Incremental GC requires that weak pointers have read barriers. This is mostly
+// an issue for empty shapes stored in JSCompartment. The problem happens when,
+// during an incremental GC, some JS code stores one of the compartment's empty
+// shapes into an object already marked black. Normally, this would not be a
+// problem, because the empty shape would have been part of the initial snapshot
+// when the GC started. However, since this is a weak pointer, it isn't. So we
+// may collect the empty shape even though a live object points to it. To fix
+// this, we mark these empty shapes black whenever they get read out.
+template <class T>
+class ReadBarriered : public ReadBarrieredBase<T>
+{
+  public:
+    ReadBarriered() : ReadBarrieredBase<T>(GCMethods<T>::initial()) {}
+    explicit ReadBarriered(T v) : ReadBarrieredBase<T>(v) {}
 
     T get() const {
-        if (!InternalGCMethods<T>::isMarkable(value))
+        if (!InternalGCMethods<T>::isMarkable(this->value))
             return GCMethods<T>::initial();
-        InternalGCMethods<T>::readBarrier(value);
-        return value;
+        this->read();
+        return this->value;
     }
 
     T unbarrieredGet() const {
-        return value;
+        return this->value;
     }
 
     operator T() const { return get(); }
 
     T& operator*() const { return *get(); }
     T operator->() const { return get(); }
 
-    T* unsafeGet() { return &value; }
-    T const * unsafeGet() const { return &value; }
+    T* unsafeGet() { return &this->value; }
+    T const* unsafeGet() const { return &this->value; }
+
+    void set(T v) { this->value = v; }
+};
 
-    void set(T v) { value = v; }
-};
+// Add Value operations to all Barrier types. Note, this must be defined before
+// HeapSlot for HeapSlot's base to get these operations.
+template <>
+class BarrieredBaseMixins<JS::Value> : public ValueOperations<WriteBarrieredBase<JS::Value>>
+{};
 
 // A pre- and post-barriered Value that is specialized to be aware that it
 // resides in a slots or elements vector. This allows it to be relocated in
 // memory, but with substantially less overhead than a RelocatablePtr.
 class HeapSlot : public WriteBarrieredBase<Value>
 {
   public:
     enum Kind {
--- a/js/src/gc/Marking.cpp
+++ b/js/src/gc/Marking.cpp
@@ -2325,17 +2325,17 @@ template <typename T>
 bool
 IsMarked(WriteBarrieredBase<T>* thingp)
 {
     return IsMarkedInternal(ConvertToBase(thingp->unsafeGet()));
 }
 
 template <typename T>
 bool
-IsMarked(ReadBarriered<T>* thingp)
+IsMarked(ReadBarrieredBase<T>* thingp)
 {
     return IsMarkedInternal(ConvertToBase(thingp->unsafeGet()));
 }
 
 template <typename T>
 bool
 IsAboutToBeFinalizedUnbarriered(T* thingp)
 {
@@ -2346,29 +2346,29 @@ template <typename T>
 bool
 IsAboutToBeFinalized(WriteBarrieredBase<T>* thingp)
 {
     return IsAboutToBeFinalizedInternal(ConvertToBase(thingp->unsafeGet()));
 }
 
 template <typename T>
 bool
-IsAboutToBeFinalized(ReadBarriered<T>* thingp)
+IsAboutToBeFinalized(ReadBarrieredBase<T>* thingp)
 {
     return IsAboutToBeFinalizedInternal(ConvertToBase(thingp->unsafeGet()));
 }
 
 // Instantiate a copy of the Tracing templates for each derived type.
 #define INSTANTIATE_ALL_VALID_TRACE_FUNCTIONS(type) \
     template bool IsMarkedUnbarriered<type>(type*); \
     template bool IsMarked<type>(WriteBarrieredBase<type>*); \
-    template bool IsMarked<type>(ReadBarriered<type>*); \
+    template bool IsMarked<type>(ReadBarrieredBase<type>*); \
     template bool IsAboutToBeFinalizedUnbarriered<type>(type*); \
     template bool IsAboutToBeFinalized<type>(WriteBarrieredBase<type>*); \
-    template bool IsAboutToBeFinalized<type>(ReadBarriered<type>*);
+    template bool IsAboutToBeFinalized<type>(ReadBarrieredBase<type>*);
 FOR_EACH_GC_POINTER_TYPE(INSTANTIATE_ALL_VALID_TRACE_FUNCTIONS)
 #undef INSTANTIATE_ALL_VALID_TRACE_FUNCTIONS
 
 } /* namespace gc */
 } /* namespace js */
 
 
 /*** Type Marking *********************************************************************************/
--- a/js/src/gc/Marking.h
+++ b/js/src/gc/Marking.h
@@ -387,29 +387,29 @@ bool
 IsMarkedUnbarriered(T* thingp);
 
 template <typename T>
 bool
 IsMarked(WriteBarrieredBase<T>* thingp);
 
 template <typename T>
 bool
-IsMarked(ReadBarriered<T>* thingp);
+IsMarked(ReadBarrieredBase<T>* thingp);
 
 template <typename T>
 bool
 IsAboutToBeFinalizedUnbarriered(T* thingp);
 
 template <typename T>
 bool
 IsAboutToBeFinalized(WriteBarrieredBase<T>* thingp);
 
 template <typename T>
 bool
-IsAboutToBeFinalized(ReadBarriered<T>* thingp);
+IsAboutToBeFinalized(ReadBarrieredBase<T>* thingp);
 
 inline Cell*
 ToMarkable(const Value& v)
 {
     if (v.isMarkable())
         return (Cell*)v.toGCThing();
     return nullptr;
 }