From a7971a0e0f0874a41aa8775b62493d285ff7e821 Mon Sep 17 00:00:00 2001 From: Mark Rowe Date: Mon, 27 Jul 2026 20:46:09 -0700 Subject: [PATCH 1/2] Add BN_NOINLINE for keeping cold paths out of hot functions --- base/compiler.h | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/base/compiler.h b/base/compiler.h index dce97ca109..87348e2f19 100644 --- a/base/compiler.h +++ b/base/compiler.h @@ -96,3 +96,13 @@ #else #define BN_EMPTY_BASES #endif + +// BN_NOINLINE +// +// Suppress inlining of a function at its call sites. + +#if defined(_MSC_VER) +#define BN_NOINLINE __declspec(noinline) +#else +#define BN_NOINLINE __attribute__((noinline)) +#endif From e9ef61a04f3f9e974b1d96780b4499ec173beeb6 Mon Sep 17 00:00:00 2001 From: Mark Rowe Date: Mon, 2 Mar 2026 18:21:54 -0800 Subject: [PATCH 2/2] Implement bn::base::{mutex, recursive_mutex} for Windows and Linux On Windows, `mutex` wraps a `std::shared_mutex`, which unlike `std::mutex` is a thin wrapper around `SRWLOCK`, so it is both fast and compact. On Linux, it is modeled after `mutex3` from Drepper's "Futexes are Tricky". A `std::atomic` holds the state, and blocking is handled via `platform_wait`, a thin abstraction over futex wait/wake that also has `__ulock` and `WaitOnAddress` backends for macOS and Windows. `recursive_mutex` is a wrapper around `mutex` that tracks the owning thread and recursion count. These are comparable in performance to the equivalent standard library types, but they're significantly smaller (on Windows 8 bytes vs 80, on Linux 4 bytes vs 40). --- base/mutex.h | 267 ++++++++++++++++++++++++++++++++++++++++--- base/platform_wait.h | 147 ++++++++++++++++++++++++ base/tsan.h | 59 ++++++++++ 3 files changed, 459 insertions(+), 14 deletions(-) create mode 100644 base/platform_wait.h diff --git a/base/mutex.h b/base/mutex.h index dea34d4ca9..d402239c3d 100644 --- a/base/mutex.h +++ b/base/mutex.h @@ -23,26 +23,22 @@ // Mutex types that satisfy the C++ Lockable requirements. // // On macOS, mutex wraps os_unfair_lock and recursive_mutex wraps -// os_unfair_recursive_lock. These are faster and significantly smaller than -// std::mutex / std::recursive_mutex, which wrap the equivalent pthread mutexes. +// os_unfair_recursive_lock. With the MSVC STL, mutex wraps std::shared_mutex, +// which is a thin wrapper around SRWLOCK. Elsewhere, mutex uses an atomic-based +// implementation that blocks via platform_wait. // -// On other platforms, these are type aliases for the std equivalents. +// Outside macOS, recursive_mutex is built on top of mutex. +// +// These are faster and significantly smaller than std::mutex / +// std::recursive_mutex. // // Note that `std::mutex` must still be used with `std::condition_variable` as // `std::condition_variable` only works with `std::mutex`. -#ifndef __APPLE__ - -#include - -namespace bn::base { - -using mutex = std::mutex; -using recursive_mutex = std::recursive_mutex; - -} // namespace bn::base +#include "base/assertions.h" // IWYU pragma: keep +#include "base/compiler.h" // IWYU pragma: keep -#else +#ifdef __APPLE__ #include @@ -131,4 +127,247 @@ class recursive_mutex } // namespace bn::base +#else // Windows or Linux + +#include "base/tsan.h" + +#include + +#ifdef _MSC_VER +#include +#else +#include "base/platform_wait.h" +#endif + +#ifdef _WIN32 +#include +#else +#include +#endif + +namespace bn::base { + +namespace detail { + +#ifdef _WIN32 + +using ThreadId = std::thread::id; +inline ThreadId CurrentThreadID() noexcept +{ + static thread_local ThreadId tid = std::this_thread::get_id(); + return tid; +} + +#else + +// libstdc++'s std::thread::id is 8 bytes. Using it would force 8-byte alignment on recursive_mutex. +// pid_t is only 4 bytes, which decreases both the size and required alignment of recursive_mutex. +using ThreadId = pid_t; +inline ThreadId CurrentThreadID() noexcept +{ + static thread_local ThreadId tid = gettid(); + return tid; +} + +#endif + +} // namespace detail + +#ifdef _MSC_VER + +class mutex +{ + std::shared_mutex m_lock; + +#if BN_ASSERTIONS_ENABLED + std::atomic m_dbgOwner{detail::ThreadId{}}; +#endif + +public: + mutex() = default; + + mutex(const mutex&) = delete; + mutex& operator=(const mutex&) = delete; + mutex(mutex&&) = delete; + mutex& operator=(mutex&&) = delete; + + void lock() noexcept + { + BN_ASSERT(m_dbgOwner.load(std::memory_order_relaxed) != detail::CurrentThreadID() + && "deadlock: locking a mutex already held by this thread"); + m_lock.lock(); + BN_ASSERT(m_dbgOwner.exchange(detail::CurrentThreadID(), std::memory_order_relaxed) == detail::ThreadId{}); + } + + bool try_lock() noexcept + { + if (!m_lock.try_lock()) + return false; + BN_ASSERT(m_dbgOwner.exchange(detail::CurrentThreadID(), std::memory_order_relaxed) == detail::ThreadId{}); + return true; + } + + void unlock() noexcept + { + BN_ASSERT(m_dbgOwner.load(std::memory_order_relaxed) == detail::CurrentThreadID() + && "unlock called by non-owning thread"); + BN_ASSERT(m_dbgOwner.exchange(detail::ThreadId{}, std::memory_order_relaxed) != detail::ThreadId{}); + m_lock.unlock(); + } +}; + +#else + +class mutex +{ + enum class State : int + { + Unlocked, + Locked, + Contended, + }; + + std::atomic m_state{State::Unlocked}; + +#if BN_ASSERTIONS_ENABLED + std::atomic m_dbgOwner{detail::ThreadId{}}; +#endif + +public: + mutex() = default; + + mutex(const mutex&) = delete; + mutex& operator=(const mutex&) = delete; + mutex(mutex&&) = delete; + mutex& operator=(mutex&&) = delete; + + void lock() noexcept + { + BN_ASSERT(m_dbgOwner.load(std::memory_order_relaxed) != detail::CurrentThreadID() + && "deadlock: locking a mutex already held by this thread"); + + BN_TSAN_MUTEX_PRE_LOCK(this); + auto expected = State::Unlocked; + if (!m_state.compare_exchange_strong(expected, State::Locked, + std::memory_order_acquire, std::memory_order_relaxed)) [[unlikely]] + lock_slow(expected); + BN_TSAN_MUTEX_POST_LOCK(this); + + BN_ASSERT(m_dbgOwner.exchange(detail::CurrentThreadID(), std::memory_order_relaxed) == detail::ThreadId{}); + } + + bool try_lock() noexcept + { + BN_TSAN_MUTEX_PRE_TRY_LOCK(this); + auto expected = State::Unlocked; + if (!m_state.compare_exchange_strong(expected, State::Locked, + std::memory_order_acquire, std::memory_order_relaxed)) [[unlikely]] + { + BN_TSAN_MUTEX_POST_TRY_LOCK_FAILED(this); + return false; + } + BN_TSAN_MUTEX_POST_TRY_LOCK(this); + BN_ASSERT(m_dbgOwner.exchange(detail::CurrentThreadID(), std::memory_order_relaxed) == detail::ThreadId{}); + return true; + } + + void unlock() noexcept + { + BN_ASSERT(m_dbgOwner.load(std::memory_order_relaxed) == detail::CurrentThreadID() + && "unlock called by non-owning thread"); + BN_ASSERT(m_dbgOwner.exchange(detail::ThreadId{}, std::memory_order_relaxed) != detail::ThreadId{}); + + BN_TSAN_MUTEX_PRE_UNLOCK(this); + if (m_state.exchange(State::Unlocked, std::memory_order_release) == State::Contended) [[unlikely]] + detail::platform_notify_one(&m_state); + BN_TSAN_MUTEX_POST_UNLOCK(this); + } + +private: + BN_NOINLINE + void lock_slow(State expected) noexcept + { + while (true) + { + // If the lock is held (not yet marked contended), exchange to mark it + // contended and try to acquire in one step. + if (expected != State::Contended) + expected = m_state.exchange(State::Contended, std::memory_order_acquire); + + if (expected == State::Unlocked) + return; + + detail::platform_wait(&m_state, State::Contended); + expected = State::Locked; + } + } + +}; + +#endif // _MSC_VER + +class recursive_mutex +{ + mutex m_mutex; + std::atomic m_owner{detail::ThreadId{}}; + int m_count{0}; + +public: + recursive_mutex() = default; + + recursive_mutex(const recursive_mutex&) = delete; + recursive_mutex& operator=(const recursive_mutex&) = delete; + recursive_mutex(recursive_mutex&&) = delete; + recursive_mutex& operator=(recursive_mutex&&) = delete; + + void lock() noexcept + { + auto me = detail::CurrentThreadID(); + if (m_owner.load(std::memory_order_relaxed) == me) + { + BN_TSAN_MUTEX_PRE_RECURSIVE_LOCK(this); + ++m_count; + BN_TSAN_MUTEX_POST_RECURSIVE_LOCK(this); + return; + } + m_mutex.lock(); + m_owner.store(me, std::memory_order_relaxed); + m_count = 1; + } + + bool try_lock() noexcept + { + auto me = detail::CurrentThreadID(); + if (m_owner.load(std::memory_order_relaxed) == me) + { + BN_TSAN_MUTEX_PRE_RECURSIVE_LOCK(this); + ++m_count; + BN_TSAN_MUTEX_POST_RECURSIVE_LOCK(this); + return true; + } + if (!m_mutex.try_lock()) + return false; + m_owner.store(me, std::memory_order_relaxed); + m_count = 1; + return true; + } + + void unlock() noexcept + { + BN_ASSERT(m_owner.load(std::memory_order_relaxed) == detail::CurrentThreadID() + && "unlock called by non-owning thread"); + BN_ASSERT(m_count > 0 && "unlock called on unheld recursive_mutex"); + if (--m_count > 0) + { + BN_TSAN_MUTEX_PRE_RECURSIVE_UNLOCK(this); + BN_TSAN_MUTEX_POST_RECURSIVE_UNLOCK(this); + return; + } + m_owner.store(detail::ThreadId{}, std::memory_order_relaxed); + m_mutex.unlock(); + } +}; + +} // namespace bn::base + #endif diff --git a/base/platform_wait.h b/base/platform_wait.h new file mode 100644 index 0000000000..ad23340afb --- /dev/null +++ b/base/platform_wait.h @@ -0,0 +1,147 @@ +// Copyright (c) 2026 Vector 35 Inc +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to +// deal in the Software without restriction, including without limitation the +// rights to use, copy, modify, merge, publish, distribute, sublicense, and/or +// sell copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING +// FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS +// IN THE SOFTWARE. + +#pragma once + +// Platform-native address-based wait/wake primitives. +// +// On Linux, wraps futex(). On macOS, wraps __ulock_wait/__ulock_wake. +// On Windows, wraps WaitOnAddress/WakeByAddress*. +// +// These bypass the C++ standard library's std::atomic::wait/notify, which +// adds spin/backoff overhead and (on some platforms) routes through a shared +// contention table instead of waiting directly on the user's address. + +#include +#include + +#ifdef __linux__ + +#include +#include +#include +#include + +namespace bn::base::detail { + +template +inline void platform_wait(std::atomic* addr, T expected) noexcept +{ + static_assert(sizeof(T) == 4, "Linux futex requires 4-byte values"); + syscall(SYS_futex, addr, FUTEX_WAIT_PRIVATE, static_cast(expected), + nullptr, nullptr, 0); +} + +template +inline void platform_notify_one(std::atomic* addr) noexcept +{ + static_assert(sizeof(T) == 4); + syscall(SYS_futex, addr, FUTEX_WAKE_PRIVATE, 1, nullptr, nullptr, 0); +} + +template +inline void platform_notify_all(std::atomic* addr) noexcept +{ + static_assert(sizeof(T) == 4); + syscall(SYS_futex, addr, FUTEX_WAKE_PRIVATE, INT_MAX, nullptr, nullptr, 0); +} + +} // namespace bn::base::detail + +#elif defined(__APPLE__) + +namespace bn::base::detail { + +// From xnu/bsd/sys/ulock.h (private API, stable ABI). +constexpr uint32_t kULockCompareAndWait = 1; +constexpr uint32_t kULockCompareAndWait64 = 5; +constexpr uint32_t kULockWakeAll = 0x00000100; + +extern "C" int __ulock_wait(uint32_t operation, void* addr, uint64_t value, + uint32_t timeout); +extern "C" int __ulock_wake(uint32_t operation, void* addr, + uint64_t wake_value); + +template +inline void platform_wait(std::atomic* addr, T expected) noexcept +{ + static_assert(sizeof(T) == 4 || sizeof(T) == 8, + "macOS __ulock requires 4- or 8-byte values"); + if constexpr (sizeof(T) == 4) + __ulock_wait(kULockCompareAndWait, addr, + static_cast(static_cast(expected)), 0); + else + __ulock_wait(kULockCompareAndWait64, addr, + static_cast(expected), 0); +} + +template +inline void platform_notify_one(std::atomic* addr) noexcept +{ + static_assert(sizeof(T) == 4 || sizeof(T) == 8); + if constexpr (sizeof(T) == 4) + __ulock_wake(kULockCompareAndWait, addr, 0); + else + __ulock_wake(kULockCompareAndWait64, addr, 0); +} + +template +inline void platform_notify_all(std::atomic* addr) noexcept +{ + static_assert(sizeof(T) == 4 || sizeof(T) == 8); + if constexpr (sizeof(T) == 4) + __ulock_wake(kULockCompareAndWait | kULockWakeAll, addr, 0); + else + __ulock_wake(kULockCompareAndWait64 | kULockWakeAll, addr, 0); +} + +} // namespace bn::base::detail + +#elif defined(_WIN32) + +#ifndef WIN32_LEAN_AND_MEAN +#define WIN32_LEAN_AND_MEAN +#endif +#include + +namespace bn::base::detail { + +template +inline void platform_wait(std::atomic* addr, T expected) noexcept +{ + static_assert(sizeof(T) <= 8, "WaitOnAddress supports values up to 8 bytes"); + WaitOnAddress(addr, &expected, sizeof(T), INFINITE); +} + +template +inline void platform_notify_one(std::atomic* addr) noexcept +{ + WakeByAddressSingle(addr); +} + +template +inline void platform_notify_all(std::atomic* addr) noexcept +{ + WakeByAddressAll(addr); +} + +} // namespace bn::base::detail + +#endif diff --git a/base/tsan.h b/base/tsan.h index ebe3aa5d0e..edc916cc21 100644 --- a/base/tsan.h +++ b/base/tsan.h @@ -58,3 +58,62 @@ extern "C" void __tsan_release(void* addr); #define BN_TSAN_ACQUIRE(addr) ((void)0) #define BN_TSAN_RELEASE(addr) ((void)0) #endif + +// BN_TSAN_MUTEX_PRE_*(addr) / BN_TSAN_MUTEX_POST_*(addr) +// +// Describes a lock built from atomics to the thread sanitizer so that it models the +// happens-before edges the lock establishes and reports misuse such as unlocking a lock +// that is not held. Each operation is bracketed by a PRE call before the atomics run and +// a POST call after they complete, with the failed try-lock variant reported instead of +// POST_TRY_LOCK when the attempt does not acquire. The exclusive, recursive and shared +// families correspond to the lock modes the sanitizer distinguishes. All expand to +// nothing when the thread sanitizer is not in use. + +#if BN_HAS_THREAD_SANITIZER +#include + +#define BN_TSAN_MUTEX_PRE_LOCK(addr) __tsan_mutex_pre_lock(addr, 0) +#define BN_TSAN_MUTEX_POST_LOCK(addr) __tsan_mutex_post_lock(addr, 0, 0) +#define BN_TSAN_MUTEX_PRE_TRY_LOCK(addr) __tsan_mutex_pre_lock(addr, __tsan_mutex_try_lock) +#define BN_TSAN_MUTEX_POST_TRY_LOCK(addr) __tsan_mutex_post_lock(addr, __tsan_mutex_try_lock, 0) +#define BN_TSAN_MUTEX_POST_TRY_LOCK_FAILED(addr) __tsan_mutex_post_lock(addr, __tsan_mutex_try_lock | __tsan_mutex_try_lock_failed, 0) +#define BN_TSAN_MUTEX_PRE_UNLOCK(addr) __tsan_mutex_pre_unlock(addr, 0) +#define BN_TSAN_MUTEX_POST_UNLOCK(addr) __tsan_mutex_post_unlock(addr, 0) + +#define BN_TSAN_MUTEX_PRE_RECURSIVE_LOCK(addr) __tsan_mutex_pre_lock(addr, __tsan_mutex_recursive_lock) +#define BN_TSAN_MUTEX_POST_RECURSIVE_LOCK(addr) __tsan_mutex_post_lock(addr, __tsan_mutex_recursive_lock, 0) +#define BN_TSAN_MUTEX_PRE_RECURSIVE_UNLOCK(addr) __tsan_mutex_pre_unlock(addr, __tsan_mutex_recursive_unlock) +#define BN_TSAN_MUTEX_POST_RECURSIVE_UNLOCK(addr) __tsan_mutex_post_unlock(addr, __tsan_mutex_recursive_unlock) + +#define BN_TSAN_MUTEX_PRE_LOCK_SHARED(addr) __tsan_mutex_pre_lock(addr, __tsan_mutex_read_lock) +#define BN_TSAN_MUTEX_POST_LOCK_SHARED(addr) __tsan_mutex_post_lock(addr, __tsan_mutex_read_lock, 0) +#define BN_TSAN_MUTEX_PRE_TRY_LOCK_SHARED(addr) __tsan_mutex_pre_lock(addr, __tsan_mutex_read_lock | __tsan_mutex_try_lock) +#define BN_TSAN_MUTEX_POST_TRY_LOCK_SHARED(addr) __tsan_mutex_post_lock(addr, __tsan_mutex_read_lock | __tsan_mutex_try_lock, 0) +#define BN_TSAN_MUTEX_POST_TRY_LOCK_SHARED_FAILED(addr) __tsan_mutex_post_lock(addr, __tsan_mutex_read_lock | __tsan_mutex_try_lock | __tsan_mutex_try_lock_failed, 0) +#define BN_TSAN_MUTEX_PRE_UNLOCK_SHARED(addr) __tsan_mutex_pre_unlock(addr, __tsan_mutex_read_lock) +#define BN_TSAN_MUTEX_POST_UNLOCK_SHARED(addr) __tsan_mutex_post_unlock(addr, __tsan_mutex_read_lock) + +#else + +#define BN_TSAN_MUTEX_PRE_LOCK(addr) +#define BN_TSAN_MUTEX_POST_LOCK(addr) +#define BN_TSAN_MUTEX_PRE_TRY_LOCK(addr) +#define BN_TSAN_MUTEX_POST_TRY_LOCK(addr) +#define BN_TSAN_MUTEX_POST_TRY_LOCK_FAILED(addr) +#define BN_TSAN_MUTEX_PRE_UNLOCK(addr) +#define BN_TSAN_MUTEX_POST_UNLOCK(addr) + +#define BN_TSAN_MUTEX_PRE_RECURSIVE_LOCK(addr) +#define BN_TSAN_MUTEX_POST_RECURSIVE_LOCK(addr) +#define BN_TSAN_MUTEX_PRE_RECURSIVE_UNLOCK(addr) +#define BN_TSAN_MUTEX_POST_RECURSIVE_UNLOCK(addr) + +#define BN_TSAN_MUTEX_PRE_LOCK_SHARED(addr) +#define BN_TSAN_MUTEX_POST_LOCK_SHARED(addr) +#define BN_TSAN_MUTEX_PRE_TRY_LOCK_SHARED(addr) +#define BN_TSAN_MUTEX_POST_TRY_LOCK_SHARED(addr) +#define BN_TSAN_MUTEX_POST_TRY_LOCK_SHARED_FAILED(addr) +#define BN_TSAN_MUTEX_PRE_UNLOCK_SHARED(addr) +#define BN_TSAN_MUTEX_POST_UNLOCK_SHARED(addr) + +#endif