[PATCH] Add a lock() function in PassRegistry to speed up multi-thread synchronization.

Erik Eckstein eeckstein at apple.com
Fri Feb 20 05:24:36 PST 2015


With this patch I'm trying to solve the following problem:

I'm running llvm in multiple threads (each instance uses its private LLVMContext).
With a release-asserts build of llvm I got terrible multi-thread performance. I found that the problem is the mutex in PassRegistry::getPassInfo(), mainly called from PMDataManager::verifyPreservedAnalysis().

My first idea was to make an option to disable verifyPreservedAnalysis(), even in an asserts-build. 
But I didn't like it because I don't want to give up this verification and the mutex also cause overhead outside verifyPreservedAnalysis().

So I did the following:
I added a lock() function in PassRegistry which I call when I'm sure that all passes are registered.
After the PassRegistry is locked it can safely access its maps without using a mutex.

This completely solves the multi-thread performance problem.
The use of the lock() function is optional. If not used, nothing changes.

http://reviews.llvm.org/D7787

Files:
  include/llvm/PassRegistry.h
  lib/IR/PassRegistry.cpp

Index: include/llvm/PassRegistry.h
===================================================================
--- include/llvm/PassRegistry.h
+++ include/llvm/PassRegistry.h
@@ -41,6 +41,9 @@
 class PassRegistry {
   mutable sys::SmartRWMutex<true> Lock;
 
+  /// Only if false, synchronization must use the Lock mutex.
+  std::atomic<bool> locked;
+
   /// PassInfoMap - Keep track of the PassInfo object for each registered pass.
   typedef DenseMap<const void *, const PassInfo *> MapType;
   MapType PassInfoMap;
@@ -52,7 +55,7 @@
   std::vector<PassRegistrationListener *> Listeners;
 
 public:
-  PassRegistry() {}
+  PassRegistry() : locked(false) {}
   ~PassRegistry();
 
   /// getPassRegistry - Access the global registry object, which is
@@ -60,6 +63,10 @@
   /// llvm_shutdown.
   static PassRegistry *getPassRegistry();
 
+  /// Enables fast thread synchronization in getPassInfo().
+  /// After calling lock() no more passes may be registered.
+  void lock() { locked = true; }
+
   /// getPassInfo - Look up a pass' corresponding PassInfo, indexed by the pass'
   /// type identifier (&MyPass::ID).
   const PassInfo *getPassInfo(const void *TI) const;
Index: lib/IR/PassRegistry.cpp
===================================================================
--- lib/IR/PassRegistry.cpp
+++ lib/IR/PassRegistry.cpp
@@ -39,15 +39,29 @@
 PassRegistry::~PassRegistry() {}
 
 const PassInfo *PassRegistry::getPassInfo(const void *TI) const {
-  sys::SmartScopedReader<true> Guard(Lock);
-  MapType::const_iterator I = PassInfoMap.find(TI);
-  return I != PassInfoMap.end() ? I->second : nullptr;
+  if (locked) {
+    // We don't need thread synchronization because the PassInfoMap is read-only
+    // after the PassRegistry is locked.
+    MapType::const_iterator I = PassInfoMap.find(TI);
+    return I != PassInfoMap.end() ? I->second : nullptr;
+  } else {
+    sys::SmartScopedReader<true> Guard(Lock);
+    MapType::const_iterator I = PassInfoMap.find(TI);
+    return I != PassInfoMap.end() ? I->second : nullptr;
+  }
 }
 
 const PassInfo *PassRegistry::getPassInfo(StringRef Arg) const {
-  sys::SmartScopedReader<true> Guard(Lock);
-  StringMapType::const_iterator I = PassInfoStringMap.find(Arg);
-  return I != PassInfoStringMap.end() ? I->second : nullptr;
+  if (locked) {
+    // We don't need thread synchronization because the PassInfoStringMap is
+    // read-only after the PassRegistry is locked.
+    StringMapType::const_iterator I = PassInfoStringMap.find(Arg);
+    return I != PassInfoStringMap.end() ? I->second : nullptr;
+  } else {
+    sys::SmartScopedReader<true> Guard(Lock);
+    StringMapType::const_iterator I = PassInfoStringMap.find(Arg);
+    return I != PassInfoStringMap.end() ? I->second : nullptr;
+  }
 }
 
 //===----------------------------------------------------------------------===//
@@ -55,6 +69,9 @@
 //
 
 void PassRegistry::registerPass(const PassInfo &PI, bool ShouldFree) {
+
+  assert(!locked && "Trying to register a pass in a locked PassRegistry");
+
   sys::SmartScopedWriter<true> Guard(Lock);
   bool Inserted =
       PassInfoMap.insert(std::make_pair(PI.getTypeInfo(), &PI)).second;
@@ -68,6 +85,8 @@
 
   if (ShouldFree)
     ToFree.push_back(std::unique_ptr<const PassInfo>(&PI));
+
+  assert(!locked && "PassRegistry locked during registering a pass");
 }
 
 void PassRegistry::enumerateWith(PassRegistrationListener *L) {

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D7787.20392.patch
Type: text/x-patch
Size: 3386 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150220/53f00425/attachment.bin>


More information about the llvm-commits mailing list