Merge pull request swiftlang#11066 from augusto2112/cache-failure-find-types

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

Author: adrian-prantl

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