[Mlir-commits] [mlir] 87e59e4 - [mlir] Remove the Identifier ThreadLocalCache from MLIRContext

[River Riddle]

Author: River Riddle
Date: 2021-06-22T19:56:05Z
New Revision: 87e59e47e936f15e407dba1b963393dd96ff96fb

URL: https://github.com/llvm/llvm-project/commit/87e59e47e936f15e407dba1b963393dd96ff96fb
DIFF: https://github.com/llvm/llvm-project/commit/87e59e47e936f15e407dba1b963393dd96ff96fb.diff

LOG: [mlir] Remove the Identifier ThreadLocalCache from MLIRContext

This used to be important for reducing lock contention when accessing identifiers, but
the cost of the cache can be quite large if parsing in a multi-threaded context. After
D104167, the win of keeping a cache is not worth the cost.

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

Added: 
    

Modified: 
    mlir/lib/IR/MLIRContext.cpp

Removed: 
    


################################################################################
diff  --git a/mlir/lib/IR/MLIRContext.cpp b/mlir/lib/IR/MLIRContext.cpp
index 1041f1c9f5033..8aa4fe7db3cbd 100644
--- a/mlir/lib/IR/MLIRContext.cpp
+++ b/mlir/lib/IR/MLIRContext.cpp
@@ -24,7 +24,6 @@
 #include "mlir/IR/OpImplementation.h"
 #include "mlir/IR/Types.h"
 #include "mlir/Support/DebugAction.h"
-#include "mlir/Support/ThreadLocalCache.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/SetVector.h"
@@ -275,10 +274,6 @@ class MLIRContextImpl {
   llvm::StringMap<PointerUnion<Dialect *, MLIRContext *>,
                   llvm::BumpPtrAllocator &>
       identifiers;
-  /// A thread local cache of identifiers to reduce lock contention.
-  ThreadLocalCache<llvm::StringMap<
-      llvm::StringMapEntry<PointerUnion<Dialect *, MLIRContext *>> *>>
-      localIdentifierCache;
 
   /// An allocator used for AbstractAttribute and AbstractType objects.
   llvm::BumpPtrAllocator abstractDialectSymbolAllocator;
@@ -811,26 +806,18 @@ Identifier Identifier::get(const Twine &string, MLIRContext *context) {
     return Identifier(&*insertedIt.first);
   }
 
-  // Check for an existing instance in the local cache.
-  auto *&localEntry = (*impl.localIdentifierCache)[str];
-  if (localEntry)
-    return Identifier(localEntry);
-
   // Check for an existing identifier in read-only mode.
   {
     llvm::sys::SmartScopedReader<true> contextLock(impl.identifierMutex);
     auto it = impl.identifiers.find(str);
-    if (it != impl.identifiers.end()) {
-      localEntry = &*it;
-      return Identifier(localEntry);
-    }
+    if (it != impl.identifiers.end())
+      return Identifier(&*it);
   }
 
   // Acquire a writer-lock so that we can safely create the new instance.
   llvm::sys::SmartScopedWriter<true> contextLock(impl.identifierMutex);
   auto it = impl.identifiers.insert({str, getDialectOrContext()}).first;
-  localEntry = &*it;
-  return Identifier(localEntry);
+  return Identifier(&*it);
 }
 
 Dialect *Identifier::getDialect() {