From 8c508334455629ed88bc49fd2c355d4af99f0c6a Mon Sep 17 00:00:00 2001 From: MerryMage Date: Thu, 14 Apr 2016 12:53:05 +0100 Subject: [PATCH 1/2] common/thread: Correct code style --- src/common/thread.h | 40 +++++++++++++++++++--------------------- 1 file changed, 19 insertions(+), 21 deletions(-) diff --git a/src/common/thread.h b/src/common/thread.h index 8255ee6d3..9bd0e8e7f 100644 --- a/src/common/thread.h +++ b/src/common/thread.h @@ -30,8 +30,7 @@ # endif #endif -namespace Common -{ +namespace Common { int CurrentThreadId(); @@ -43,55 +42,55 @@ public: Event() : is_set(false) {} void Set() { - std::lock_guard lk(m_mutex); + std::lock_guard lk(mutex); if (!is_set) { is_set = true; - m_condvar.notify_one(); + condvar.notify_one(); } } void Wait() { - std::unique_lock lk(m_mutex); - m_condvar.wait(lk, [&]{ return is_set; }); + std::unique_lock lk(mutex); + condvar.wait(lk, [&]{ return is_set; }); is_set = false; } void Reset() { - std::unique_lock lk(m_mutex); + std::unique_lock lk(mutex); // no other action required, since wait loops on the predicate and any lingering signal will get cleared on the first iteration is_set = false; } private: bool is_set; - std::condition_variable m_condvar; - std::mutex m_mutex; + std::condition_variable condvar; + std::mutex mutex; }; class Barrier { public: - Barrier(size_t count) : m_count(count), m_waiting(0) {} + explicit Barrier(size_t count_) : count(count_), waiting(0) {} /// Blocks until all "count" threads have called Sync() void Sync() { - std::unique_lock lk(m_mutex); + std::unique_lock lk(mutex); // TODO: broken when next round of Sync()s // is entered before all waiting threads return from the notify_all - if (++m_waiting == m_count) { - m_waiting = 0; - m_condvar.notify_all(); + if (++waiting == count) { + waiting = 0; + condvar.notify_all(); } else { - m_condvar.wait(lk, [&]{ return m_waiting == 0; }); + condvar.wait(lk, [&]{ return waiting == 0; }); } } private: - std::condition_variable m_condvar; - std::mutex m_mutex; - const size_t m_count; - size_t m_waiting; + std::condition_variable condvar; + std::mutex mutex; + const size_t count; + size_t waiting; }; void SleepCurrentThread(int ms); @@ -100,8 +99,7 @@ void SwitchCurrentThread(); // On Linux, this is equal to sleep 1ms // Use this function during a spin-wait to make the current thread // relax while another thread is working. This may be more efficient // than using events because event functions use kernel calls. -inline void YieldCPU() -{ +inline void YieldCPU() { std::this_thread::yield(); } From 3c710f9b104acf218bce1054bd325f9a2515f8ca Mon Sep 17 00:00:00 2001 From: MerryMage Date: Thu, 14 Apr 2016 12:54:06 +0100 Subject: [PATCH 2/2] Thread: Make Barrier reusable --- src/common/thread.h | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/common/thread.h b/src/common/thread.h index 9bd0e8e7f..bbfa8befa 100644 --- a/src/common/thread.h +++ b/src/common/thread.h @@ -69,20 +69,19 @@ private: class Barrier { public: - explicit Barrier(size_t count_) : count(count_), waiting(0) {} + explicit Barrier(size_t count_) : count(count_), waiting(0), generation(0) {} /// Blocks until all "count" threads have called Sync() void Sync() { std::unique_lock lk(mutex); - - // TODO: broken when next round of Sync()s - // is entered before all waiting threads return from the notify_all + const size_t current_generation = generation; if (++waiting == count) { + generation++; waiting = 0; condvar.notify_all(); } else { - condvar.wait(lk, [&]{ return waiting == 0; }); + condvar.wait(lk, [this, current_generation]{ return current_generation != generation; }); } } @@ -91,6 +90,7 @@ private: std::mutex mutex; const size_t count; size_t waiting; + size_t generation; // Incremented once each time the barrier is used }; void SleepCurrentThread(int ms);