Commit 6808ac5a authored by Pranjal Raihan's avatar Pranjal Raihan Committed by Facebook GitHub Bot

Refactor MicroLock and remove its slots

Summary:
`folly::MicroLock` is actually 4 locks in one. Turns out that it's not very useful considering there is not a single use in fbcode. Instead `MicroLock` should allow a nicer API for exposing the 6 unused bits as user data.

By limiting the lock to one slot (least significant 2 bits), we can make the assumption that the 6 contiguous most significant bits are user data. This means that all the masking logic can go away (see `slotMask` and `LockGuardWithDataSlots`).

Now `lockAndLoad`, `unlockAndStore`, `LockGuardWithData` "just work" with needing to worry about user data interfering with the slots used for locking.

Reviewed By: davidtgoldblatt

Differential Revision: D25909068

fbshipit-source-id: 3f050fa7393b40cff949ed6e30b691d7f40d0edf
parent 3c764a2d
...@@ -24,22 +24,22 @@ namespace folly { ...@@ -24,22 +24,22 @@ namespace folly {
uint8_t MicroLockCore::lockSlowPath( uint8_t MicroLockCore::lockSlowPath(
uint32_t oldWord, uint32_t oldWord,
detail::Futex<>* wordPtr, detail::Futex<>* wordPtr,
uint32_t slotHeldBit, uint32_t heldBit,
unsigned maxSpins, unsigned maxSpins,
unsigned maxYields) { unsigned maxYields) noexcept {
uint32_t newWord; uint32_t newWord;
unsigned spins = 0; unsigned spins = 0;
uint32_t slotWaitBit = slotHeldBit << 1; uint32_t waitBit = heldBit << 1;
uint32_t needWaitBit = 0; uint32_t needWaitBit = 0;
retry: retry:
if ((oldWord & slotHeldBit) != 0) { if ((oldWord & heldBit) != 0) {
++spins; ++spins;
if (spins > maxSpins + maxYields) { if (spins > maxSpins + maxYields) {
// Somebody appears to have the lock. Block waiting for the // Somebody appears to have the lock. Block waiting for the
// holder to unlock the lock. We set heldbit(slot) so that the // holder to unlock the lock. We set heldbit(slot) so that the
// lock holder knows to FUTEX_WAKE us. // lock holder knows to FUTEX_WAKE us.
newWord = oldWord | slotWaitBit; newWord = oldWord | waitBit;
if (newWord != oldWord) { if (newWord != oldWord) {
if (!wordPtr->compare_exchange_weak( if (!wordPtr->compare_exchange_weak(
oldWord, oldWord,
...@@ -49,8 +49,8 @@ retry: ...@@ -49,8 +49,8 @@ retry:
goto retry; goto retry;
} }
} }
detail::futexWait(wordPtr, newWord, slotHeldBit); detail::futexWait(wordPtr, newWord, heldBit);
needWaitBit = slotWaitBit; needWaitBit = waitBit;
} else if (spins > maxSpins) { } else if (spins > maxSpins) {
// sched_yield(), but more portable // sched_yield(), but more portable
std::this_thread::yield(); std::this_thread::yield();
...@@ -61,7 +61,7 @@ retry: ...@@ -61,7 +61,7 @@ retry:
goto retry; goto retry;
} }
newWord = oldWord | slotHeldBit | needWaitBit; newWord = oldWord | heldBit | needWaitBit;
if (!wordPtr->compare_exchange_weak( if (!wordPtr->compare_exchange_weak(
oldWord, oldWord,
newWord, newWord,
...@@ -69,6 +69,6 @@ retry: ...@@ -69,6 +69,6 @@ retry:
std::memory_order_relaxed)) { std::memory_order_relaxed)) {
goto retry; goto retry;
} }
return byteFromWord(newWord); return decodeDataFromWord(newWord);
} }
} // namespace folly } // namespace folly
This diff is collapsed.
...@@ -17,7 +17,6 @@ ...@@ -17,7 +17,6 @@
#pragma once #pragma once
#include <atomic> #include <atomic>
#include <limits>
#include <utility> #include <utility>
#include <folly/Likely.h> #include <folly/Likely.h>
...@@ -40,7 +39,7 @@ class basic_once_flag; ...@@ -40,7 +39,7 @@ class basic_once_flag;
// //
// An alternative flag template that can be used with call_once that uses only // An alternative flag template that can be used with call_once that uses only
// 1 byte. Internally, compact_once_flag uses folly::MicroLock and its data // 1 byte. Internally, compact_once_flag uses folly::MicroLock and its data
// slots. // storage API.
class compact_once_flag; class compact_once_flag;
// call_once // call_once
...@@ -165,12 +164,12 @@ class compact_once_flag { ...@@ -165,12 +164,12 @@ class compact_once_flag {
template <typename F, typename... Args> template <typename F, typename... Args>
FOLLY_NOINLINE void call_once_slow(F&& f, Args&&... args) { FOLLY_NOINLINE void call_once_slow(F&& f, Args&&... args) {
folly::MicroLock::LockGuardWithDataSlots<1> guard(mutex_, 0); folly::MicroLock::LockGuardWithData guard(mutex_);
if (guard.loadedValue() != 0) { if (guard.loadedValue() != 0) {
return; return;
} }
invoke(std::forward<F>(f), std::forward<Args>(args)...); invoke(std::forward<F>(f), std::forward<Args>(args)...);
guard.storeValue(std::numeric_limits<uint8_t>::max()); guard.storeValue(1);
} }
template <typename OnceFlag, typename F, typename... Args> template <typename OnceFlag, typename F, typename... Args>
...@@ -178,24 +177,20 @@ class compact_once_flag { ...@@ -178,24 +177,20 @@ class compact_once_flag {
template <typename F, typename... Args> template <typename F, typename... Args>
FOLLY_NOINLINE bool try_call_once_slow(F&& f, Args&&... args) noexcept { FOLLY_NOINLINE bool try_call_once_slow(F&& f, Args&&... args) noexcept {
folly::MicroLock::LockGuardWithDataSlots<1> guard(mutex_); folly::MicroLock::LockGuardWithData guard(mutex_);
if (guard.loadedValue() != 0) { if (guard.loadedValue() != 0) {
return true; return true;
} }
const auto pass = static_cast<bool>( const auto pass = static_cast<bool>(
invoke(std::forward<F>(f), std::forward<Args>(args)...)); invoke(std::forward<F>(f), std::forward<Args>(args)...));
guard.storeValue(pass ? std::numeric_limits<uint8_t>::max() : 0); guard.storeValue(pass ? 1 : 0);
return pass; return pass;
} }
FOLLY_ALWAYS_INLINE bool test_once() const noexcept { FOLLY_ALWAYS_INLINE bool test_once() const noexcept {
const uint8_t storedValue = mutex_.load(std::memory_order_acquire) & return mutex_.load(std::memory_order_acquire) != 0;
folly::MicroLock::slotMask<1>();
return (storedValue != 0);
} }
// We're only using slot0 of the MicroLock as a mutex, slot1 stores a bool
// indicating whether this flag has been already called.
folly::MicroLock mutex_; folly::MicroLock mutex_;
}; };
......
...@@ -249,14 +249,12 @@ struct SimpleBarrier { ...@@ -249,14 +249,12 @@ struct SimpleBarrier {
} // namespace } // namespace
TEST(SmallLocks, MicroLock) { TEST(SmallLocks, MicroLock) {
volatile uint64_t counters[4] = {0, 0, 0, 0}; volatile uint64_t counter = 0;
std::vector<std::thread> threads; std::vector<std::thread> threads;
static const unsigned nrThreads = 20; static const unsigned nrThreads = 20;
static const unsigned iterPerThread = 10000; static const unsigned iterPerThread = 10000;
SimpleBarrier startBarrier; SimpleBarrier startBarrier;
assert(iterPerThread % 4 == 0);
// Embed the lock in a larger structure to ensure that we do not // Embed the lock in a larger structure to ensure that we do not
// affect bits outside the ones MicroLock is defined to affect. // affect bits outside the ones MicroLock is defined to affect.
struct { struct {
...@@ -292,16 +290,15 @@ TEST(SmallLocks, MicroLock) { ...@@ -292,16 +290,15 @@ TEST(SmallLocks, MicroLock) {
threads.emplace_back([&] { threads.emplace_back([&] {
startBarrier.wait(); startBarrier.wait();
for (unsigned iter = 0; iter < iterPerThread; ++iter) { for (unsigned iter = 0; iter < iterPerThread; ++iter) {
unsigned slotNo = iter % 4; x.alock.lock();
x.alock.lock(slotNo); counter += 1;
counters[slotNo] += 1;
// The occasional sleep makes it more likely that we'll // The occasional sleep makes it more likely that we'll
// exercise the futex-wait path inside MicroLock. // exercise the futex-wait path inside MicroLock.
if (iter % 1000 == 0) { if (iter % 1000 == 0) {
struct timespec ts = {0, 10000}; struct timespec ts = {0, 10000};
(void)nanosleep(&ts, nullptr); (void)nanosleep(&ts, nullptr);
} }
x.alock.unlock(slotNo); x.alock.unlock();
} }
}); });
} }
...@@ -317,9 +314,7 @@ TEST(SmallLocks, MicroLock) { ...@@ -317,9 +314,7 @@ TEST(SmallLocks, MicroLock) {
EXPECT_EQ(x.a, 'a'); EXPECT_EQ(x.a, 'a');
EXPECT_EQ(x.b, (uint8_t)(origB + iterPerThread / 2)); EXPECT_EQ(x.b, (uint8_t)(origB + iterPerThread / 2));
EXPECT_EQ(x.d, (uint8_t)(origD + iterPerThread / 2)); EXPECT_EQ(x.d, (uint8_t)(origD + iterPerThread / 2));
for (unsigned i = 0; i < 4; ++i) { EXPECT_EQ(counter, ((uint64_t)nrThreads * iterPerThread));
EXPECT_EQ(counters[i], ((uint64_t)nrThreads * iterPerThread) / 4);
}
} }
TEST(SmallLocks, MicroLockTryLock) { TEST(SmallLocks, MicroLockTryLock) {
...@@ -330,50 +325,37 @@ TEST(SmallLocks, MicroLockTryLock) { ...@@ -330,50 +325,37 @@ TEST(SmallLocks, MicroLockTryLock) {
lock.unlock(); lock.unlock();
} }
TEST(SmallLocks, MicroLockSlotsAsData) { TEST(SmallLocks, MicroLockWithData) {
MicroLock lock; MicroLock lock;
lock.init(); lock.init();
EXPECT_EQ(lock.load(std::memory_order_relaxed), 0); EXPECT_EQ(lock.load(std::memory_order_relaxed), 0);
lock.lock(0); EXPECT_EQ(lock.lockAndLoad(), 0);
EXPECT_EQ(lock.load(std::memory_order_relaxed), 0b00000001); lock.unlockAndStore(42);
EXPECT_EQ(lock.load(std::memory_order_relaxed), 42);
EXPECT_EQ(lock.lockAndLoad(1), 0b00000101);
// Mask out only the slot that is being unlocked EXPECT_EQ(lock.lockAndLoad(), 42);
lock.unlockAndStore( lock.unlock();
1, ~MicroLock::slotMask<0>(), std::numeric_limits<uint8_t>::max()); EXPECT_EQ(lock.load(std::memory_order_relaxed), 42);
EXPECT_EQ(lock.load(std::memory_order_relaxed), 0b11110001);
lock.init();
lock.lock(0);
{
MicroLock::LockGuardWithDataSlots<2, 3> guard(lock, 1);
EXPECT_EQ(lock.load(std::memory_order_relaxed), 0b00000101);
EXPECT_EQ(guard.loadedValue(), 0);
guard.storeValue(std::numeric_limits<uint8_t>::max());
EXPECT_EQ(lock.load(std::memory_order_relaxed), 0b00000101);
}
// Only slots 2 and 3 should be set
EXPECT_EQ(lock.load(std::memory_order_relaxed), 0b11110001);
{
MicroLock::LockGuardWithDataSlots<2> guard(lock, 1);
EXPECT_EQ(lock.load(std::memory_order_relaxed), 0b11110101);
EXPECT_EQ(guard.loadedValue(), 0b00110000);
guard.storeValue(0);
EXPECT_EQ(lock.load(std::memory_order_relaxed), 0b11110101);
}
// Only slot 2 should be unset
EXPECT_EQ(lock.load(std::memory_order_relaxed), 0b11000001);
lock.store(12, std::memory_order_relaxed);
{ {
MicroLock::LockGuardWithDataSlots<3> guard(lock, 1); MicroLock::LockGuardWithData guard{lock};
EXPECT_EQ(lock.load(std::memory_order_relaxed), 0b11000101); EXPECT_EQ(guard.loadedValue(), 12);
EXPECT_EQ(guard.loadedValue(), 0b11000000); EXPECT_EQ(lock.load(std::memory_order_relaxed), 12);
guard.storeValue(24);
// Should not have immediate effect
EXPECT_EQ(guard.loadedValue(), 12);
EXPECT_EQ(lock.load(std::memory_order_relaxed), 12);
} }
EXPECT_EQ(lock.load(std::memory_order_relaxed), 0b11000001); EXPECT_EQ(lock.load(std::memory_order_relaxed), 24);
// Do not modify unless something is stored
// Drop the two most significant bits
lock.lock();
lock.unlockAndStore(0b10011001);
EXPECT_EQ(lock.load(std::memory_order_relaxed), 0b00011001);
lock.store(0b11101110, std::memory_order_relaxed);
EXPECT_EQ(lock.load(std::memory_order_relaxed), 0b00101110);
} }
namespace { namespace {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment