From ff019b0b551888330b69d6323506eae710e1ab6d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Peter=20Bostr=C3=B6m?= Date: Thu, 30 Apr 2015 14:16:07 +0200 Subject: [PATCH] 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 | 2 +- webrtc/base/atomicops.h | 169 +++++------------- webrtc/base/atomicops_unittest.cc | 71 +------- webrtc/base/base.gyp | 2 +- webrtc/base/criticalsection.h | 52 +----- webrtc/base/refcount.h | 8 +- .../video_render/incoming_video_stream.h | 1 + webrtc/system_wrappers/source/trace_impl.cc | 1 + webrtc/video/video_send_stream_tests.cc | 1 + 9 files changed, 60 insertions(+), 247 deletions(-) diff --git a/webrtc/base/BUILD.gn b/webrtc/base/BUILD.gn index 35d33ab99..a9868e931 100644 --- a/webrtc/base/BUILD.gn +++ b/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", diff --git a/webrtc/base/atomicops.h b/webrtc/base/atomicops.h index 6096e8c08..80b212444 100644 --- a/webrtc/base/atomicops.h +++ b/webrtc/base/atomicops.h @@ -11,139 +11,60 @@ #ifndef WEBRTC_BASE_ATOMICOPS_H_ #define WEBRTC_BASE_ATOMICOPS_H_ -#include - -#include "webrtc/base/basictypes.h" -#include "webrtc/base/common.h" -#include "webrtc/base/logging.h" -#include "webrtc/base/scoped_ptr.h" +#if defined(WEBRTC_WIN) +// Include winsock2.h before including to maintain consistency with +// win32.h. We can't include win32.h directly here since it pulls in +// headers such as basictypes.h which causes problems in Chromium where webrtc +// exists as two separate projects, webrtc and libjingle. +#include +#include +#endif // defined(WEBRTC_WIN) namespace rtc { - -// 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 -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 - +class AtomicOps { public: - // Constructs an empty queue, with capacity 0. - FixedSizeLockFreeQueue() : pushed_count_(0), - popped_count_(0), - capacity_(0), - 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; +#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(i)); } - - // Retrieves the oldest value pushed onto the queue. Returns true if there was - // 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 Decrement(volatile int* i) { + return ::InterlockedDecrement(reinterpret_cast(i)); } - - // Retrieves the oldest value pushed onto the queue and removes it from the - // 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 int Load(volatile const int* i) { + return *i; } - - // Clears the current items in the queue and sets the new (fixed) size. This - // 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 void Store(volatile int* i, int value) { + *i = value; } - - // Returns true if there is no space left in the queue for new elements. - int IsFull() const { return pushed_count_ == popped_count_ + capacity_; } - // Returns true if there are no elements in the queue. - int IsEmpty() const { return pushed_count_ == popped_count_; } - // Returns the current number of elements in the queue. This is always in the - // range [0, capacity] - size_t Size() const { return pushed_count_ - popped_count_; } - - // Returns the capacity of the queue (max size). - size_t capacity() const { return capacity_; } - - private: - volatile Atomic32 pushed_count_; - volatile Atomic32 popped_count_; - size_t capacity_; - rtc::scoped_ptr data_; - DISALLOW_COPY_AND_ASSIGN(FixedSizeLockFreeQueue); + static int CompareAndSwap(volatile int* i, int old_value, int new_value) { + return ::InterlockedCompareExchange(reinterpret_cast(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(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 }; + + } #endif // WEBRTC_BASE_ATOMICOPS_H_ diff --git a/webrtc/base/atomicops_unittest.cc b/webrtc/base/atomicops_unittest.cc index 5152c4de6..2aa28cac6 100644 --- a/webrtc/base/atomicops_unittest.cc +++ b/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__) -// For testing purposes, define faked versions of the atomic operations -#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 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 queue(5); - EXPECT_EQ(5u, queue.capacity()); - EXPECT_EQ(0u, queue.Size()); - int val; - EXPECT_FALSE(queue.PopFront(&val)); -} - -TEST(FixedSizeLockFreeQueueTest, TestPushPop) { - rtc::FixedSizeLockFreeQueue 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 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)); -} +// TODO(pbos): Move AtomicOps tests to here from +// webrtc/base/criticalsection_unittest.cc. diff --git a/webrtc/base/base.gyp b/webrtc/base/base.gyp index 51b72917d..76d85a760 100644 --- a/webrtc/base/base.gyp +++ b/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', diff --git a/webrtc/base/criticalsection.h b/webrtc/base/criticalsection.h index d4e6bd76e..bd45d85bc 100644 --- a/webrtc/base/criticalsection.h +++ b/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__ -#define WEBRTC_BASE_CRITICALSECTION_H__ +#ifndef 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(i)); - } - static int Decrement(volatile int* i) { - return ::InterlockedDecrement(reinterpret_cast(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(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(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_ diff --git a/webrtc/base/refcount.h b/webrtc/base/refcount.h index e7306d3df..79b0591b2 100644 --- a/webrtc/base/refcount.h +++ b/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_ -#define TALK_APP_BASE_REFCOUNT_H_ +#ifndef WEBRTC_BASE_REFCOUNT_H_ +#define WEBRTC_BASE_REFCOUNT_H_ #include -#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_ diff --git a/webrtc/modules/video_render/incoming_video_stream.h b/webrtc/modules/video_render/incoming_video_stream.h index 47db31b91..c636e13a4 100644 --- a/webrtc/modules/video_render/incoming_video_stream.h +++ b/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 { diff --git a/webrtc/system_wrappers/source/trace_impl.cc b/webrtc/system_wrappers/source/trace_impl.cc index 6a293ccbd..e6032d507 100644 --- a/webrtc/system_wrappers/source/trace_impl.cc +++ b/webrtc/system_wrappers/source/trace_impl.cc @@ -15,6 +15,7 @@ #include #include +#include "webrtc/base/atomicops.h" #ifdef _WIN32 #include "webrtc/system_wrappers/source/trace_win.h" #else diff --git a/webrtc/video/video_send_stream_tests.cc b/webrtc/video/video_send_stream_tests.cc index d2a81fc8b..1821db495 100644 --- a/webrtc/video/video_send_stream_tests.cc +++ b/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"