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

[Erik Eckstein]

Thanks for fixing this!
(I didn't see this on macos)

> On 04 Mar 2015, at 20:16, Reid Kleckner <rnk at google.com> wrote:
> 
> I had to add a missing <atomic> include in r231277 for MSVC.
> 
> On Wed, Mar 4, 2015 at 10:57 AM, Erik Eckstein <eeckstein at apple.com <mailto:eeckstein at apple.com>> wrote:
> Author: eeckstein
> Date: Wed Mar  4 12:57:11 2015
> New Revision: 231276
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=231276&view=rev <http://llvm.org/viewvc/llvm-project?rev=231276&view=rev>
> Log:
> Add a lock() function in PassRegistry to speed up multi-thread synchronization.
> 
> When calling lock() after all passes are registered, the PassRegistry doesn't need a mutex anymore to look up passes.
> This speeds up multithreaded llvm execution by ~5% (tested with 4 threads).
> In an asserts build of llvm this has an even bigger impact.
> 
> Note that it's not required to use the lock function.
> 
> 
> Modified:
>     llvm/trunk/include/llvm/PassRegistry.h
>     llvm/trunk/lib/IR/PassRegistry.cpp
> 
> Modified: llvm/trunk/include/llvm/PassRegistry.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/PassRegistry.h?rev=231276&r1=231275&r2=231276&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/PassRegistry.h?rev=231276&r1=231275&r2=231276&view=diff>
> ==============================================================================
> --- llvm/trunk/include/llvm/PassRegistry.h (original)
> +++ llvm/trunk/include/llvm/PassRegistry.h Wed Mar  4 12:57:11 2015
> @@ -41,6 +41,9 @@ struct PassRegistrationListener;
>  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 @@ class PassRegistry {
>    std::vector<PassRegistrationListener *> Listeners;
> 
>  public:
> -  PassRegistry() {}
> +  PassRegistry() : locked(false) {}
>    ~PassRegistry();
> 
>    /// getPassRegistry - Access the global registry object, which is
> @@ -60,6 +63,10 @@ public:
>    /// 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;
> 
> Modified: llvm/trunk/lib/IR/PassRegistry.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/PassRegistry.cpp?rev=231276&r1=231275&r2=231276&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/PassRegistry.cpp?rev=231276&r1=231275&r2=231276&view=diff>
> ==============================================================================
> --- llvm/trunk/lib/IR/PassRegistry.cpp (original)
> +++ llvm/trunk/lib/IR/PassRegistry.cpp Wed Mar  4 12:57:11 2015
> @@ -13,6 +13,7 @@
>  //===----------------------------------------------------------------------===//
> 
>  #include "llvm/PassRegistry.h"
> +#include "llvm/ADT/Optional.h"
>  #include "llvm/IR/Function.h"
>  #include "llvm/PassSupport.h"
>  #include "llvm/Support/Compiler.h"
> @@ -39,13 +40,23 @@ PassRegistry *PassRegistry::getPassRegis
>  PassRegistry::~PassRegistry() {}
> 
>  const PassInfo *PassRegistry::getPassInfo(const void *TI) const {
> -  sys::SmartScopedReader<true> Guard(Lock);
> +  // We don't need thread synchronization after the PassRegistry is locked
> +  // (that means: is read-only).
> +  Optional<sys::SmartScopedReader<true>> Guard;
> +  if (!locked)
> +    Guard.emplace(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);
> +  // We don't need thread synchronization after the PassRegistry is locked
> +  // (that means: is read-only).
> +  Optional<sys::SmartScopedReader<true>> Guard;
> +  if (!locked)
> +    Guard.emplace(Lock);
> +
>    StringMapType::const_iterator I = PassInfoStringMap.find(Arg);
>    return I != PassInfoStringMap.end() ? I->second : nullptr;
>  }
> @@ -55,6 +66,9 @@ const PassInfo *PassRegistry::getPassInf
>  //
> 
>  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 +82,8 @@ void PassRegistry::registerPass(const Pa
> 
>    if (ShouldFree)
>      ToFree.push_back(std::unique_ptr<const PassInfo>(&PI));
> +
> +  assert(!locked && "PassRegistry locked during registering a pass");
>  }
> 
>  void PassRegistry::enumerateWith(PassRegistrationListener *L) {
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu <mailto:llvm-commits at cs.uiuc.edu>
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits <http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits>
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150304/ada5f022/attachment.html>