Merge pull request #9508 from swiftlang/cherrypick-parallel-load

[lldb] Create dependent modules in parallel (llvm#114507)

Author: JDevlieghere

lldb/include/lldb/Target/PathMappingList.h

@@ -25,7 +25,6 @@ class PathMappingList {
   typedef void (*ChangedCallback)(const PathMappingList &path_list,
                                   void *baton);
 
-  // Constructors and Destructors
   PathMappingList();
 
   PathMappingList(ChangedCallback callback, void *callback_baton);
@@ -53,12 +52,12 @@ class PathMappingList {
   llvm::json::Value ToJSON();
 
   bool IsEmpty() const {
-    std::lock_guard<std::recursive_mutex> lock(m_mutex);
+    std::lock_guard<std::mutex> lock(m_pairs_mutex);
     return m_pairs.empty();
   }
 
   size_t GetSize() const {
-    std::lock_guard<std::recursive_mutex> lock(m_mutex);
+    std::lock_guard<std::mutex> lock(m_pairs_mutex);
     return m_pairs.size();
   }
 
@@ -134,28 +133,35 @@ class PathMappingList {
   ///     The newly remapped filespec that is guaranteed to exist.
   std::optional<FileSpec> FindFile(const FileSpec &orig_spec) const;
 
-  uint32_t FindIndexForPath(llvm::StringRef path) const;
-
   uint32_t GetModificationID() const {
-    std::lock_guard<std::recursive_mutex> lock(m_mutex);
+    std::lock_guard<std::mutex> lock(m_pairs_mutex);
     return m_mod_id;
   }
 
 protected:
-  mutable std::recursive_mutex m_mutex;
   typedef std::pair<ConstString, ConstString> pair;
   typedef std::vector<pair> collection;
   typedef collection::iterator iterator;
   typedef collection::const_iterator const_iterator;
 
+  void AppendNoLock(llvm::StringRef path, llvm::StringRef replacement);
+  uint32_t FindIndexForPathNoLock(llvm::StringRef path) const;
+  void Notify(bool notify) const;
+
   iterator FindIteratorForPath(ConstString path);
 
   const_iterator FindIteratorForPath(ConstString path) const;
 
   collection m_pairs;
+  mutable std::mutex m_pairs_mutex;
+
   ChangedCallback m_callback = nullptr;
   void *m_callback_baton = nullptr;
-  uint32_t m_mod_id = 0; // Incremented anytime anything is added or removed.
+  mutable std::mutex m_callback_mutex;
+
+  /// Incremented anytime anything is added to or removed from m_pairs. Also
+  /// protected by m_pairs_mutex.
+  uint32_t m_mod_id = 0;
 };
 
 } // namespace lldb_private

lldb/source/Target/PathMappingList.cpp

@@ -48,7 +48,8 @@ PathMappingList::PathMappingList(const PathMappingList &rhs)
 
 const PathMappingList &PathMappingList::operator=(const PathMappingList &rhs) {
   if (this != &rhs) {
-    std::scoped_lock<std::recursive_mutex, std::recursive_mutex> locks(m_mutex, rhs.m_mutex);
+    std::scoped_lock<std::mutex, std::mutex, std::mutex> locks(
+        m_callback_mutex, m_pairs_mutex, rhs.m_pairs_mutex);
     m_pairs = rhs.m_pairs;
     m_callback = nullptr;
     m_callback_baton = nullptr;
@@ -59,85 +60,111 @@ const PathMappingList &PathMappingList::operator=(const PathMappingList &rhs) {
 
 PathMappingList::~PathMappingList() = default;
 
-void PathMappingList::Append(llvm::StringRef path, llvm::StringRef replacement,
-                             bool notify) {
-  std::lock_guard<std::recursive_mutex> lock(m_mutex);
+void PathMappingList::AppendNoLock(llvm::StringRef path,
+                                   llvm::StringRef replacement) {
   ++m_mod_id;
   m_pairs.emplace_back(pair(NormalizePath(path), NormalizePath(replacement)));
-  if (notify && m_callback)
-    m_callback(*this, m_callback_baton);
+}
+
+void PathMappingList::Notify(bool notify) const {
+  ChangedCallback callback = nullptr;
+  void *baton = nullptr;
+  {
+    std::lock_guard<std::mutex> lock(m_callback_mutex);
+    callback = m_callback;
+    baton = m_callback_baton;
+  }
+  if (notify && callback)
+    callback(*this, baton);
+}
+
+void PathMappingList::Append(llvm::StringRef path, llvm::StringRef replacement,
+                             bool notify) {
+  {
+    std::lock_guard<std::mutex> lock(m_pairs_mutex);
+    AppendNoLock(path, replacement);
+  }
+  Notify(notify);
 }
 
 void PathMappingList::Append(const PathMappingList &rhs, bool notify) {
-  std::scoped_lock<std::recursive_mutex, std::recursive_mutex> locks(m_mutex, rhs.m_mutex);
-  ++m_mod_id;
-  if (!rhs.m_pairs.empty()) {
+  {
+    std::scoped_lock<std::mutex, std::mutex> locks(m_pairs_mutex,
+                                                   rhs.m_pairs_mutex);
+    ++m_mod_id;
+    if (rhs.m_pairs.empty())
+      return;
     const_iterator pos, end = rhs.m_pairs.end();
     for (pos = rhs.m_pairs.begin(); pos != end; ++pos)
       m_pairs.push_back(*pos);
-    if (notify && m_callback)
-      m_callback(*this, m_callback_baton);
   }
+  Notify(notify);
 }
 
 bool PathMappingList::AppendUnique(llvm::StringRef path,
                                    llvm::StringRef replacement, bool notify) {
   auto normalized_path = NormalizePath(path);
   auto normalized_replacement = NormalizePath(replacement);
-  std::lock_guard<std::recursive_mutex> lock(m_mutex);
-  for (const auto &pair : m_pairs) {
-    if (pair.first.GetStringRef() == normalized_path &&
-        pair.second.GetStringRef() == normalized_replacement)
-      return false;
+  {
+    std::lock_guard<std::mutex> lock(m_pairs_mutex);
+    for (const auto &pair : m_pairs) {
+      if (pair.first.GetStringRef() == normalized_path &&
+          pair.second.GetStringRef() == normalized_replacement)
+        return false;
+    }
+    AppendNoLock(path, replacement);
   }
-  Append(path, replacement, notify);
+  Notify(notify);
   return true;
 }
 
 void PathMappingList::Insert(llvm::StringRef path, llvm::StringRef replacement,
                              uint32_t index, bool notify) {
-  std::lock_guard<std::recursive_mutex> lock(m_mutex);
-  ++m_mod_id;
-  iterator insert_iter;
-  if (index >= m_pairs.size())
-    insert_iter = m_pairs.end();
-  else
-    insert_iter = m_pairs.begin() + index;
-  m_pairs.emplace(insert_iter, pair(NormalizePath(path),
-                                    NormalizePath(replacement)));
-  if (notify && m_callback)
-    m_callback(*this, m_callback_baton);
+  {
+    std::lock_guard<std::mutex> lock(m_pairs_mutex);
+    ++m_mod_id;
+    iterator insert_iter;
+    if (index >= m_pairs.size())
+      insert_iter = m_pairs.end();
+    else
+      insert_iter = m_pairs.begin() + index;
+    m_pairs.emplace(insert_iter,
+                    pair(NormalizePath(path), NormalizePath(replacement)));
+  }
+  Notify(notify);
 }
 
 bool PathMappingList::Replace(llvm::StringRef path, llvm::StringRef replacement,
                               uint32_t index, bool notify) {
-  std::lock_guard<std::recursive_mutex> lock(m_mutex);
-  if (index >= m_pairs.size())
-    return false;
-  ++m_mod_id;
-  m_pairs[index] = pair(NormalizePath(path), NormalizePath(replacement));
-  if (notify && m_callback)
-    m_callback(*this, m_callback_baton);
+  {
+    std::lock_guard<std::mutex> lock(m_pairs_mutex);
+    if (index >= m_pairs.size())
+      return false;
+    ++m_mod_id;
+    m_pairs[index] = pair(NormalizePath(path), NormalizePath(replacement));
+  }
+  Notify(notify);
   return true;
 }
 
 bool PathMappingList::Remove(size_t index, bool notify) {
-  std::lock_guard<std::recursive_mutex> lock(m_mutex);
-  if (index >= m_pairs.size())
-    return false;
+  {
+    std::lock_guard<std::mutex> lock(m_pairs_mutex);
+    if (index >= m_pairs.size())
+      return false;
 
-  ++m_mod_id;
-  iterator iter = m_pairs.begin() + index;
-  m_pairs.erase(iter);
-  if (notify && m_callback)
-    m_callback(*this, m_callback_baton);
+    ++m_mod_id;
+    iterator iter = m_pairs.begin() + index;
+    m_pairs.erase(iter);
+  }
+  Notify(notify);
   return true;
 }
 
 // For clients which do not need the pair index dumped, pass a pair_index >= 0
 // to only dump the indicated pair.
 void PathMappingList::Dump(Stream *s, int pair_index) {
-  std::lock_guard<std::recursive_mutex> lock(m_mutex);
+  std::lock_guard<std::mutex> lock(m_pairs_mutex);
   unsigned int numPairs = m_pairs.size();
 
   if (pair_index < 0) {
@@ -155,7 +182,7 @@ void PathMappingList::Dump(Stream *s, int pair_index) {
 
 llvm::json::Value PathMappingList::ToJSON() {
   llvm::json::Array entries;
-  std::lock_guard<std::recursive_mutex> lock(m_mutex);
+  std::lock_guard<std::mutex> lock(m_pairs_mutex);
   for (const auto &pair : m_pairs) {
     llvm::json::Array entry{pair.first.GetStringRef().str(),
                             pair.second.GetStringRef().str()};
@@ -165,12 +192,13 @@ llvm::json::Value PathMappingList::ToJSON() {
 }
 
 void PathMappingList::Clear(bool notify) {
-  std::lock_guard<std::recursive_mutex> lock(m_mutex);
-  if (!m_pairs.empty())
-    ++m_mod_id;
-  m_pairs.clear();
-  if (notify && m_callback)
-    m_callback(*this, m_callback_baton);
+  {
+    std::lock_guard<std::mutex> lock(m_pairs_mutex);
+    if (!m_pairs.empty())
+      ++m_mod_id;
+    m_pairs.clear();
+  }
+  Notify(notify);
 }
 
 bool PathMappingList::RemapPath(ConstString path,
@@ -196,7 +224,7 @@ static void AppendPathComponents(FileSpec &path, llvm::StringRef components,
 
 std::optional<FileSpec> PathMappingList::RemapPath(llvm::StringRef mapping_path,
                                                    bool only_if_exists) const {
-  std::lock_guard<std::recursive_mutex> lock(m_mutex);
+  std::lock_guard<std::mutex> lock(m_pairs_mutex);
   if (m_pairs.empty() || mapping_path.empty())
     return {};
   LazyBool path_is_relative = eLazyBoolCalculate;
@@ -235,7 +263,7 @@ std::optional<llvm::StringRef>
 PathMappingList::ReverseRemapPath(const FileSpec &file, FileSpec &fixed) const {
   std::string path = file.GetPath();
   llvm::StringRef path_ref(path);
-  std::lock_guard<std::recursive_mutex> lock(m_mutex);
+  std::lock_guard<std::mutex> lock(m_pairs_mutex);
   for (const auto &it : m_pairs) {
     llvm::StringRef removed_prefix = it.second.GetStringRef();
     if (!path_ref.consume_front(it.second.GetStringRef()))
@@ -264,34 +292,35 @@ PathMappingList::FindFile(const FileSpec &orig_spec) const {
 
 bool PathMappingList::Replace(llvm::StringRef path, llvm::StringRef new_path,
                               bool notify) {
-  std::lock_guard<std::recursive_mutex> lock(m_mutex);
-  uint32_t idx = FindIndexForPath(path);
-  if (idx < m_pairs.size()) {
+  {
+    std::lock_guard<std::mutex> lock(m_pairs_mutex);
+    uint32_t idx = FindIndexForPathNoLock(path);
+    if (idx >= m_pairs.size())
+      return false;
     ++m_mod_id;
     m_pairs[idx].second = ConstString(new_path);
-    if (notify && m_callback)
-      m_callback(*this, m_callback_baton);
-    return true;
   }
-  return false;
+  Notify(notify);
+  return true;
 }
 
 bool PathMappingList::Remove(ConstString path, bool notify) {
-  std::lock_guard<std::recursive_mutex> lock(m_mutex);
-  iterator pos = FindIteratorForPath(path);
-  if (pos != m_pairs.end()) {
+  {
+    std::lock_guard<std::mutex> lock(m_pairs_mutex);
+    iterator pos = FindIteratorForPath(path);
+    if (pos == m_pairs.end())
+      return false;
+
     ++m_mod_id;
     m_pairs.erase(pos);
-    if (notify && m_callback)
-      m_callback(*this, m_callback_baton);
-    return true;
   }
-  return false;
+  Notify(notify);
+  return true;
 }
 
 PathMappingList::const_iterator
 PathMappingList::FindIteratorForPath(ConstString path) const {
-  std::lock_guard<std::recursive_mutex> lock(m_mutex);
+  std::lock_guard<std::mutex> lock(m_pairs_mutex);
   const_iterator pos;
   const_iterator begin = m_pairs.begin();
   const_iterator end = m_pairs.end();
@@ -305,7 +334,7 @@ PathMappingList::FindIteratorForPath(ConstString path) const {
 
 PathMappingList::iterator
 PathMappingList::FindIteratorForPath(ConstString path) {
-  std::lock_guard<std::recursive_mutex> lock(m_mutex);
+  std::lock_guard<std::mutex> lock(m_pairs_mutex);
   iterator pos;
   iterator begin = m_pairs.begin();
   iterator end = m_pairs.end();
@@ -319,7 +348,7 @@ PathMappingList::FindIteratorForPath(ConstString path) {
 
 bool PathMappingList::GetPathsAtIndex(uint32_t idx, ConstString &path,
                                       ConstString &new_path) const {
-  std::lock_guard<std::recursive_mutex> lock(m_mutex);
+  std::lock_guard<std::mutex> lock(m_pairs_mutex);
   if (idx < m_pairs.size()) {
     path = m_pairs[idx].first;
     new_path = m_pairs[idx].second;
@@ -328,9 +357,9 @@ bool PathMappingList::GetPathsAtIndex(uint32_t idx, ConstString &path,
   return false;
 }
 
-uint32_t PathMappingList::FindIndexForPath(llvm::StringRef orig_path) const {
+uint32_t
+PathMappingList::FindIndexForPathNoLock(llvm::StringRef orig_path) const {
   const ConstString path = ConstString(NormalizePath(orig_path));
-  std::lock_guard<std::recursive_mutex> lock(m_mutex);
   const_iterator pos;
   const_iterator begin = m_pairs.begin();
   const_iterator end = m_pairs.end();