New lock implementation for mac

webrtc/base/criticalsection.cc

 /*
  *  Copyright 2015 The WebRTC Project Authors. All rights reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
 #include "webrtc/base/criticalsection.h"
 
 #include "webrtc/base/checks.h"
 
 namespace rtc {
 
 CriticalSection::CriticalSection() {
 #if defined(WEBRTC_WIN)
   InitializeCriticalSection(&crit_);
 #else
+#if defined(WEBRTC_MAC)
+  lock_queue_ = 0;
+  owning_thread_ = 0;
+  recursion_ = 0;
+  semaphore_ = dispatch_semaphore_create(0);
+  RTC_DCHECK(semaphore_);

[torbjorng (webrtc), 2016/01/20 15:46:43]

DCHECK? Shouldn't we barf in all builds if this call fails?

[tommi, 2016/01/21 15:04:21]

I'll just remove this check.  It's almost akin to checking new(), which we don't
do.  We also don't check calls that instantiate OS based primitives such as
locks in general.

+#else
   pthread_mutexattr_t mutex_attribute;
   pthread_mutexattr_init(&mutex_attribute);
   pthread_mutexattr_settype(&mutex_attribute, PTHREAD_MUTEX_RECURSIVE);
   pthread_mutex_init(&mutex_, &mutex_attribute);
   pthread_mutexattr_destroy(&mutex_attribute);
+#endif
   CS_DEBUG_CODE(thread_ = 0);
   CS_DEBUG_CODE(recursion_count_ = 0);
 #endif
 }
 
 CriticalSection::~CriticalSection() {
 #if defined(WEBRTC_WIN)
   DeleteCriticalSection(&crit_);
 #else
+#if defined(WEBRTC_MAC)
+  dispatch_release(semaphore_);
+#else
   pthread_mutex_destroy(&mutex_);
 #endif
+#endif
 }
 
 void CriticalSection::Enter() EXCLUSIVE_LOCK_FUNCTION() {
 #if defined(WEBRTC_WIN)
   EnterCriticalSection(&crit_);
 #else
+#if defined(WEBRTC_MAC)
+  int spin = 2000;
+  do {
+    if (TryEnter())

[torbjorng (webrtc), 2016/01/20 15:46:43]

TryEnter seems to perform a CompareAndSwap unconditionally (except for same
thread). That will move the lock cache-line to this thread's current physical
core, putting the cache-line in "dirty" state. This is bad when we fail to get
the lock, since subsequently the lock owner will need to cache the lock
cache-line before unlocking.

I leaner poll just reads the lock, and then conditionally immediately attempts
to lock it.  The racing condition between the poll and attempted locking should
not matter (at least not if all contenders use the same non-aggressive
approach).

[tommi, 2016/01/21 15:04:21]

I rewrote the loop to not call TryEnter() but instead do this special case since
I don't think it's relevant to the TryEnter case (i.e. there's no spinning in
that case).

In my timing measurements I didn't see a significant enough difference to be
able to say with confidence that there's a clear win, but it's not slower at
least and I understand why it makes sense to do this.  Included cache line
barriers in my test code btw.

+      return;
+    sched_yield();
+  } while (--spin);
+
+  if (AtomicOps::Increment(&lock_queue_) > 0) {
+    // Owning thread cannot be the current thread since TryEnter() would
+    // have succeeded.
+    RTC_DCHECK(owning_thread_ != pthread_self());
+    // Wait for the lock to become available.
+    dispatch_semaphore_wait(semaphore_, DISPATCH_TIME_FOREVER);
+    RTC_DCHECK(owning_thread_ == 0);
+    RTC_DCHECK(!recursion_);
+    owning_thread_ = pthread_self();
+  }
+
+  RTC_DCHECK(owning_thread_ == pthread_self());
+  ++recursion_;
+
+#else
   pthread_mutex_lock(&mutex_);
+#endif
+
 #if CS_DEBUG_CHECKS
   if (!recursion_count_) {
     RTC_DCHECK(!thread_);
     thread_ = pthread_self();
   } else {
     RTC_DCHECK(CurrentThreadIsOwner());
   }
   ++recursion_count_;
 #endif
 #endif
 }
 
 bool CriticalSection::TryEnter() EXCLUSIVE_TRYLOCK_FUNCTION(true) {
 #if defined(WEBRTC_WIN)
   return TryEnterCriticalSection(&crit_) != FALSE;
 #else
+#if defined(WEBRTC_MAC)
+  if (owning_thread_ != pthread_self()) {
+    if (AtomicOps::CompareAndSwap(&lock_queue_, 0, 1) != 0)
+      return false;
+    owning_thread_ = pthread_self();
+    RTC_DCHECK(!recursion_);
+  } else {
+    AtomicOps::Increment(&lock_queue_);
+  }
+  ++recursion_;
+#else
   if (pthread_mutex_trylock(&mutex_) != 0)
     return false;
+#endif
 #if CS_DEBUG_CHECKS
   if (!recursion_count_) {
     RTC_DCHECK(!thread_);
     thread_ = pthread_self();
   } else {
     RTC_DCHECK(CurrentThreadIsOwner());
   }
   ++recursion_count_;
 #endif
   return true;
 #endif
 }
 void CriticalSection::Leave() UNLOCK_FUNCTION() {
   RTC_DCHECK(CurrentThreadIsOwner());
 #if defined(WEBRTC_WIN)
   LeaveCriticalSection(&crit_);
 #else
 #if CS_DEBUG_CHECKS
   --recursion_count_;
   RTC_DCHECK(recursion_count_ >= 0);
   if (!recursion_count_)
     thread_ = 0;
 #endif
+#if defined(WEBRTC_MAC)
+  RTC_DCHECK_EQ(owning_thread_, pthread_self());
+  RTC_DCHECK_GE(recursion_, 0);
+  --recursion_;
+  if (!recursion_)
+    owning_thread_ = 0;
+
+  if (AtomicOps::Decrement(&lock_queue_) > 0 && !recursion_)
+    dispatch_semaphore_signal(semaphore_);
+#else
   pthread_mutex_unlock(&mutex_);
 #endif
+#endif
 }
 
 bool CriticalSection::CurrentThreadIsOwner() const {
 #if defined(WEBRTC_WIN)
   // OwningThread has type HANDLE but actually contains the Thread ID:
   // http://stackoverflow.com/questions/12675301/why-is-the-owningthread-member-of-critical-section-of-type-handle-when-it-is-de
   // Converting through size_t avoids the VS 2015 warning C4312: conversion from
   // 'type1' to 'type2' of greater size
   return crit_.OwningThread ==
          reinterpret_cast<HANDLE>(static_cast<size_t>(GetCurrentThreadId()));
@@ skipping 31 matching lines @@
   if (locked_)
     cs_->Leave();
 }
 
 bool TryCritScope::locked() const {
   CS_DEBUG_CODE(lock_was_called_ = true);
   return locked_;
 }
 
 void GlobalLockPod::Lock() {
-#if !defined(WEBRTC_WIN)
+#if !defined(WEBRTC_WIN) && !defined(WEBRTC_MAC)
   const struct timespec ts_null = {0};
 #endif
 
   while (AtomicOps::CompareAndSwap(&lock_acquired, 0, 1)) {
 #if defined(WEBRTC_WIN)
     ::Sleep(0);
+#elif defined(WEBRTC_MAC)
+    sched_yield();
 #else
     nanosleep(&ts_null, nullptr);
 #endif
   }
 }
 
 void GlobalLockPod::Unlock() {
   int old_value = AtomicOps::CompareAndSwap(&lock_acquired, 1, 0);
   RTC_DCHECK_EQ(1, old_value) << "Unlock called without calling Lock first";
 }
 
 GlobalLock::GlobalLock() {
   lock_acquired = 0;
 }
 
 GlobalLockScope::GlobalLockScope(GlobalLockPod* lock)
     : lock_(lock) {
   lock_->Lock();
 }
 
 GlobalLockScope::~GlobalLockScope() {
   lock_->Unlock();
 }
 
 }  // namespace rtc