From ae605642a625b6583ee0ce44a85a57adacbf4320 Mon Sep 17 00:00:00 2001 From: Nathan Froyd Date: Wed, 8 Feb 2017 20:52:55 -0500 Subject: [PATCH] Bug 1312086 - part 2 - separate out a ConditionVariableImpl class; r=fitzgen For moving the platform-specific bits of Mutex and ConditionVariable to mozglue, we'd rather not have to bring along things like js::LockGuard. To do that, we need to separate out the platform-specific bits of ConditionVariable into a ConditionVariableImpl class, similar to what's already done for Mutex. Although we won't have type-checking to ensure that we pass in locked mutexes for the wait* methods, we expect higher-level implementations to take care of those details. --- js/src/threading/ConditionVariable.h | 77 ++++++++++++++----- js/src/threading/Mutex.h | 6 +- js/src/threading/posix/ConditionVariable.cpp | 24 +++--- .../threading/windows/ConditionVariable.cpp | 24 +++--- 4 files changed, 85 insertions(+), 46 deletions(-) diff --git a/js/src/threading/ConditionVariable.h b/js/src/threading/ConditionVariable.h index 35eeaeeb08b8..976cd1937a9e 100644 --- a/js/src/threading/ConditionVariable.h +++ b/js/src/threading/ConditionVariable.h @@ -21,21 +21,19 @@ namespace js { -template using UniqueLock = LockGuard; - enum class CVStatus { NoTimeout, Timeout }; -// A poly-fill for std::condition_variable. -class ConditionVariable -{ +namespace detail { + +class ConditionVariableImpl { public: struct PlatformData; - ConditionVariable(); - ~ConditionVariable(); + ConditionVariableImpl(); + ~ConditionVariableImpl(); // Wake one thread that is waiting on this condition. void notify_one(); @@ -45,7 +43,55 @@ public: // Block the current thread of execution until this condition variable is // woken from another thread via notify_one or notify_all. - void wait(UniqueLock& lock); + void wait(Mutex& lock); + + CVStatus wait_for(Mutex& lock, + const mozilla::TimeDuration& rel_time); + +private: + ConditionVariableImpl(const ConditionVariableImpl&) = delete; + ConditionVariableImpl& operator=(const ConditionVariableImpl&) = delete; + + PlatformData* platformData(); + +#ifndef XP_WIN + void* platformData_[sizeof(pthread_cond_t) / sizeof(void*)]; + static_assert(sizeof(pthread_cond_t) / sizeof(void*) != 0 && + sizeof(pthread_cond_t) % sizeof(void*) == 0, + "pthread_cond_t must have pointer alignment"); +#else + void* platformData_[4]; +#endif +}; + +} // namespace detail + +template using UniqueLock = LockGuard; + +// A poly-fill for std::condition_variable. +class ConditionVariable +{ +public: + struct PlatformData; + + ConditionVariable() = default; + ~ConditionVariable() = default; + + // Wake one thread that is waiting on this condition. + void notify_one() { + impl_.notify_one(); + } + + // Wake all threads that are waiting on this condition. + void notify_all() { + impl_.notify_all(); + } + + // Block the current thread of execution until this condition variable is + // woken from another thread via notify_one or notify_all. + void wait(UniqueLock& lock) { + impl_.wait(lock.lock); + } // As with |wait|, block the current thread of execution until woken from // another thread. This method will resume waiting once woken until the given @@ -88,7 +134,9 @@ public: // has a minimum granularity of the system's scheduling interval, and may // encounter substantially longer delays, depending on system load. CVStatus wait_for(UniqueLock& lock, - const mozilla::TimeDuration& rel_time); + const mozilla::TimeDuration& rel_time) { + return impl_.wait_for(lock.lock, rel_time); + } // As with |wait_for|, block the current thread of execution until woken from // another thread or the given time duration has elapsed. This method will @@ -106,16 +154,7 @@ private: ConditionVariable(const ConditionVariable&) = delete; ConditionVariable& operator=(const ConditionVariable&) = delete; - PlatformData* platformData(); - -#ifndef XP_WIN - void* platformData_[sizeof(pthread_cond_t) / sizeof(void*)]; - static_assert(sizeof(pthread_cond_t) / sizeof(void*) != 0 && - sizeof(pthread_cond_t) % sizeof(void*) == 0, - "pthread_cond_t must have pointer alignment"); -#else - void* platformData_[4]; -#endif + detail::ConditionVariableImpl impl_; }; } // namespace js diff --git a/js/src/threading/Mutex.h b/js/src/threading/Mutex.h index 2b300842f126..f6b0ea0a3b63 100644 --- a/js/src/threading/Mutex.h +++ b/js/src/threading/Mutex.h @@ -14,10 +14,10 @@ namespace js { -class ConditionVariable; - namespace detail { +class ConditionVariableImpl; + class MutexImpl { public: @@ -57,7 +57,7 @@ private: void* platformData_[64 / sizeof(void*)]; #endif - friend class js::ConditionVariable; + friend class js::detail::ConditionVariableImpl; }; } // namespace detail diff --git a/js/src/threading/posix/ConditionVariable.cpp b/js/src/threading/posix/ConditionVariable.cpp index cd0a35b46e04..2dd69739ba03 100644 --- a/js/src/threading/posix/ConditionVariable.cpp +++ b/js/src/threading/posix/ConditionVariable.cpp @@ -61,12 +61,12 @@ moz_timespecadd(struct timespec* lhs, struct timespec* rhs, struct timespec* res } #endif -struct js::ConditionVariable::PlatformData +struct js::detail::ConditionVariableImpl::PlatformData { pthread_cond_t ptCond; }; -js::ConditionVariable::ConditionVariable() +js::detail::ConditionVariableImpl::ConditionVariableImpl() { pthread_cond_t* ptCond = &platformData()->ptCond; @@ -89,39 +89,39 @@ js::ConditionVariable::ConditionVariable() #endif } -js::ConditionVariable::~ConditionVariable() +js::detail::ConditionVariableImpl::~ConditionVariableImpl() { int r = pthread_cond_destroy(&platformData()->ptCond); MOZ_RELEASE_ASSERT(r == 0); } void -js::ConditionVariable::notify_one() +js::detail::ConditionVariableImpl::notify_one() { int r = pthread_cond_signal(&platformData()->ptCond); MOZ_RELEASE_ASSERT(r == 0); } void -js::ConditionVariable::notify_all() +js::detail::ConditionVariableImpl::notify_all() { int r = pthread_cond_broadcast(&platformData()->ptCond); MOZ_RELEASE_ASSERT(r == 0); } void -js::ConditionVariable::wait(UniqueLock& lock) +js::detail::ConditionVariableImpl::wait(Mutex& lock) { pthread_cond_t* ptCond = &platformData()->ptCond; - pthread_mutex_t* ptMutex = &lock.lock.platformData()->ptMutex; + pthread_mutex_t* ptMutex = &lock.platformData()->ptMutex; int r = pthread_cond_wait(ptCond, ptMutex); MOZ_RELEASE_ASSERT(r == 0); } js::CVStatus -js::ConditionVariable::wait_for(UniqueLock& lock, - const TimeDuration& a_rel_time) +js::detail::ConditionVariableImpl::wait_for(Mutex& lock, + const TimeDuration& a_rel_time) { if (a_rel_time == TimeDuration::Forever()) { wait(lock); @@ -129,7 +129,7 @@ js::ConditionVariable::wait_for(UniqueLock& lock, } pthread_cond_t* ptCond = &platformData()->ptCond; - pthread_mutex_t* ptMutex = &lock.lock.platformData()->ptMutex; + pthread_mutex_t* ptMutex = &lock.platformData()->ptMutex; int r; // Clamp to 0, as time_t is unsigned. @@ -164,8 +164,8 @@ js::ConditionVariable::wait_for(UniqueLock& lock, return CVStatus::Timeout; } -js::ConditionVariable::PlatformData* -js::ConditionVariable::platformData() +js::detail::ConditionVariableImpl::PlatformData* +js::detail::ConditionVariableImpl::platformData() { static_assert(sizeof platformData_ >= sizeof(PlatformData), "platformData_ is too small"); diff --git a/js/src/threading/windows/ConditionVariable.cpp b/js/src/threading/windows/ConditionVariable.cpp index 8c900751414d..8c27400891d9 100644 --- a/js/src/threading/windows/ConditionVariable.cpp +++ b/js/src/threading/windows/ConditionVariable.cpp @@ -29,41 +29,41 @@ #endif // Wrapper for native condition variable APIs. -struct js::ConditionVariable::PlatformData +struct js::ConditionVariableImpl::PlatformData { CONDITION_VARIABLE cv_; }; -js::ConditionVariable::ConditionVariable() +js::detail::ConditionVariableImpl::ConditionVariableImpl() { InitializeConditionVariable(&platformData()->cv_); } void -js::ConditionVariable::notify_one() +js::detail::ConditionVariableImpl::notify_one() { WakeConditionVariable(&platformData()->cv_); } void -js::ConditionVariable::notify_all() +js::detail::ConditionVariableImpl::notify_all() { WakeAllConditionVariable(&platformData()->cv_); } void -js::ConditionVariable::wait(UniqueLock& lock) +js::detail::ConditionVariableImpl::wait(Mutex& lock) { - CRITICAL_SECTION* cs = &lock.lock.platformData()->criticalSection; + CRITICAL_SECTION* cs = &lock.platformData()->criticalSection; bool r = SleepConditionVariableCS(&platformData()->cv_, cs, INFINITE); MOZ_RELEASE_ASSERT(r); } js::CVStatus -js::ConditionVariable::wait_for(UniqueLock& lock, - const mozilla::TimeDuration& rel_time) +js::detail::ConditionVariableImpl::wait_for(Mutex& lock, + const mozilla::TimeDuration& rel_time) { - CRITICAL_SECTION* cs = &lock.lock.platformData()->criticalSection; + CRITICAL_SECTION* cs = &lock.platformData()->criticalSection; // Note that DWORD is unsigned, so we have to be careful to clamp at 0. // If rel_time is Forever, then ToMilliseconds is +inf, which evaluates as @@ -82,13 +82,13 @@ js::ConditionVariable::wait_for(UniqueLock& lock, return CVStatus::Timeout; } -js::ConditionVariable::~ConditionVariable() +js::detail::ConditionVariableImpl::~ConditionVariableImpl() { // Native condition variables don't require cleanup. } -inline js::ConditionVariable::PlatformData* -js::ConditionVariable::platformData() +inline js::detail::ConditionVariableImpl::PlatformData* +js::detail::ConditionVariableImpl::platformData() { static_assert(sizeof platformData_ >= sizeof(PlatformData), "platformData_ is too small");