diff --git a/api/include/opentelemetry/common/spin_lock_mutex.h b/api/include/opentelemetry/common/spin_lock_mutex.h index 7031fa4d23..7d33fbedfd 100644 --- a/api/include/opentelemetry/common/spin_lock_mutex.h +++ b/api/include/opentelemetry/common/spin_lock_mutex.h @@ -80,12 +80,7 @@ class SpinLockMutex /** * Attempts to lock the mutex. Return immediately with `true` (success) or `false` (failure). */ - bool try_lock() noexcept - { - return !flag_.load(std::memory_order_relaxed) && - !flag_.exchange(true, std::memory_order_acquire); - } - + bool try_lock() noexcept { return !flag_.exchange(true, std::memory_order_acquire); } /** * Blocks until a lock can be obtained for the current thread. * @@ -95,13 +90,9 @@ class SpinLockMutex */ void lock() noexcept { - for (;;) + // Try once + while (!try_lock()) { - // Try once - if (!flag_.exchange(true, std::memory_order_acquire)) - { - return; - } // Spin-Fast (goal ~10ns) for (std::size_t i = 0; i < SPINLOCK_FAST_ITERATIONS; ++i) { diff --git a/api/test/common/spinlock_benchmark.cc b/api/test/common/spinlock_benchmark.cc index 6cd1c9558d..e0177db42b 100644 --- a/api/test/common/spinlock_benchmark.cc +++ b/api/test/common/spinlock_benchmark.cc @@ -4,6 +4,7 @@ #include #include #include +#include #include #include @@ -27,8 +28,8 @@ constexpr int TightLoopLocks = 10000; // // lock: A lambda denoting how to lock. Accepts a reference to `SpinLockType`. // unlock: A lambda denoting how to unlock. Accepts a reference to `SpinLockType`. -template -inline void SpinThrash(benchmark::State &s, SpinLockType &spinlock, LockF lock, UnlockF unlock) +template +void SpinThrash(benchmark::State &s, LockF lock, UnlockF unlock) { auto num_threads = s.range(0); // Value we will increment, fighting over a spinlock. @@ -49,9 +50,9 @@ inline void SpinThrash(benchmark::State &s, SpinLockType &spinlock, LockF lock, // to ensure maximum thread contention. for (int i = 0; i < TightLoopLocks; i++) { - lock(spinlock); + lock(); value++; - unlock(spinlock); + unlock(); } }); } @@ -63,35 +64,35 @@ inline void SpinThrash(benchmark::State &s, SpinLockType &spinlock, LockF lock, } // Benchmark of full spin-lock implementation. -static void BM_SpinLockThrashing(benchmark::State &s) +void BM_SpinLockThrashing(benchmark::State &s) { SpinLockMutex spinlock; - SpinThrash(s, spinlock, [](SpinLockMutex &m) { m.lock(); }, [](SpinLockMutex &m) { m.unlock(); }); + SpinThrash(s, [&] { spinlock.lock(); }, [&] { spinlock.unlock(); }); } // Naive `while(try_lock()) {}` implementation of lock. -static void BM_NaiveSpinLockThrashing(benchmark::State &s) +void BM_NaiveSpinLockThrashing(benchmark::State &s) { SpinLockMutex spinlock; SpinThrash( - s, spinlock, - [](SpinLockMutex &m) { - while (!m.try_lock()) + s, + [&] { + while (!spinlock.try_lock()) { // Left this comment to keep the same format on old and new versions of clang-format } }, - [](SpinLockMutex &m) { m.unlock(); }); + [&] { spinlock.unlock(); }); } // Simple `while(try_lock()) { yield-processor }` -static void BM_ProcYieldSpinLockThrashing(benchmark::State &s) +void BM_ProcYieldSpinLockThrashing(benchmark::State &s) { SpinLockMutex spinlock; - SpinThrash( - s, spinlock, - [](SpinLockMutex &m) { - while (!m.try_lock()) + SpinThrash( + s, + [&] { + while (!spinlock.try_lock()) { #if defined(_MSC_VER) YieldProcessor(); @@ -108,33 +109,33 @@ static void BM_ProcYieldSpinLockThrashing(benchmark::State &s) #endif } }, - [](SpinLockMutex &m) { m.unlock(); }); + [&] { spinlock.unlock(); }); } // SpinLock thrashing with thread::yield(). -static void BM_ThreadYieldSpinLockThrashing(benchmark::State &s) +void BM_ThreadYieldSpinLockThrashing(benchmark::State &s) { #if defined(__cpp_lib_atomic_value_initialization) && \ __cpp_lib_atomic_value_initialization >= 201911L std::atomic_flag mutex{}; #else - std::atomic_flag mutex = ATOMIC_FLAG_INIT; + alignas(8) std::atomic_flag mutex = ATOMIC_FLAG_INIT; #endif - SpinThrash( - s, mutex, - [](std::atomic_flag &l) { - uint32_t try_count = 0; - while (l.test_and_set(std::memory_order_acq_rel)) + SpinThrash( + s, + [&]() { + while (mutex.test_and_set(std::memory_order_acq_rel)) { - ++try_count; - if (try_count % 32) - { - std::this_thread::yield(); - } + std::this_thread::yield(); } - std::this_thread::yield(); }, - [](std::atomic_flag &l) { l.clear(std::memory_order_release); }); + [&] { mutex.clear(std::memory_order_release); }); +} + +void BM_StdMutexCheck(benchmark::State &s) +{ + std::mutex mtx; + SpinThrash(s, [&] { mtx.lock(); }, [&] { mtx.unlock(); }); } // Run the benchmarks at 2x thread/core and measure the amount of time to thrash around. @@ -162,6 +163,12 @@ BENCHMARK(BM_ThreadYieldSpinLockThrashing) ->MeasureProcessCPUTime() ->UseRealTime() ->Unit(benchmark::kMillisecond); +BENCHMARK(BM_StdMutexCheck) + ->RangeMultiplier(2) + ->Range(1, std::thread::hardware_concurrency()) + ->MeasureProcessCPUTime() + ->UseRealTime() + ->Unit(benchmark::kMillisecond); } // namespace