rename io::PortableSpinLock to SpinLock
authorAdam Simpkins <simpkins@fb.com>
Thu, 11 Dec 2014 20:23:34 +0000 (12:23 -0800)
committerJoelMarcey <joelm@fb.com>
Thu, 18 Dec 2014 20:29:40 +0000 (12:29 -0800)
Summary:
folly::io::PortableSpinLock seems generally useful outside of the io
code.  This moves it into the base folly namespace, and renames it from
PortableSpinLock to just SpinLock.

For most users, the main difference between MicroSpinLock and SpinLock
is that SpinLock provides a constructor that does the right thing, while
MicroSpinLock has to be explicitly initialized.

Test Plan:
Added some new unit tests, and tested both the MicroSpinLock and
pthread_spinlock_t implementations.  I didn't test the Mac OS version,
although that code remains unchanged.

Reviewed By: seanc@fb.com

Subscribers: trunkagent, doug, net-systems@, exa, folly-diffs@

FB internal diff: D1734632

Signature: t1:1734632:1418394033:99f6fbe73b485a1d631a2ef7d1b39ea6f49ecb0b

folly/Makefile.am
folly/SmallLocks.h
folly/SpinLock.h [new file with mode: 0644]
folly/io/PortableSpinLock.h
folly/test/Makefile.am
folly/test/SpinLockTest.cpp [new file with mode: 0644]

index 74bbb43fa6fc9060752740176bf6ba4e32b3e4a1..944fd36fafb08be3396ac12cda53ccd3f7a1416f 100644 (file)
@@ -208,6 +208,7 @@ nobase_follyinclude_HEADERS = \
        small_vector.h \
        SocketAddress.h \
        sorted_vector_types.h \
+       SpinLock.h \
        SpookyHashV1.h \
        SpookyHashV2.h \
        stats/BucketedTimeSeries-defs.h \
index 6d06cff335cd6ff7555ef3df5df7e4d081f92820..9b2e005c0cb45ed823686f39796bdf716a224f18 100644 (file)
@@ -57,7 +57,7 @@ namespace folly {
 namespace detail {
 
   /*
-   * A helper object for the condended case. Starts off with eager
+   * A helper object for the contended case. Starts off with eager
    * spinning, and falls back to sleeping for small quantums.
    */
   class Sleeper {
diff --git a/folly/SpinLock.h b/folly/SpinLock.h
new file mode 100644 (file)
index 0000000..2747a4d
--- /dev/null
@@ -0,0 +1,122 @@
+/*
+ * Copyright 2014 Facebook, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#pragma once
+
+#include <boost/noncopyable.hpp>
+#include <folly/Portability.h>
+
+// This is a wrapper SpinLock implementation that works around the
+// x64 limitation of the base folly MicroSpinLock. If that is available, this
+// simply thinly wraps it. Otherwise, it uses the simplest analog available on
+// iOS (or 32-bit Mac) or, failing that, POSIX (on Android et. al.)
+
+#if __x86_64__
+#include <folly/SmallLocks.h>
+
+namespace folly {
+
+class SpinLock {
+ public:
+  FOLLY_ALWAYS_INLINE SpinLock() {
+    lock_.init();
+  }
+  FOLLY_ALWAYS_INLINE void lock() const {
+    lock_.lock();
+  }
+  FOLLY_ALWAYS_INLINE void unlock() const {
+    lock_.unlock();
+  }
+  FOLLY_ALWAYS_INLINE bool trylock() const {
+    return lock_.try_lock();
+  }
+ private:
+  mutable folly::MicroSpinLock lock_;
+};
+
+}
+
+#elif __APPLE__
+#include <libkern/OSAtomic.h>
+
+namespace folly {
+
+class SpinLock {
+ public:
+  FOLLY_ALWAYS_INLINE SpinLock() : lock_(0) {}
+  FOLLY_ALWAYS_INLINE void lock() const {
+    OSSpinLockLock(&lock_);
+  }
+  FOLLY_ALWAYS_INLINE void unlock() const {
+    OSSpinLockUnlock(&lock_);
+  }
+  FOLLY_ALWAYS_INLINE bool trylock() const {
+    return OSSpinLockTry(&lock_);
+  }
+ private:
+  mutable OSSpinLock lock_;
+};
+
+}
+
+#else
+#include <pthread.h>
+#include <glog/logging.h>
+
+namespace folly {
+
+class SpinLock {
+ public:
+  FOLLY_ALWAYS_INLINE SpinLock() {
+    pthread_mutex_init(&lock_, nullptr);
+  }
+  void lock() const {
+    int rc = pthread_mutex_lock(&lock_);
+    CHECK_EQ(0, rc);
+  }
+  FOLLY_ALWAYS_INLINE void unlock() const {
+    int rc = pthread_mutex_unlock(&lock_);
+    CHECK_EQ(0, rc);
+  }
+  FOLLY_ALWAYS_INLINE bool trylock() const {
+    int rc = pthread_mutex_trylock(&lock_);
+    CHECK_GE(rc, 0);
+    return rc == 0;
+  }
+ private:
+  mutable pthread_mutex_t lock_;
+};
+
+}
+
+#endif
+
+namespace folly {
+
+class SpinLockGuard : private boost::noncopyable {
+ public:
+  FOLLY_ALWAYS_INLINE explicit SpinLockGuard(SpinLock& lock) :
+    lock_(lock) {
+    lock_.lock();
+  }
+  FOLLY_ALWAYS_INLINE ~SpinLockGuard() {
+    lock_.unlock();
+  }
+ private:
+  SpinLock& lock_;
+};
+
+}
index 619c202bd51ca985bfdaa04b841d980c91dfd73a..7b86aa0071aee3394d459a9ee3f1354bdfdd62e4 100644 (file)
 
 #pragma once
 
-#include <boost/noncopyable.hpp>
-
-// This is a wrapper SpinLock implementation that works around the
-// x64 limitation of the base folly MicroSpinLock. If that is available, this
-// simply thinly wraps it. Otherwise, it uses the simplest analog available on
-// iOS (or 32-bit Mac) or, failing that, POSIX (on Android et. al.)
-
-#if __x86_64__
-#include <folly/SmallLocks.h>
-
-namespace folly { namespace io {
-
-class PortableSpinLock {
- public:
-  FOLLY_ALWAYS_INLINE PortableSpinLock() {
-    lock_.init();
-  }
-  FOLLY_ALWAYS_INLINE void lock() const {
-    lock_.lock();
-  }
-  FOLLY_ALWAYS_INLINE void unlock() const {
-    lock_.unlock();
-  }
-  FOLLY_ALWAYS_INLINE bool trylock() const {
-    return lock_.try_lock();
-  }
- private:
-  mutable folly::MicroSpinLock lock_;
-};
-
-}}
-
-#elif __APPLE__
-#include <libkern/OSAtomic.h>
-
-namespace folly { namespace io {
-
-class PortableSpinLock {
- public:
-  FOLLY_ALWAYS_INLINE PortableSpinLock() : lock_(0) {}
-  FOLLY_ALWAYS_INLINE void lock() const {
-    OSSpinLockLock(&lock_);
-  }
-  FOLLY_ALWAYS_INLINE void unlock() const {
-    OSSpinLockUnlock(&lock_);
-  }
-  FOLLY_ALWAYS_INLINE bool trylock() const {
-    return OSSpinLockTry(&lock_);
-  }
- private:
-  mutable OSSpinLock lock_;
-};
-
-}}
-
-#else
-#include <pthread.h>
-
-namespace folly { namespace io {
-
-class PortableSpinLock {
- public:
-  FOLLY_ALWAYS_INLINE PortableSpinLock() {
-    pthread_mutex_init(&lock_, nullptr);
-  }
-  void lock() const {
-    int rc = pthread_mutex_lock(&lock_);
-    CHECK_EQ(0, rc);
-  }
-  FOLLY_ALWAYS_INLINE void unlock() const {
-    int rc = pthread_mutex_unlock(&lock_);
-    CHECK_EQ(0, rc);
-  }
-  FOLLY_ALWAYS_INLINE bool trylock() const {
-    int rc = pthread_mutex_trylock(&lock_);
-    CHECK_GE(rc, 0);
-    return rc == 0;
-  }
- private:
-  mutable pthread_mutex_t lock_;
-};
-
-}}
-
-#endif
+#include <folly/SpinLock.h>
 
 namespace folly { namespace io {
 
-class PortableSpinLockGuard : private boost::noncopyable {
- public:
-  FOLLY_ALWAYS_INLINE explicit PortableSpinLockGuard(PortableSpinLock& lock) :
-    lock_(lock) {
-    lock_.lock();
-  }
-  FOLLY_ALWAYS_INLINE ~PortableSpinLockGuard() {
-    lock_.unlock();
-  }
- private:
-  PortableSpinLock& lock_;
-};
+// These are temporary typedefs to ease the transition from
+// folly::io::PortableSpinLock to folly::SpinLock.
+//
+// I will remove these in a separate diff after updating all call sites
+// that use PortableSpinLock.
+typedef ::folly::SpinLock PortableSpinLock;
+typedef ::folly::SpinLockGuard PortableSpinLockGuard;
 
 }}
index afdcaabf8a0d1af9147e097eab105b6c8fec98de..151e836264793504f8e69669ab6a143f90e6000d 100644 (file)
@@ -26,6 +26,10 @@ libgtest_la_SOURCES = gtest-1.6.0/src/gtest-all.cc
 noinst_HEADERS = FBStringTestBenchmarks.cpp.h \
                 FBVectorTestBenchmarks.cpp.h
 
+small_locks_test_SOURCES = SpinLockTest.cpp
+small_locks_test_LDADD = libgtestmain.la $(top_builddir)/libfolly.la
+TESTS += spin_lock_test
+
 if HAVE_X86_64
 small_locks_test_SOURCES = SmallLocksTest.cpp
 small_locks_test_LDADD = libgtestmain.la $(top_builddir)/libfolly.la
diff --git a/folly/test/SpinLockTest.cpp b/folly/test/SpinLockTest.cpp
new file mode 100644 (file)
index 0000000..323050e
--- /dev/null
@@ -0,0 +1,118 @@
+/*
+ * Copyright 2014 Facebook, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+#include <folly/SpinLock.h>
+
+#include <gtest/gtest.h>
+#include <thread>
+
+using folly::SpinLock;
+using folly::SpinLockGuard;
+
+namespace {
+
+struct LockedVal {
+  int ar[1024];
+  SpinLock lock;
+
+  LockedVal() {
+    memset(ar, 0, sizeof ar);
+  }
+};
+
+LockedVal v;
+void splock_test() {
+  const int max = 1000;
+  unsigned int seed = (uintptr_t)pthread_self();
+  for (int i = 0; i < max; i++) {
+    asm("pause");
+    SpinLockGuard g(v.lock);
+
+    int first = v.ar[0];
+    for (size_t i = 1; i < sizeof v.ar / sizeof i; ++i) {
+      EXPECT_EQ(first, v.ar[i]);
+    }
+
+    int byte = rand_r(&seed);
+    memset(v.ar, char(byte), sizeof v.ar);
+  }
+}
+
+struct TryLockState {
+  SpinLock lock1;
+  SpinLock lock2;
+  bool locked{false};
+  uint64_t obtained{0};
+  uint64_t failed{0};
+};
+TryLockState tryState;
+
+void trylock_test() {
+  const int max = 1000;
+  while (true) {
+    asm("pause");
+    SpinLockGuard g(tryState.lock1);
+    if (tryState.obtained >= max) {
+      break;
+    }
+
+    bool ret = tryState.lock2.trylock();
+    EXPECT_NE(tryState.locked, ret);
+
+    if (ret) {
+      // We got lock2.
+      ++tryState.obtained;
+      tryState.locked = true;
+
+      // Release lock1 and let other threads try to obtain lock2
+      tryState.lock1.unlock();
+      asm("pause");
+      tryState.lock1.lock();
+
+      tryState.locked = false;
+      tryState.lock2.unlock();
+    } else {
+      ++tryState.failed;
+    }
+  }
+}
+
+} // unnamed namespace
+
+TEST(SpinLock, Correctness) {
+  int nthrs = sysconf(_SC_NPROCESSORS_ONLN) * 2;
+  std::vector<std::thread> threads;
+  for (int i = 0; i < nthrs; ++i) {
+    threads.push_back(std::thread(splock_test));
+  }
+  for (auto& t : threads) {
+    t.join();
+  }
+}
+
+TEST(SpinLock, TryLock) {
+  int nthrs = sysconf(_SC_NPROCESSORS_ONLN) * 2;
+  std::vector<std::thread> threads;
+  for (int i = 0; i < nthrs; ++i) {
+    threads.push_back(std::thread(trylock_test));
+  }
+  for (auto& t : threads) {
+    t.join();
+  }
+
+  EXPECT_EQ(1000, tryState.obtained);
+  EXPECT_GT(tryState.failed, 0);
+  LOG(INFO) << "failed count: " << tryState.failed;
+}