From c3ca39e2cdd69292f281c9ef09a8b225f72cfc87 Mon Sep 17 00:00:00 2001 From: Keith Adams Date: Thu, 12 Feb 2015 15:52:39 -0800 Subject: [PATCH] std::atomicize MicroSpinLock. Summary: A colleague at another company started making fun of MicroSpinLock for its x86 assembly and ad hoc compiler memory barriers. Use C++11 (which wasn't really a thing at the time I wrote this). Test Plan: folly's runtests. What else would we like? Reviewed By: andrei.alexandrescu@fb.com Subscribers: folly-diffs@, yfeldblum FB internal diff: D1841563 Signature: t1:1841563:1423780458:a447c081fbd72e3420b23e95dcf26575c9a06798 --- folly/SmallLocks.h | 42 +++++++++++++++-------------------- folly/test/SmallLocksTest.cpp | 4 ++-- 2 files changed, 20 insertions(+), 26 deletions(-) diff --git a/folly/SmallLocks.h b/folly/SmallLocks.h index 9b2e005c..2f5643ed 100644 --- a/folly/SmallLocks.h +++ b/folly/SmallLocks.h @@ -42,6 +42,7 @@ #include #include #include +#include #include #include @@ -102,29 +103,13 @@ namespace detail { */ struct MicroSpinLock { enum { FREE = 0, LOCKED = 1 }; + // lock_ can't be std::atomic<> to preserve POD-ness. uint8_t lock_; - /* - * Atomically move lock_ from "compare" to "newval". Return boolean - * success. Do not play on or around. - */ - bool cas(uint8_t compare, uint8_t newVal) { - bool out; - bool memVal; // only set if the cmpxchg fails - asm volatile("lock; cmpxchgb %[newVal], (%[lockPtr]);" - "setz %[output];" - : [output] "=r" (out), "=a" (memVal) - : "a" (compare), // cmpxchgb constrains this to be in %al - [newVal] "q" (newVal), // Needs to be byte-accessible - [lockPtr] "r" (&lock_) - : "memory", "flags"); - return out; - } - // Initialize this MSL. It is unnecessary to call this if you // zero-initialize the MicroSpinLock. void init() { - lock_ = FREE; + payload()->store(FREE); } bool try_lock() { @@ -134,18 +119,27 @@ struct MicroSpinLock { void lock() { detail::Sleeper sleeper; do { - while (lock_ != FREE) { - asm volatile("" : : : "memory"); + while (payload()->load() != FREE) { sleeper.wait(); } } while (!try_lock()); - DCHECK(lock_ == LOCKED); + DCHECK(payload()->load() == LOCKED); } void unlock() { - CHECK(lock_ == LOCKED); - asm volatile("" : : : "memory"); - lock_ = FREE; // release barrier on x86 + CHECK(payload()->load() == LOCKED); + payload()->store(FREE, std::memory_order_release); + } + + private: + std::atomic* payload() { + return reinterpret_cast*>(&this->lock_); + } + + bool cas(uint8_t compare, uint8_t newVal) { + return std::atomic_compare_exchange_strong_explicit(payload(), &compare, newVal, + std::memory_order_acquire, + std::memory_order_relaxed); } }; diff --git a/folly/test/SmallLocksTest.cpp b/folly/test/SmallLocksTest.cpp index bafb6832..f5863eeb 100644 --- a/folly/test/SmallLocksTest.cpp +++ b/folly/test/SmallLocksTest.cpp @@ -48,10 +48,10 @@ struct LockedVal { // these classes are POD). FOLLY_PACK_PUSH struct ignore1 { MicroSpinLock msl; int16_t foo; } FOLLY_PACK_ATTR; -struct ignore2 { PicoSpinLock psl; int16_t foo; } - FOLLY_PACK_ATTR; +struct ignore2 { PicoSpinLock psl; int16_t foo; } FOLLY_PACK_ATTR; static_assert(sizeof(ignore1) == 3, "Size check failed"); static_assert(sizeof(ignore2) == 6, "Size check failed"); +static_assert(sizeof(MicroSpinLock) == 1, "Size check failed"); FOLLY_PACK_POP LockedVal v; -- 2.34.1