Bug 888548 - Part 2: Refactor and cleanup mozilla::Atomic<T> implementation. r=froydnj
authorBirunthan Mohanathas <birunthan@mohanathas.com>
Thu, 01 Aug 2013 21:21:32 -0400
changeset 153328 99511f74122542a4227c8efdb61fa3cd54b10b2b
parent 153327 dbce2a590a2bfd6e48f725c6cc9bfb751eb3e057
child 153329 424e1cb4850cb38b6ab289bf50de32f47c92cf9e
push id2859
push userakeybl@mozilla.com
push dateMon, 16 Sep 2013 19:14:59 +0000
treeherdermozilla-beta@87d3c51cd2bf [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfroydnj
bugs888548
milestone25.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 888548 - Part 2: Refactor and cleanup mozilla::Atomic<T> implementation. r=froydnj This moves the increment and decrement operators from detail::AtomicBase to detail::AtomicBaseIncDec and moves the implementation of the assignment operator into detail::AtomicBase. Additionally, this changes the integral implementation to use mozilla::EnableIf for its specialization.
mfbt/Atomics.h
--- a/mfbt/Atomics.h
+++ b/mfbt/Atomics.h
@@ -825,38 +825,44 @@ struct AtomicIntrinsics<T*, Order> : pub
 
 namespace mozilla {
 
 namespace detail {
 
 template<typename T, MemoryOrdering Order>
 class AtomicBase
 {
+    // We only support 32-bit types on 32-bit Windows, which constrains our
+    // implementation elsewhere.  But we support pointer-sized types everywhere.
+    static_assert(sizeof(T) == 4 || (sizeof(uintptr_t) == 8 && sizeof(T) == 8),
+                  "mozilla/Atomics.h only supports 32-bit and pointer-sized types");
+
   protected:
     typedef typename detail::AtomicIntrinsics<T, Order> Intrinsics;
     typename Intrinsics::ValueType mValue;
 
   public:
     AtomicBase() : mValue() {}
     AtomicBase(T aInit) { Intrinsics::store(mValue, aInit); }
 
-    T operator++(int) { return Intrinsics::inc(mValue); }
-    T operator--(int) { return Intrinsics::dec(mValue); }
-    T operator++() { return Intrinsics::inc(mValue) + 1; }
-    T operator--() { return Intrinsics::dec(mValue) - 1; }
+    operator T() const { return Intrinsics::load(mValue); }
 
-    operator T() const { return Intrinsics::load(mValue); }
+    T operator=(T aValue) {
+      Intrinsics::store(mValue, aValue);
+      return aValue;
+    }
 
     /**
      * Performs an atomic swap operation.  aValue is stored and the previous
      * value of this variable is returned.
      */
     T exchange(T aValue) {
       return Intrinsics::exchange(mValue, aValue);
     }
+
     /**
      * Performs an atomic compare-and-swap operation and returns true if it
      * succeeded. This is equivalent to atomically doing
      *
      *   if (mValue == aOldValue) {
      *     mValue = aNewValue;
      *     return true;
      *   } else {
@@ -867,107 +873,116 @@ class AtomicBase
       return Intrinsics::compareExchange(mValue, aOldValue, aNewValue);
     }
 
   private:
     template<MemoryOrdering AnyOrder>
     AtomicBase(const AtomicBase<T, AnyOrder>& aCopy) MOZ_DELETE;
 };
 
+template<typename T, MemoryOrdering Order>
+class AtomicBaseIncDec : public AtomicBase<T, Order>
+{
+    typedef typename detail::AtomicBase<T, Order> Base;
+
+  public:
+    AtomicBaseIncDec() : Base() {}
+    AtomicBaseIncDec(T aInit) : Base(aInit) {}
+
+    using Base::operator=;
+
+    T operator++(int) { return Base::Intrinsics::inc(Base::mValue); }
+    T operator--(int) { return Base::Intrinsics::dec(Base::mValue); }
+    T operator++() { return Base::Intrinsics::inc(Base::mValue) + 1; }
+    T operator--() { return Base::Intrinsics::dec(Base::mValue) - 1; }
+
+  private:
+    template<MemoryOrdering AnyOrder>
+    AtomicBaseIncDec(const AtomicBaseIncDec<T, AnyOrder>& aCopy) MOZ_DELETE;
+};
+
 } // namespace detail
 
 /**
  * A wrapper for a type that enforces that all memory accesses are atomic.
  *
- * In general, where a variable |T foo| exists, |Atomic<T> foo| can be
- * used in its place.  In addition to atomic store and load operations,
- * compound assignment and increment/decrement operators are implemented
- * which perform the corresponding read-modify-write operation
- * atomically.  Finally, an atomic swap method is provided.
+ * In general, where a variable |T foo| exists, |Atomic<T> foo| can be used in
+ * its place.  Implementations for integral and pointer types are provided
+ * below.
  *
  * Atomic accesses are sequentially consistent by default.  You should
  * use the default unless you are tall enough to ride the
  * memory-ordering roller coaster (if you're not sure, you aren't) and
  * you have a compelling reason to do otherwise.
  *
  * There is one exception to the case of atomic memory accesses: providing an
  * initial value of the atomic value is not guaranteed to be atomic.  This is a
  * deliberate design choice that enables static atomic variables to be declared
  * without introducing extra static constructors.
  */
-template<typename T, MemoryOrdering Order = SequentiallyConsistent>
-class Atomic : public detail::AtomicBase<T, Order>
+template<typename T,
+         MemoryOrdering Order = SequentiallyConsistent,
+         typename Enable = void>
+class Atomic;
+
+/**
+ * Atomic<T> implementation for integral types.
+ *
+ * In addition to atomic store and load operations, compound assignment and
+ * increment/decrement operators are implemented which perform the
+ * corresponding read-modify-write operation atomically.  Finally, an atomic
+ * swap method is provided.
+ */
+template<typename T, MemoryOrdering Order>
+class Atomic<T, Order, typename EnableIf<IsIntegral<T>::value>::Type>
+  : public detail::AtomicBaseIncDec<T, Order>
 {
-    // We only support 32-bit types on 32-bit Windows, which constrains our
-    // implementation elsewhere.  But we support pointer-sized types everywhere.
-    static_assert(sizeof(T) == 4 || (sizeof(uintptr_t) == 8 && sizeof(T) == 8),
-                  "mozilla/Atomics.h only supports 32-bit and pointer-sized types");
-    // Regardless of the OS, we only support integral types here.
-    static_assert(IsIntegral<T>::value, "can only have integral atomic variables");
-
-    typedef typename detail::AtomicBase<T, Order> Base;
+    typedef typename detail::AtomicBaseIncDec<T, Order> Base;
 
   public:
-    Atomic() : detail::AtomicBase<T, Order>() {}
-    Atomic(T aInit) : detail::AtomicBase<T, Order>(aInit) {}
+    Atomic() : Base() {}
+    Atomic(T aInit) : Base(aInit) {}
+
+    using Base::operator=;
 
     T operator+=(T delta) { return Base::Intrinsics::add(Base::mValue, delta) + delta; }
     T operator-=(T delta) { return Base::Intrinsics::sub(Base::mValue, delta) - delta; }
     T operator|=(T val) { return Base::Intrinsics::or_(Base::mValue, val) | val; }
     T operator^=(T val) { return Base::Intrinsics::xor_(Base::mValue, val) ^ val; }
     T operator&=(T val) { return Base::Intrinsics::and_(Base::mValue, val) & val; }
 
-    T operator=(T aValue) {
-      Base::Intrinsics::store(Base::mValue, aValue);
-      return aValue;
-    }
-
   private:
     Atomic(Atomic<T, Order>& aOther) MOZ_DELETE;
 };
 
 /**
- * A partial specialization of Atomic for pointer variables.
- *
- * Like Atomic<T>, Atomic<T*> is equivalent in most respects to a regular T*
- * variable.  An atomic compare-and-swap primitive for pointer variables is
- * provided, as are atomic increment and decement operators.  Also provided
- * are the compound assignment operators for addition and subtraction.
- * Atomic swap (via exchange()) is included as well.
+ * Atomic<T> implementation for pointer types.
  *
- * Atomic accesses are sequentially consistent by default.  You should
- * use the default unless you are tall enough to ride the
- * memory-ordering roller coaster (if you're not sure, you aren't) and
- * you have a compelling reason to do otherwise.
- *
- * There is one exception to the case of atomic memory accesses: providing an
- * initial value of the atomic value is not guaranteed to be atomic. This is a
- * deliberate design choice that enables static atomic variables to be declared
- * without introducing extra static constructors.
+ * An atomic compare-and-swap primitive for pointer variables is provided, as
+ * are atomic increment and decement operators.  Also provided are the compound
+ * assignment operators for addition and subtraction. Atomic swap (via
+ * exchange()) is included as well.
  */
 template<typename T, MemoryOrdering Order>
-class Atomic<T*, Order> : public detail::AtomicBase<T*, Order>
+class Atomic<T*, Order> : public detail::AtomicBaseIncDec<T*, Order>
 {
-    typedef typename detail::AtomicBase<T*, Order> Base;
+    typedef typename detail::AtomicBaseIncDec<T*, Order> Base;
 
   public:
-    Atomic() : detail::AtomicBase<T*, Order>() {}
-    Atomic(T* aInit) : detail::AtomicBase<T*, Order>(aInit) {}
+    Atomic() : Base() {}
+    Atomic(T* aInit) : Base(aInit) {}
 
-    T* operator +=(ptrdiff_t delta) {
+    using Base::operator=;
+
+    T* operator+=(ptrdiff_t delta) {
       return Base::Intrinsics::add(Base::mValue, delta) + delta;
     }
-    T* operator -=(ptrdiff_t delta) {
+    T* operator-=(ptrdiff_t delta) {
       return Base::Intrinsics::sub(Base::mValue, delta) - delta;
     }
 
-    T* operator=(T* aValue) {
-      Base::Intrinsics::store(Base::mValue, aValue);
-      return aValue;
-    }
-
   private:
     Atomic(Atomic<T*, Order>& aOther) MOZ_DELETE;
 };
 
 } // namespace mozilla
 
 #endif /* mozilla_Atomics_h */