[Lldb-commits] [lldb] [lldb] Use os_unfair_lock for the string pool on Darwin (PR #197851)

Jonas Devlieghere via lldb-commits lldb-commits at lists.llvm.org
Thu May 14 17:50:24 PDT 2026


https://github.com/JDevlieghere created https://github.com/llvm/llvm-project/pull/197851

The  ConstString pool guards each shard with a SmartRWMutex<false>, which on Darwin maps to pthread_rwlock and incurs heavy syscall overhead under contention. os_unfair_lock is a userspace fast path that's significantly faster for the write path and roughly on par for reads, but it's exclusive-only and not available off Apple platforms.

On a 32-thread machine, hyperfine reports the benchmark unit test binary running 4.07x faster (365.6 ms +/- 3.9 ms -> 89.8 ms +/- 5.1 ms), with system time dropping from 4.35 s to 0.39 s as the kernel pthread_rwlock path is replaced by the userspace fast path.

>From 27ca03349fd337e841c37f83a8c74226962129c4 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere <jonas at devlieghere.com>
Date: Thu, 14 May 2026 17:44:25 -0700
Subject: [PATCH] [lldb] Use os_unfair_lock for the string pool on Darwin

The  ConstString pool guards each shard with a SmartRWMutex<false>,
which on Darwin maps to pthread_rwlock and incurs heavy syscall overhead
under contention. os_unfair_lock is a userspace fast path that's
significantly faster for the write path and roughly on par for reads,
but it's exclusive-only and not available off Apple platforms.

On a 32-thread machine, hyperfine reports the benchmark unit test binary
running 4.07x faster (365.6 ms +/- 3.9 ms -> 89.8 ms +/- 5.1 ms), with
system time dropping from 4.35 s to 0.39 s as the kernel pthread_rwlock
path is replaced by the userspace fast path.
---
 lldb/source/Utility/ConstString.cpp        | 69 +++++++++++++++++++---
 lldb/unittests/Utility/ConstStringTest.cpp | 63 ++++++++++++++++++++
 2 files changed, 124 insertions(+), 8 deletions(-)

diff --git a/lldb/source/Utility/ConstString.cpp b/lldb/source/Utility/ConstString.cpp
index ea897dc611cc9..7424bd7ea9b78 100644
--- a/lldb/source/Utility/ConstString.cpp
+++ b/lldb/source/Utility/ConstString.cpp
@@ -15,18 +15,71 @@
 #include "llvm/Support/Allocator.h"
 #include "llvm/Support/DJB.h"
 #include "llvm/Support/FormatProviders.h"
-#include "llvm/Support/RWMutex.h"
 #include "llvm/Support/Threading.h"
 
 #include <array>
+#include <mutex>
+#include <shared_mutex>
 #include <utility>
 
 #include <cinttypes>
 #include <cstdint>
 #include <cstring>
 
+#if defined(__APPLE__)
+#include <os/lock.h>
+#endif
+
 using namespace lldb_private;
 
+namespace {
+/// Lock guarding each shard of the ConstString pool. On Apple platforms it
+/// wraps os_unfair_lock, which is significantly faster than pthread_rwlock for
+/// concurrent writes, and roughly on par for concurrent reads. On all other
+/// platforms, it wraps std::shared_mutex, preserving reader/writer concurrency.
+///
+/// The class satisfies both Lockable and SharedLockable so it composes with
+/// std::lock_guard and std::shared_lock.
+class PoolMutex {
+public:
+  void lock() {
+#if defined(__APPLE__)
+    os_unfair_lock_lock(&m_lock);
+#else
+    m_lock.lock();
+#endif
+  }
+  void unlock() {
+#if defined(__APPLE__)
+    os_unfair_lock_unlock(&m_lock);
+#else
+    m_lock.unlock();
+#endif
+  }
+  void lock_shared() {
+#if defined(__APPLE__)
+    os_unfair_lock_lock(&m_lock);
+#else
+    m_lock.lock_shared();
+#endif
+  }
+  void unlock_shared() {
+#if defined(__APPLE__)
+    os_unfair_lock_unlock(&m_lock);
+#else
+    m_lock.unlock_shared();
+#endif
+  }
+
+private:
+#if defined(__APPLE__)
+  os_unfair_lock m_lock = OS_UNFAIR_LOCK_INIT;
+#else
+  std::shared_mutex m_lock;
+#endif
+};
+} // namespace
+
 class Pool {
 public:
   /// The default BumpPtrAllocatorImpl slab size.
@@ -80,7 +133,7 @@ class Pool {
   StringPoolValueType GetMangledCounterpart(const char *ccstr) {
     if (ccstr != nullptr) {
       const PoolEntry &pool = selectPool(llvm::StringRef(ccstr));
-      llvm::sys::SmartScopedReader<false> rlock(pool.m_mutex);
+      std::shared_lock<PoolMutex> lock(pool.m_mutex);
       return GetStringMapEntryFromKeyData(ccstr).getValue();
     }
     return nullptr;
@@ -104,13 +157,13 @@ class Pool {
       PoolEntry &pool = selectPool(string_hash);
 
       {
-        llvm::sys::SmartScopedReader<false> rlock(pool.m_mutex);
+        std::shared_lock<PoolMutex> lock(pool.m_mutex);
         auto it = pool.m_string_map.find(string_ref, string_hash);
         if (it != pool.m_string_map.end())
           return it->getKeyData();
       }
 
-      llvm::sys::SmartScopedWriter<false> wlock(pool.m_mutex);
+      std::lock_guard<PoolMutex> lock(pool.m_mutex);
       StringPoolEntryType &entry =
           *pool.m_string_map
                .insert(std::make_pair(string_ref, nullptr), string_hash)
@@ -128,7 +181,7 @@ class Pool {
     {
       const uint32_t demangled_hash = StringPool::hash(demangled);
       PoolEntry &pool = selectPool(demangled_hash);
-      llvm::sys::SmartScopedWriter<false> wlock(pool.m_mutex);
+      std::lock_guard<PoolMutex> lock(pool.m_mutex);
 
       // Make or update string pool entry with the mangled counterpart
       StringPool &map = pool.m_string_map;
@@ -145,7 +198,7 @@ class Pool {
       // Now assign the demangled const string as the counterpart of the
       // mangled const string...
       PoolEntry &pool = selectPool(llvm::StringRef(mangled_ccstr));
-      llvm::sys::SmartScopedWriter<false> wlock(pool.m_mutex);
+      std::lock_guard<PoolMutex> lock(pool.m_mutex);
       GetStringMapEntryFromKeyData(mangled_ccstr).setValue(demangled_ccstr);
     }
 
@@ -165,7 +218,7 @@ class Pool {
   ConstString::MemoryStats GetMemoryStats() const {
     ConstString::MemoryStats stats;
     for (const auto &pool : m_string_pools) {
-      llvm::sys::SmartScopedReader<false> rlock(pool.m_mutex);
+      std::shared_lock<PoolMutex> lock(pool.m_mutex);
       const Allocator &alloc = pool.m_string_map.getAllocator();
       stats.bytes_total += alloc.getTotalMemory();
       stats.bytes_used += alloc.getBytesAllocated();
@@ -175,7 +228,7 @@ class Pool {
 
 protected:
   struct PoolEntry {
-    mutable llvm::sys::SmartRWMutex<false> m_mutex;
+    mutable PoolMutex m_mutex;
     StringPool m_string_map;
   };
 
diff --git a/lldb/unittests/Utility/ConstStringTest.cpp b/lldb/unittests/Utility/ConstStringTest.cpp
index 9fe74d1343971..70d1847aff0c7 100644
--- a/lldb/unittests/Utility/ConstStringTest.cpp
+++ b/lldb/unittests/Utility/ConstStringTest.cpp
@@ -10,6 +10,12 @@
 #include "llvm/Support/FormatVariadic.h"
 #include "gtest/gtest.h"
 
+#include <atomic>
+#include <random>
+#include <string>
+#include <thread>
+#include <vector>
+
 using namespace lldb_private;
 
 TEST(ConstStringTest, format_provider) {
@@ -152,3 +158,60 @@ TEST(ConstStringTest, StringConversions) {
   EXPECT_EQ(std::string("foo"), std::string_view(foo));
   EXPECT_EQ(std::string("foo"), std::string(foo));
 }
+
+// Stress-tests the ConstString pool from many threads at once with a mix of
+// reads (lookups of already-interned strings) and writes (interning new
+// strings). Intended both as a thread-safety regression check and as a
+// hand-runnable benchmark for changes to the pool's locking strategy. Run
+// timing is left to an external tool (e.g. hyperfine).
+TEST(ConstStringTest, ConcurrentReadsAndWritesBenchmark) {
+  // Pre-intern a set of "hot" strings that read operations will look up.
+  // Sized to spread across the pool's 256 shards so contention is realistic.
+  constexpr size_t kHotStringCount = 4096;
+  std::vector<std::string> hot_strings;
+  hot_strings.reserve(kHotStringCount);
+  for (size_t i = 0; i < kHotStringCount; ++i) {
+    hot_strings.push_back("ConstStringBench_hot_" + std::to_string(i));
+    ConstString cs(hot_strings.back());
+    (void)cs;
+  }
+
+  const unsigned num_threads =
+      std::max(2u, std::thread::hardware_concurrency());
+  constexpr size_t kIterationsPerThread = 200000;
+  // 80% reads / 20% writes, which is probably generous as most lock contentions
+  // is expected for writes while parsing the symtab in parallel.
+  constexpr int kReadPercent = 80;
+
+  std::atomic<bool> start{false};
+  std::vector<std::thread> threads;
+  threads.reserve(num_threads);
+
+  for (unsigned t = 0; t < num_threads; ++t) {
+    threads.emplace_back([&, t] {
+      while (!start.load(std::memory_order_acquire))
+        std::this_thread::yield();
+
+      std::mt19937 rng(static_cast<uint32_t>(t) * 1337u + 17u);
+      std::uniform_int_distribution<int> ratio_dist(0, 99);
+      std::uniform_int_distribution<size_t> hot_dist(0, kHotStringCount - 1);
+      uint64_t write_counter = 0;
+
+      for (size_t i = 0; i < kIterationsPerThread; ++i) {
+        if (ratio_dist(rng) < kReadPercent) {
+          ConstString cs(hot_strings[hot_dist(rng)]);
+          (void)cs;
+        } else {
+          std::string s = "ConstStringBench_tw_" + std::to_string(t) + "_" +
+                          std::to_string(write_counter++);
+          ConstString cs(s);
+          (void)cs;
+        }
+      }
+    });
+  }
+
+  start.store(true, std::memory_order_release);
+  for (auto &th : threads)
+    th.join();
+}



More information about the lldb-commits mailing list