Move rtc::AtomicOps to webrtc/base/atomicops.h.

Removes FixedSizeLockFreeQueue which isn't used anymore. This enabled
moving rtc::AtomicOps to webrtc/base/atomicops.h where they should be.

BUG=4330
R=tommi@webrtc.org

Review URL: https://webrtc-codereview.appspot.com/51789004

Cr-Commit-Position: refs/heads/master@{#9120}

webrtc/base/BUILD.gn

@@ -105,6 +105,7 @@ static_library("rtc_base_approved") {
   public_configs = [ "..:common_inherited_config" ]
 
   sources = [
+    "atomicops.h",
     "bitbuffer.cc",
     "bitbuffer.h",
     "buffer.cc",
@@ -344,7 +345,6 @@ static_library("rtc_base") {
       "asyncinvoker.cc",
       "asyncinvoker.h",
       "asyncinvoker-inl.h",
-      "atomicops.h",
       "bandwidthsmoother.cc",
       "bandwidthsmoother.h",
       "bind.h",

webrtc/base/atomicops.h

@@ -11,139 +11,60 @@
 #ifndef WEBRTC_BASE_ATOMICOPS_H_
 #define WEBRTC_BASE_ATOMICOPS_H_
 
-#include <string>
+#if defined(WEBRTC_WIN)
-
+// Include winsock2.h before including <windows.h> to maintain consistency with
-#include "webrtc/base/basictypes.h"
+// win32.h.  We can't include win32.h directly here since it pulls in
-#include "webrtc/base/common.h"
+// headers such as basictypes.h which causes problems in Chromium where webrtc
-#include "webrtc/base/logging.h"
+// exists as two separate projects, webrtc and libjingle.
-#include "webrtc/base/scoped_ptr.h"
+#include <winsock2.h>
+#include <windows.h>
+#endif  // defined(WEBRTC_WIN)
 
 namespace rtc {
-
+class AtomicOps {
-// A single-producer, single-consumer, fixed-size queue.
-// All methods not ending in Unsafe can be safely called without locking,
-// provided that calls to consumer methods (Peek/Pop) or producer methods (Push)
-// only happen on a single thread per method type. If multiple threads need to
-// read simultaneously or write simultaneously, other synchronization is
-// necessary. Synchronization is also required if a call into any Unsafe method
-// could happen at the same time as a call to any other method.
-template <typename T>
-class FixedSizeLockFreeQueue {
- private:
-// Atomic primitives and memory barrier
-#if defined(__arm__)
-  typedef uint32 Atomic32;
-
-  // Copied from google3/base/atomicops-internals-arm-v6plus.h
-  static inline void MemoryBarrier() {
-    asm volatile("dmb":::"memory");
-  }
-
-  // Adapted from google3/base/atomicops-internals-arm-v6plus.h
-  static inline void AtomicIncrement(volatile Atomic32* ptr) {
-    Atomic32 str_success, value;
-    asm volatile (
-        "1:\n"
-        "ldrex  %1, [%2]\n"
-        "add    %1, %1, #1\n"
-        "strex  %0, %1, [%2]\n"
-        "teq    %0, #0\n"
-        "bne    1b"
-        : "=&r"(str_success), "=&r"(value)
-        : "r" (ptr)
-        : "cc", "memory");
-  }
-#elif !defined(SKIP_ATOMIC_CHECK)
-#error "No atomic operations defined for the given architecture."
-#endif
-
  public:
-  // Constructs an empty queue, with capacity 0.
+#if defined(WEBRTC_WIN)
-  FixedSizeLockFreeQueue() : pushed_count_(0),
+  // Assumes sizeof(int) == sizeof(LONG), which it is on Win32 and Win64.
-                             popped_count_(0),
+  static int Increment(volatile int* i) {
-                             capacity_(0),
+    return ::InterlockedIncrement(reinterpret_cast<volatile LONG*>(i));
-                             data_() {}
-  // Constructs an empty queue with the given capacity.
-  FixedSizeLockFreeQueue(size_t capacity) : pushed_count_(0),
-                                            popped_count_(0),
-                                            capacity_(capacity),
-                                            data_(new T[capacity]) {}
-
-  // Pushes a value onto the queue. Returns true if the value was successfully
-  // pushed (there was space in the queue). This method can be safely called at
-  // the same time as PeekFront/PopFront.
-  bool PushBack(T value) {
-    if (capacity_ == 0) {
-      LOG(LS_WARNING) << "Queue capacity is 0.";
-      return false;
-    }
-    if (IsFull()) {
-      return false;
-    }
-
-    data_[pushed_count_ % capacity_] = value;
-    // Make sure the data is written before the count is incremented, so other
-    // threads can't see the value exists before being able to read it.
-    MemoryBarrier();
-    AtomicIncrement(&pushed_count_);
-    return true;
   }
-
+  static int Decrement(volatile int* i) {
-  // Retrieves the oldest value pushed onto the queue. Returns true if there was
+    return ::InterlockedDecrement(reinterpret_cast<volatile LONG*>(i));
-  // an item to peek (the queue was non-empty). This method can be safely called
-  // at the same time as PushBack.
-  bool PeekFront(T* value_out) {
-    if (capacity_ == 0) {
-      LOG(LS_WARNING) << "Queue capacity is 0.";
-      return false;
-    }
-    if (IsEmpty()) {
-      return false;
-    }
-
-    *value_out = data_[popped_count_ % capacity_];
-    return true;
   }
-
+  static int Load(volatile const int* i) {
-  // Retrieves the oldest value pushed onto the queue and removes it from the
+    return *i;
-  // queue. Returns true if there was an item to pop (the queue was non-empty).
-  // This method can be safely called at the same time as PushBack.
-  bool PopFront(T* value_out) {
-    if (PeekFront(value_out)) {
-      AtomicIncrement(&popped_count_);
-      return true;
-    }
-    return false;
   }
-
+  static void Store(volatile int* i, int value) {
-  // Clears the current items in the queue and sets the new (fixed) size. This
+    *i = value;
-  // method cannot be called at the same time as any other method.
-  void ClearAndResizeUnsafe(int new_capacity) {
-    capacity_ = new_capacity;
-    data_.reset(new T[new_capacity]);
-    pushed_count_ = 0;
-    popped_count_ = 0;
   }
-
+  static int CompareAndSwap(volatile int* i, int old_value, int new_value) {
-  // Returns true if there is no space left in the queue for new elements.
+    return ::InterlockedCompareExchange(reinterpret_cast<volatile LONG*>(i),
-  int IsFull() const { return pushed_count_ == popped_count_ + capacity_; }
+                                        new_value,
-  // Returns true if there are no elements in the queue.
+                                        old_value);
-  int IsEmpty() const { return pushed_count_ == popped_count_; }
+  }
-  // Returns the current number of elements in the queue. This is always in the
+#else
-  // range [0, capacity]
+  static int Increment(volatile int* i) {
-  size_t Size() const { return pushed_count_ - popped_count_; }
+    return __sync_add_and_fetch(i, 1);
-
+  }
-  // Returns the capacity of the queue (max size).
+  static int Decrement(volatile int* i) {
-  size_t capacity() const { return capacity_; }
+    return __sync_sub_and_fetch(i, 1);
-
+  }
- private:
+  static int Load(volatile const int* i) {
-  volatile Atomic32 pushed_count_;
+    // Adding 0 is a no-op, so const_cast is fine.
-  volatile Atomic32 popped_count_;
+    return __sync_add_and_fetch(const_cast<volatile int*>(i), 0);
-  size_t capacity_;
+  }
-  rtc::scoped_ptr<T[]> data_;
+  static void Store(volatile int* i, int value) {
-  DISALLOW_COPY_AND_ASSIGN(FixedSizeLockFreeQueue);
+    __sync_synchronize();
+    *i = value;
+  }
+  static int CompareAndSwap(volatile int* i, int old_value, int new_value) {
+    return __sync_val_compare_and_swap(i, old_value, new_value);
+  }
+#endif
 };
 
+
+
 }
 
 #endif  // WEBRTC_BASE_ATOMICOPS_H_

webrtc/base/atomicops_unittest.cc

@@ -8,72 +8,5 @@
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
-#if !defined(__arm__)
+// TODO(pbos): Move AtomicOps tests to here from
-// For testing purposes, define faked versions of the atomic operations
+// webrtc/base/criticalsection_unittest.cc.
-#include "webrtc/base/basictypes.h"
-namespace rtc {
-typedef uint32 Atomic32;
-static inline void MemoryBarrier() { }
-static inline void AtomicIncrement(volatile Atomic32* ptr) {
-  *ptr = *ptr + 1;
-}
-}
-#define SKIP_ATOMIC_CHECK
-#endif
-
-#include "webrtc/base/atomicops.h"
-#include "webrtc/base/gunit.h"
-#include "webrtc/base/helpers.h"
-#include "webrtc/base/logging.h"
-
-TEST(FixedSizeLockFreeQueueTest, TestDefaultConstruct) {
-  rtc::FixedSizeLockFreeQueue<int> queue;
-  EXPECT_EQ(0u, queue.capacity());
-  EXPECT_EQ(0u, queue.Size());
-  EXPECT_FALSE(queue.PushBack(1));
-  int val;
-  EXPECT_FALSE(queue.PopFront(&val));
-}
-
-TEST(FixedSizeLockFreeQueueTest, TestConstruct) {
-  rtc::FixedSizeLockFreeQueue<int> queue(5);
-  EXPECT_EQ(5u, queue.capacity());
-  EXPECT_EQ(0u, queue.Size());
-  int val;
-  EXPECT_FALSE(queue.PopFront(&val));
-}
-
-TEST(FixedSizeLockFreeQueueTest, TestPushPop) {
-  rtc::FixedSizeLockFreeQueue<int> queue(2);
-  EXPECT_EQ(2u, queue.capacity());
-  EXPECT_EQ(0u, queue.Size());
-  EXPECT_TRUE(queue.PushBack(1));
-  EXPECT_EQ(1u, queue.Size());
-  EXPECT_TRUE(queue.PushBack(2));
-  EXPECT_EQ(2u, queue.Size());
-  EXPECT_FALSE(queue.PushBack(3));
-  EXPECT_EQ(2u, queue.Size());
-  int val;
-  EXPECT_TRUE(queue.PopFront(&val));
-  EXPECT_EQ(1, val);
-  EXPECT_EQ(1u, queue.Size());
-  EXPECT_TRUE(queue.PopFront(&val));
-  EXPECT_EQ(2, val);
-  EXPECT_EQ(0u, queue.Size());
-  EXPECT_FALSE(queue.PopFront(&val));
-  EXPECT_EQ(0u, queue.Size());
-}
-
-TEST(FixedSizeLockFreeQueueTest, TestResize) {
-  rtc::FixedSizeLockFreeQueue<int> queue(2);
-  EXPECT_EQ(2u, queue.capacity());
-  EXPECT_EQ(0u, queue.Size());
-  EXPECT_TRUE(queue.PushBack(1));
-  EXPECT_EQ(1u, queue.Size());
-
-  queue.ClearAndResizeUnsafe(5);
-  EXPECT_EQ(5u, queue.capacity());
-  EXPECT_EQ(0u, queue.Size());
-  int val;
-  EXPECT_FALSE(queue.PopFront(&val));
-}

webrtc/base/base.gyp

@@ -31,6 +31,7 @@
       'sources': [
         '../overrides/webrtc/base/basictypes.h',
         '../overrides/webrtc/base/constructormagic.h',
+        'atomicops.h',
         'basictypes.h',
         'bitbuffer.cc',
         'bitbuffer.h',
@@ -118,7 +119,6 @@
         'asynctcpsocket.h',
         'asyncudpsocket.cc',
         'asyncudpsocket.h',
-        'atomicops.h',
         'autodetectproxy.cc',
         'autodetectproxy.h',
         'bandwidthsmoother.cc',

webrtc/base/criticalsection.h

@@ -8,9 +8,10 @@
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
-#ifndef WEBRTC_BASE_CRITICALSECTION_H__
+#ifndef WEBRTC_BASE_CRITICALSECTION_H_
-#define WEBRTC_BASE_CRITICALSECTION_H__
+#define WEBRTC_BASE_CRITICALSECTION_H_
 
+#include "webrtc/base/atomicops.h"
 #include "webrtc/base/constructormagic.h"
 #include "webrtc/base/thread_annotations.h"
 
@@ -97,51 +98,6 @@ class TryCritScope {
   DISALLOW_COPY_AND_ASSIGN(TryCritScope);
 };
 
-// TODO: Move this to atomicops.h, which can't be done easily because of
-// complex compile rules.
-class AtomicOps {
- public:
-#if defined(WEBRTC_WIN)
-  // Assumes sizeof(int) == sizeof(LONG), which it is on Win32 and Win64.
-  static int Increment(volatile int* i) {
-    return ::InterlockedIncrement(reinterpret_cast<volatile LONG*>(i));
-  }
-  static int Decrement(volatile int* i) {
-    return ::InterlockedDecrement(reinterpret_cast<volatile LONG*>(i));
-  }
-  static int Load(volatile const int* i) {
-    return *i;
-  }
-  static void Store(volatile int* i, int value) {
-    *i = value;
-  }
-  static int CompareAndSwap(volatile int* i, int old_value, int new_value) {
-    return ::InterlockedCompareExchange(reinterpret_cast<volatile LONG*>(i),
-                                        new_value,
-                                        old_value);
-  }
-#else
-  static int Increment(volatile int* i) {
-    return __sync_add_and_fetch(i, 1);
-  }
-  static int Decrement(volatile int* i) {
-    return __sync_sub_and_fetch(i, 1);
-  }
-  static int Load(volatile const int* i) {
-    // Adding 0 is a no-op, so const_cast is fine.
-    return __sync_add_and_fetch(const_cast<volatile int*>(i), 0);
-  }
-  static void Store(volatile int* i, int value) {
-    __sync_synchronize();
-    *i = value;
-  }
-  static int CompareAndSwap(volatile int* i, int old_value, int new_value) {
-    return __sync_val_compare_and_swap(i, old_value, new_value);
-  }
-#endif
-};
-
-
 // A POD lock used to protect global variables. Do NOT use for other purposes.
 // No custom constructor or private data member should be added.
 class LOCKABLE GlobalLockPod {
@@ -160,4 +116,4 @@ class GlobalLock : public GlobalLockPod {
 
 } // namespace rtc
 
-#endif // WEBRTC_BASE_CRITICALSECTION_H__
+#endif // WEBRTC_BASE_CRITICALSECTION_H_

webrtc/base/refcount.h

@@ -8,12 +8,12 @@
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
-#ifndef TALK_APP_BASE_REFCOUNT_H_
+#ifndef WEBRTC_BASE_REFCOUNT_H_
-#define TALK_APP_BASE_REFCOUNT_H_
+#define WEBRTC_BASE_REFCOUNT_H_
 
 #include <string.h>
 
-#include "webrtc/base/criticalsection.h"
+#include "webrtc/base/atomicops.h"
 
 namespace rtc {
 
@@ -126,4 +126,4 @@ class RefCountedObject : public T {
 
 }  // namespace rtc
 
-#endif  // TALK_APP_BASE_REFCOUNT_H_
+#endif  // WEBRTC_BASE_REFCOUNT_H_

webrtc/modules/video_render/incoming_video_stream.h

@@ -12,6 +12,7 @@
 #define WEBRTC_MODULES_VIDEO_RENDER_MAIN_SOURCE_INCOMING_VIDEO_STREAM_H_
 
 #include "webrtc/base/scoped_ptr.h"
+#include "webrtc/base/thread_annotations.h"
 #include "webrtc/modules/video_render/include/video_render.h"
 
 namespace webrtc {

webrtc/system_wrappers/source/trace_impl.cc

@@ -15,6 +15,7 @@
 #include <stdio.h>
 #include <string.h>
 
+#include "webrtc/base/atomicops.h"
 #ifdef _WIN32
 #include "webrtc/system_wrappers/source/trace_win.h"
 #else

webrtc/video/video_send_stream_tests.cc

@@ -14,6 +14,7 @@
 
 #include "webrtc/base/bind.h"
 #include "webrtc/base/checks.h"
+#include "webrtc/base/criticalsection.h"
 #include "webrtc/base/scoped_ptr.h"
 #include "webrtc/call.h"
 #include "webrtc/frame_callback.h"