[llvm] 33c4421 - [Reland][ADT][ConcurrentHashTable] adapt ConcurrentHashTable and its users to LLVM_ENABLE_THREADS=0 mode.

Alexey Lapshin via llvm-commits llvm-commits at lists.llvm.org
Wed Apr 12 02:36:26 PDT 2023


Author: Alexey Lapshin
Date: 2023-04-12T11:28:44+02:00
New Revision: 33c442118f3ed988190b9fd3f7c065612803ef7b

URL: https://github.com/llvm/llvm-project/commit/33c442118f3ed988190b9fd3f7c065612803ef7b
DIFF: https://github.com/llvm/llvm-project/commit/33c442118f3ed988190b9fd3f7c065612803ef7b.diff

LOG: [Reland][ADT][ConcurrentHashTable] adapt ConcurrentHashTable and its users to LLVM_ENABLE_THREADS=0 mode.

This patch hides thread specific handling under LLVM_ENABLE_THREADS guard.
It also removes usages of thread_local variables, since it has a weak
support on some platforms. Instead, the patch uses single mutex for locking
allocator. That may be replaced with more effective allocator later.
f.e. D142318

Differential Revision: https://reviews.llvm.org/D147649

Added: 
    

Modified: 
    llvm/include/llvm/ADT/ConcurrentHashtable.h
    llvm/include/llvm/DWARFLinkerParallel/StringPool.h
    llvm/lib/DWARFLinkerParallel/StringPool.cpp
    llvm/unittests/ADT/ConcurrentHashtableTest.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/ADT/ConcurrentHashtable.h b/llvm/include/llvm/ADT/ConcurrentHashtable.h
index fea5bde587605..438159d5c736b 100644
--- a/llvm/include/llvm/ADT/ConcurrentHashtable.h
+++ b/llvm/include/llvm/ADT/ConcurrentHashtable.h
@@ -175,8 +175,10 @@ class ConcurrentHashTableByPtr {
     Bucket &CurBucket = BucketsArray[getBucketIdx(Hash)];
     uint32_t ExtHashBits = getExtHashBits(Hash);
 
+#if LLVM_ENABLE_THREADS
     // Lock bucket.
     CurBucket.Guard.lock();
+#endif
 
     HashesPtr BucketHashes = CurBucket.Hashes;
     DataPtr BucketEntries = CurBucket.Entries;
@@ -194,7 +196,9 @@ class ConcurrentHashTableByPtr {
         CurBucket.NumberOfEntries++;
         RehashBucket(CurBucket);
 
+#if LLVM_ENABLE_THREADS
         CurBucket.Guard.unlock();
+#endif
 
         return {NewData, true};
       }
@@ -204,7 +208,9 @@ class ConcurrentHashTableByPtr {
         KeyDataTy *EntryData = BucketEntries[CurEntryIdx];
         if (Info::isEqual(Info::getKey(*EntryData), NewValue)) {
           // Already existed entry matched with inserted data is found.
+#if LLVM_ENABLE_THREADS
           CurBucket.Guard.unlock();
+#endif
 
           return {EntryData, false};
         }
@@ -283,8 +289,10 @@ class ConcurrentHashTableByPtr {
     // [Size] entries.
     DataPtr Entries = nullptr;
 
+#if LLVM_ENABLE_THREADS
     // Mutex for this bucket.
     std::mutex Guard;
+#endif
   };
 
   // Reallocate and rehash bucket if this is full enough.

diff  --git a/llvm/include/llvm/DWARFLinkerParallel/StringPool.h b/llvm/include/llvm/DWARFLinkerParallel/StringPool.h
index 9eb0d759f6a9c..5406e03c8af19 100644
--- a/llvm/include/llvm/DWARFLinkerParallel/StringPool.h
+++ b/llvm/include/llvm/DWARFLinkerParallel/StringPool.h
@@ -22,19 +22,26 @@ namespace dwarflinker_parallel {
 /// and a string body which is placed right after StringEntry.
 using StringEntry = StringMapEntry<DwarfStringPoolEntry *>;
 
-class PerThreadStringAllocator
-    : public AllocatorBase<PerThreadStringAllocator> {
+class StringAllocator : public AllocatorBase<StringAllocator> {
 public:
   inline LLVM_ATTRIBUTE_RETURNS_NONNULL void *Allocate(size_t Size,
                                                        size_t Alignment) {
-    return ThreadLocalAllocator.Allocate(Size, Align(Alignment));
+#if LLVM_ENABLE_THREADS
+    std::lock_guard<std::mutex> Guard(AllocatorMutex);
+#endif
+
+    return Allocator.Allocate(Size, Align(Alignment));
   }
 
   // Pull in base class overloads.
-  using AllocatorBase<PerThreadStringAllocator>::Allocate;
+  using AllocatorBase<StringAllocator>::Allocate;
 
 private:
-  static thread_local BumpPtrAllocator ThreadLocalAllocator;
+#if LLVM_ENABLE_THREADS
+  std::mutex AllocatorMutex;
+#endif
+
+  BumpPtrAllocator Allocator;
 };
 
 class StringPoolEntryInfo {
@@ -56,29 +63,27 @@ class StringPoolEntryInfo {
 
   /// \returns newly created object of KeyDataTy type.
   static inline StringEntry *create(const StringRef &Key,
-                                    PerThreadStringAllocator &Allocator) {
+                                    StringAllocator &Allocator) {
     return StringEntry::create(Key, Allocator);
   }
 };
 
-class StringPool : public ConcurrentHashTableByPtr<StringRef, StringEntry,
-                                                   PerThreadStringAllocator,
-                                                   StringPoolEntryInfo> {
+class StringPool
+    : public ConcurrentHashTableByPtr<StringRef, StringEntry, StringAllocator,
+                                      StringPoolEntryInfo> {
 public:
   StringPool()
-      : ConcurrentHashTableByPtr<StringRef, StringEntry,
-                                 PerThreadStringAllocator, StringPoolEntryInfo>(
-            Allocator) {}
+      : ConcurrentHashTableByPtr<StringRef, StringEntry, StringAllocator,
+                                 StringPoolEntryInfo>(Allocator) {}
 
   StringPool(size_t InitialSize)
-      : ConcurrentHashTableByPtr<StringRef, StringEntry,
-                                 PerThreadStringAllocator, StringPoolEntryInfo>(
-            Allocator, InitialSize) {}
+      : ConcurrentHashTableByPtr<StringRef, StringEntry, StringAllocator,
+                                 StringPoolEntryInfo>(Allocator, InitialSize) {}
 
-  PerThreadStringAllocator &getAllocatorRef() { return Allocator; }
+  StringAllocator &getAllocatorRef() { return Allocator; }
 
 private:
-  PerThreadStringAllocator Allocator;
+  StringAllocator Allocator;
 };
 
 } // end of namespace dwarflinker_parallel

diff  --git a/llvm/lib/DWARFLinkerParallel/StringPool.cpp b/llvm/lib/DWARFLinkerParallel/StringPool.cpp
index aa8bc61e2d324..fbff6b05e3a54 100644
--- a/llvm/lib/DWARFLinkerParallel/StringPool.cpp
+++ b/llvm/lib/DWARFLinkerParallel/StringPool.cpp
@@ -7,6 +7,3 @@
 //===----------------------------------------------------------------------===//
 
 #include "llvm/DWARFLinkerParallel/StringPool.h"
-
-thread_local llvm::BumpPtrAllocator
-    llvm::dwarflinker_parallel::PerThreadStringAllocator::ThreadLocalAllocator;

diff  --git a/llvm/unittests/ADT/ConcurrentHashtableTest.cpp b/llvm/unittests/ADT/ConcurrentHashtableTest.cpp
index 8138633bc6de9..895bde85ea9e7 100644
--- a/llvm/unittests/ADT/ConcurrentHashtableTest.cpp
+++ b/llvm/unittests/ADT/ConcurrentHashtableTest.cpp
@@ -36,25 +36,38 @@ class String {
   std::array<char, 0x20> ExtraData;
 };
 
-static thread_local BumpPtrAllocator ThreadLocalAllocator;
-class PerThreadAllocator : public AllocatorBase<PerThreadAllocator> {
+class SimpleAllocator : public AllocatorBase<SimpleAllocator> {
 public:
   inline LLVM_ATTRIBUTE_RETURNS_NONNULL void *Allocate(size_t Size,
                                                        size_t Alignment) {
-    return ThreadLocalAllocator.Allocate(Size, Align(Alignment));
+#if LLVM_ENABLE_THREADS
+    std::lock_guard<std::mutex> Guard(AllocatorMutex);
+#endif
+
+    return Allocator.Allocate(Size, Align(Alignment));
   }
-  inline size_t getBytesAllocated() const {
-    return ThreadLocalAllocator.getBytesAllocated();
+  inline size_t getBytesAllocated() {
+#if LLVM_ENABLE_THREADS
+    std::lock_guard<std::mutex> Guard(AllocatorMutex);
+#endif
+
+    return Allocator.getBytesAllocated();
   }
 
   // Pull in base class overloads.
-  using AllocatorBase<PerThreadAllocator>::Allocate;
+  using AllocatorBase<SimpleAllocator>::Allocate;
+
+protected:
+#if LLVM_ENABLE_THREADS
+  std::mutex AllocatorMutex;
+#endif
+  BumpPtrAllocator Allocator;
 } Allocator;
 
 TEST(ConcurrentHashTableTest, AddStringEntries) {
   ConcurrentHashTableByPtr<
-      std::string, String, PerThreadAllocator,
-      ConcurrentHashTableInfoByPtr<std::string, String, PerThreadAllocator>>
+      std::string, String, SimpleAllocator,
+      ConcurrentHashTableInfoByPtr<std::string, String, SimpleAllocator>>
       HashTable(Allocator, 10);
 
   size_t AllocatedBytesAtStart = Allocator.getBytesAllocated();
@@ -102,8 +115,8 @@ TEST(ConcurrentHashTableTest, AddStringEntries) {
 TEST(ConcurrentHashTableTest, AddStringMultiplueEntries) {
   const size_t NumElements = 10000;
   ConcurrentHashTableByPtr<
-      std::string, String, PerThreadAllocator,
-      ConcurrentHashTableInfoByPtr<std::string, String, PerThreadAllocator>>
+      std::string, String, SimpleAllocator,
+      ConcurrentHashTableInfoByPtr<std::string, String, SimpleAllocator>>
       HashTable(Allocator);
 
   // Check insertion.
@@ -147,8 +160,8 @@ TEST(ConcurrentHashTableTest, AddStringMultiplueEntriesWithResize) {
   // Number of elements exceeds original size, thus hashtable should be resized.
   const size_t NumElements = 20000;
   ConcurrentHashTableByPtr<
-      std::string, String, PerThreadAllocator,
-      ConcurrentHashTableInfoByPtr<std::string, String, PerThreadAllocator>>
+      std::string, String, SimpleAllocator,
+      ConcurrentHashTableInfoByPtr<std::string, String, SimpleAllocator>>
       HashTable(Allocator, 100);
 
   // Check insertion.
@@ -191,8 +204,8 @@ TEST(ConcurrentHashTableTest, AddStringMultiplueEntriesWithResize) {
 TEST(ConcurrentHashTableTest, AddStringEntriesParallel) {
   const size_t NumElements = 10000;
   ConcurrentHashTableByPtr<
-      std::string, String, PerThreadAllocator,
-      ConcurrentHashTableInfoByPtr<std::string, String, PerThreadAllocator>>
+      std::string, String, SimpleAllocator,
+      ConcurrentHashTableInfoByPtr<std::string, String, SimpleAllocator>>
       HashTable(Allocator);
 
   // Check parallel insertion.
@@ -235,8 +248,8 @@ TEST(ConcurrentHashTableTest, AddStringEntriesParallel) {
 TEST(ConcurrentHashTableTest, AddStringEntriesParallelWithResize) {
   const size_t NumElements = 20000;
   ConcurrentHashTableByPtr<
-      std::string, String, PerThreadAllocator,
-      ConcurrentHashTableInfoByPtr<std::string, String, PerThreadAllocator>>
+      std::string, String, SimpleAllocator,
+      ConcurrentHashTableInfoByPtr<std::string, String, SimpleAllocator>>
       HashTable(Allocator, 100);
 
   // Check parallel insertion.


        


More information about the llvm-commits mailing list