Bug 885954 - Add comments to barrier classes explaining our use of C++ move semantics; r=jimb
authorTerrence Cole <terrence@mozilla.com>
Fri, 07 Feb 2014 10:03:21 -0800
changeset 167591 64c3fe7dd96b71615626e921136a0d17c01b1a86
parent 167590 a961fb6594c079f1c9257845a45fd682322ff825
child 167592 479ce9c26b7e411d6953bf3a83b591fc668e3a40
push id39495
push usertcole@mozilla.com
push dateFri, 07 Feb 2014 21:53:24 +0000
treeherdermozilla-inbound@64c3fe7dd96b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjimb
bugs885954
milestone30.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 885954 - Add comments to barrier classes explaining our use of C++ move semantics; r=jimb
js/public/RootingAPI.h
js/src/gc/Barrier.h
--- a/js/public/RootingAPI.h
+++ b/js/public/RootingAPI.h
@@ -195,16 +195,23 @@ class Heap : public js::HeapBase<T>
 {
   public:
     Heap() {
         static_assert(sizeof(T) == sizeof(Heap<T>),
                       "Heap<T> must be binary compatible with T.");
         init(js::GCMethods<T>::initial());
     }
     explicit Heap(T p) { init(p); }
+
+    /*
+     * For Heap, move semantics are equivalent to copy semantics. In C++, a
+     * copy constructor taking const-ref is the way to get a single function
+     * that will be used for both lvalue and rvalue copies, so we can simply
+     * omit the rvalue variant.
+     */
     explicit Heap(const Heap<T> &p) { init(p.ptr); }
 
     ~Heap() {
         if (js::GCMethods<T>::needsPostBarrier(ptr))
             relocate();
     }
 
     bool operator==(const Heap<T> &other) { return ptr == other.ptr; }
--- a/js/src/gc/Barrier.h
+++ b/js/src/gc/Barrier.h
@@ -325,16 +325,22 @@ class BarrieredPtr
     T *operator->() const { return value; }
 
     operator T*() const { return value; }
 
   protected:
     void pre() { T::writeBarrierPre(value); }
 };
 
+/*
+ * EncapsulatedPtr 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, typename Unioned = uintptr_t>
 class EncapsulatedPtr : public BarrieredPtr<T, Unioned>
 {
   public:
     EncapsulatedPtr() : BarrieredPtr<T, Unioned>(nullptr) {}
     EncapsulatedPtr(T *v) : BarrieredPtr<T, Unioned>(v) {}
     explicit EncapsulatedPtr(const EncapsulatedPtr<T, Unioned> &v)
       : BarrieredPtr<T, Unioned>(v.value) {}
@@ -358,17 +364,24 @@ class EncapsulatedPtr : public Barriered
         this->value = v.value;
         return *this;
     }
 };
 
 /*
  * A pre- and post-barriered heap pointer, for use inside the JS engine.
  *
- * Not to be confused with JS::Heap<T>.
+ * Not to be confused with JS::Heap<T>. This is a different class from the
+ * external interface and implements substantially different semantics.
+ *
+ * The post-barriers implemented by this class are faster than those
+ * implemented by RelocatablePtr<T> or JS::Heap<T> at the cost of not
+ * automatically handling deletion or movement. It should generally only be
+ * stored in memory that has GC lifetime. HeapPtr must not be used in contexts
+ * where it may be implicitly moved or deleted, e.g. most containers.
  */
 template <class T, class Unioned = uintptr_t>
 class HeapPtr : public BarrieredPtr<T, Unioned>
 {
   public:
     HeapPtr() : BarrieredPtr<T, Unioned>(nullptr) {}
     explicit HeapPtr(T *v) : BarrieredPtr<T, Unioned>(v) { post(); }
     explicit HeapPtr(const HeapPtr<T, Unioned> &v) : BarrieredPtr<T, Unioned>(v) { post(); }
@@ -401,17 +414,23 @@ class HeapPtr : public BarrieredPtr<T, U
     /* Make this friend so it can access pre() and post(). */
     template <class T1, class T2>
     friend inline void
     BarrieredSetPair(Zone *zone,
                      HeapPtr<T1> &v1, T1 *val1,
                      HeapPtr<T2> &v2, T2 *val2);
 
   private:
-    /* The default move construction and assignment operators would be incorrect. */
+    /*
+     * Unlike RelocatablePtr<T>, HeapPtr<T> must be managed with GC lifetimes.
+     * Specifically, the memory used by the pointer itself must be live until
+     * at least the next minor GC. For that reason, move semantics are invalid
+     * and are deleted here. Please note that not all containers support move
+     * semantics, so this does not completely prevent invalid uses.
+     */
     HeapPtr(HeapPtr<T> &&) MOZ_DELETE;
     HeapPtr<T, Unioned> &operator=(HeapPtr<T, Unioned> &&) MOZ_DELETE;
 };
 
 /*
  * FixedHeapPtr is designed for one very narrow case: replacing immutable raw
  * pointers to GC-managed things, implicitly converting to a handle type for
  * ease of use.  Pointers encapsulated by this type must:
@@ -436,25 +455,39 @@ class FixedHeapPtr
         return Handle<T*>::fromMarkedLocation(&value);
     }
 
     void init(T *ptr) {
         value = ptr;
     }
 };
 
+/*
+ * A pre- and post-barriered heap pointer, for use inside the JS engine.
+ *
+ * Unlike HeapPtr<T>, it can be used in memory that is not managed by the GC,
+ * i.e. in C++ containers.  It is, however, somewhat slower, so should only be
+ * used in contexts where this ability is necessary.
+ */
 template <class T>
 class RelocatablePtr : public BarrieredPtr<T>
 {
   public:
     RelocatablePtr() : BarrieredPtr<T>(nullptr) {}
     explicit RelocatablePtr(T *v) : BarrieredPtr<T>(v) {
         if (v)
             post();
     }
+
+    /*
+     * For RelocatablePtr, move semantics are equivalent to copy semantics. In
+     * C++, a copy constructor taking const-ref is the way to get a single
+     * function that will be used for both lvalue and rvalue copies, so we can
+     * simply omit the rvalue variant.
+     */
     RelocatablePtr(const RelocatablePtr<T> &v) : BarrieredPtr<T>(v) {
         if (this->value)
             post();
     }
 
     ~RelocatablePtr() {
         if (this->value)
             relocate();
@@ -656,16 +689,18 @@ class BarrieredValue : public ValueOpera
         return reinterpret_cast<JS::shadow::Runtime*>(runtimeFromAnyThread(v));
     }
 
   private:
     friend class ValueOperations<BarrieredValue>;
     const Value * extract() const { return &value; }
 };
 
+// Like EncapsulatedPtr, but specialized for Value.
+// See the comments on that class for details.
 class EncapsulatedValue : public BarrieredValue
 {
   public:
     EncapsulatedValue(const Value &v) : BarrieredValue(v) {}
     EncapsulatedValue(const EncapsulatedValue &v) : BarrieredValue(v) {}
 
     EncapsulatedValue &operator=(const Value &v) {
         pre();
@@ -677,21 +712,18 @@ class EncapsulatedValue : public Barrier
     EncapsulatedValue &operator=(const EncapsulatedValue &v) {
         pre();
         JS_ASSERT(!IsPoisonedValue(v));
         value = v.get();
         return *this;
     }
 };
 
-/*
- * A pre- and post-barriered heap JS::Value, for use inside the JS engine.
- *
- * Not to be confused with JS::Heap<JS::Value>.
- */
+// Like HeapPtr, but specialized for Value.
+// See the comments on that class for details.
 class HeapValue : public BarrieredValue
 {
   public:
     explicit HeapValue()
       : BarrieredValue(UndefinedValue())
     {
         post();
     }
@@ -781,21 +813,22 @@ class HeapValue : public BarrieredValue
     void post() {
         writeBarrierPost(value, &value);
     }
 
     void post(JSRuntime *rt) {
         writeBarrierPost(rt, value, &value);
     }
 
-    /* The default move construction and assignment operators would be incorrect. */
     HeapValue(HeapValue &&) MOZ_DELETE;
     HeapValue &operator=(HeapValue &&) MOZ_DELETE;
 };
 
+// Like RelocatablePtr, but specialized for Value.
+// See the comments on that class for details.
 class RelocatableValue : public BarrieredValue
 {
   public:
     explicit RelocatableValue() : BarrieredValue(UndefinedValue()) {}
 
     explicit RelocatableValue(const Value &v)
       : BarrieredValue(v)
     {
@@ -860,16 +893,19 @@ class RelocatableValue : public Barriere
     void relocate(JSRuntime *rt) {
 #ifdef JSGC_GENERATIONAL
         JS::shadow::Runtime *shadowRuntime = JS::shadow::Runtime::asShadowRuntime(rt);
         shadowRuntime->gcStoreBufferPtr()->removeRelocatableValue(&value);
 #endif
     }
 };
 
+// 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 BarrieredValue
 {
   public:
     enum Kind {
         Slot,
         Element
     };
 
@@ -1030,31 +1066,35 @@ class BarrieredId
                 js::gc::MarkStringUnbarriered(shadowZone->barrierTracer(), &str, "write barrier");
                 JS_ASSERT(str == JSID_TO_STRING(value));
             }
         }
 #endif
     }
 };
 
+// Like EncapsulatedPtr, but specialized for jsid.
+// See the comments on that class for details.
 class EncapsulatedId : public BarrieredId
 {
   public:
     explicit EncapsulatedId(jsid id) : BarrieredId(id) {}
     explicit EncapsulatedId() : BarrieredId(JSID_VOID) {}
 
     EncapsulatedId &operator=(const EncapsulatedId &v) {
         if (v.value != value)
             pre();
         JS_ASSERT(!IsPoisonedId(v.value));
         value = v.value;
         return *this;
     }
 };
 
+// Like RelocatablePtr, but specialized for jsid.
+// See the comments on that class for details.
 class RelocatableId : public BarrieredId
 {
   public:
     explicit RelocatableId() : BarrieredId(JSID_VOID) {}
     explicit inline RelocatableId(jsid id) : BarrieredId(id) {}
     ~RelocatableId() { pre(); }
 
     bool operator==(jsid id) const { return value == id; }
@@ -1077,21 +1117,18 @@ class RelocatableId : public BarrieredId
         if (v.value != value)
             pre();
         JS_ASSERT(!IsPoisonedId(v.value));
         value = v.value;
         return *this;
     }
 };
 
-/*
- * A pre- and post-barriered heap jsid, for use inside the JS engine.
- *
- * Not to be confused with JS::Heap<jsid>.
- */
+// Like HeapPtr, but specialized for jsid.
+// See the comments on that class for details.
 class HeapId : public BarrieredId
 {
   public:
     explicit HeapId() : BarrieredId(JSID_VOID) {}
 
     explicit HeapId(jsid id)
       : BarrieredId(id)
     {
@@ -1125,17 +1162,16 @@ class HeapId : public BarrieredId
         return *this;
     }
 
   private:
     void post() {};
 
     HeapId(const HeapId &v) MOZ_DELETE;
 
-    /* The default move construction and assignment operators would be incorrect. */
     HeapId(HeapId &&) MOZ_DELETE;
     HeapId &operator=(HeapId &&) MOZ_DELETE;
 };
 
 /*
  * 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