[Mlir-commits] [mlir] [mlir] Fix race condition introduced in ThreadLocalCache (PR #93280)

[Jeff Niu]

================
@@ -25,28 +24,91 @@ namespace mlir {
 /// cache has very large lock contention.
 template <typename ValueT>
 class ThreadLocalCache {
+  struct PerInstanceState;
+
+  /// The "observer" is owned by a thread-local cache instance. It is
+  /// constructed the first time a `ThreadLocalCache` instance is accessed by a
+  /// thread, unless `perInstanceState` happens to get re-allocated to the same
+  /// address as a previous one. This class is destructed the thread in which
+  /// the `thread_local` cache lives is destroyed.
+  ///
+  /// This class is called the "observer" because while values cached in
+  /// thread-local caches are owned by `PerInstanceState`, a reference is stored
+  /// via this class in the TLC. With a double pointer, it knows when the
+  /// referenced value has been destroyed.
+  struct Observer {
+    Observer() : ptr(std::make_unique<ValueT *>(nullptr)) {}
+
+    /// This is the double pointer, explicitly allocated because we need to keep
+    /// the address stable if the TLC map re-allocates. It is owned by the
+    /// observer and shared with the value owner.
+    std::unique_ptr<ValueT *> ptr;
----------------
Mogball wrote:

TL;DR: compared to before, we had a single `weak_ptr<ValueT *>` that served the purposes of both `ptr` and `keepalive` in this new TLC entry. Splitting them up allows the common case to be faster by not having the atomic.

https://github.com/llvm/llvm-project/pull/93280