[lldb] Fix caching, add lookup failures to TSSTyperef's LookupClangType

By profiling LLDB running frame variable in a large application with
many many .o files, I noticed that one of the biggest bottlenecks of the
operation was looking (and failing) to find a certain potential clang
type over and over again.

While fixing this I found that we will cache the result, but not the
decl context to find that result, which introduces the risk of
returning the wrong type if the type is cached, but a different decl
context is used.

Author: augusto2112

lldb/include/lldb/Utility/ThreadSafeStringMap.h

@@ -0,0 +1,61 @@
+//===-- ThreadSafeStringMap.h ------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLDB_UTILITY_THREADSAFESTRINGMAP_H
+#define LLDB_UTILITY_THREADSAFESTRINGMAP_H
+
+#include <mutex>
+
+#include "llvm/ADT/StringMap.h"
+
+namespace lldb_private {
+
+template <typename _ValueType> class ThreadSafeStringMap {
+public:
+  typedef llvm::StringMap<_ValueType> LLVMMapType;
+
+  ThreadSafeStringMap(unsigned map_initial_capacity = 0)
+      : m_map(map_initial_capacity), m_mutex() {}
+
+  void Insert(llvm::StringRef k, _ValueType v) {
+    std::lock_guard<std::mutex> guard(m_mutex);
+    m_map.insert(std::make_pair(k, v));
+  }
+
+  void Erase(llvm::StringRef k) {
+    std::lock_guard<std::mutex> guard(m_mutex);
+    m_map.erase(k);
+  }
+
+  _ValueType Lookup(llvm::StringRef k) {
+    std::lock_guard<std::mutex> guard(m_mutex);
+    return m_map.lookup(k);
+  }
+
+  bool Lookup(llvm::StringRef k, _ValueType &v) {
+    std::lock_guard<std::mutex> guard(m_mutex);
+    auto iter = m_map.find(k), end = m_map.end();
+    if (iter == end)
+      return false;
+    v = iter->second;
+    return true;
+  }
+
+  void Clear() {
+    std::lock_guard<std::mutex> guard(m_mutex);
+    m_map.clear();
+  }
+
+protected:
+  LLVMMapType m_map;
+  std::mutex m_mutex;
+};
+
+} // namespace lldb_private
+
+#endif // LLDB_UTILITY_THREADSAFESTRINGMAP_H

lldb/source/Plugins/TypeSystem/Swift/TypeSystemSwiftTypeRef.cpp

@@ -422,35 +422,42 @@ TypeSP TypeSystemSwiftTypeRef::LookupClangType(
 TypeSP TypeSystemSwiftTypeRefForExpressions::LookupClangType(
     StringRef name_ref, llvm::ArrayRef<CompilerContext> decl_context,
     SymbolContext sc) {
-  // Check the cache first. Negative results are also cached.
+  // Build up the cache key (the concatenation of the name and the stringified
+  // decl context).
+  StreamString stream;
+  stream << name_ref;
+  for (auto &context : decl_context)
+    context.Dump(stream);
+
   TypeSP result;
-  ConstString name(name_ref);
-  if (m_clang_type_cache.Lookup(name.AsCString(), result))
+  // Check the cache first. Negative results are also cached.
+  auto key = stream.GetString();
+  if (m_clang_type_cache.Lookup(key, result))
     return result;
 
-  ModuleSP cur_module = sc.module_sp;
   auto lookup = [&](const ModuleSP &m) -> bool {
-    // Already visited this.
-    if (m == cur_module)
-      return true;
-
     // Don't recursively call into LookupClangTypes() to avoid filling
     // hundreds of image caches with negative results.
     result = ::LookupClangType(const_cast<Module &>(*m), decl_context);
     // Cache it in the expression context.
     if (result)
-      m_clang_type_cache.Insert(name.AsCString(), result);
+      m_clang_type_cache.Insert(key, result);
     return !result;
   };
 
   // Visit the current module first as a performance optimization heuristic.
+  ModuleSP cur_module = sc.module_sp;
   if (cur_module)
     if (!lookup(cur_module))
       return result;
 
   if (TargetSP target_sp = GetTargetWP().lock())
     target_sp->GetImages().ForEach(lookup);
 
+  /// Cache the negative result. This is safe to do because ModulesDidLoad will
+  /// clear the cache.
+  if (!result)
+    m_clang_type_cache.Insert(key, result);
   return result;
 }
 

lldb/source/Plugins/TypeSystem/Swift/TypeSystemSwiftTypeRef.h

@@ -18,6 +18,7 @@
 #include "lldb/Symbol/SymbolContext.h"
 #include "lldb/Target/ExecutionContext.h"
 #include "lldb/Utility/ThreadSafeDenseMap.h"
+#include "lldb/Utility/ThreadSafeStringMap.h"
 #include "swift/Demangling/ManglingFlavor.h"
 
 // FIXME: needed only for the DenseMap.
@@ -686,8 +687,9 @@ class TypeSystemSwiftTypeRefForExpressions : public TypeSystemSwiftTypeRef {
   /// Perform all the implicit imports for the current frame.
   mutable std::unique_ptr<SymbolContext> m_initial_symbol_context_up;
   std::unique_ptr<SwiftPersistentExpressionState> m_persistent_state_up;
-  /// Map ConstString Clang type identifiers to Clang types.
-  ThreadSafeDenseMap<const char *, lldb::TypeSP> m_clang_type_cache;
+  /// Map ConstString Clang type identifiers and the concatenation of the
+  /// compiler context used to find them to Clang types.
+  ThreadSafeStringMap<lldb::TypeSP> m_clang_type_cache;
 };
 
 } // namespace lldb_private