https://github.com/augusto2112 created
https://github.com/llvm/llvm-project/pull/152582
This reverts commit 229d86026fa0e5d9412a0d5004532f0d9733aac6.
>From 2c9d14251c8a5bd2354d2747d44b8bb7db30c3b1 Mon Sep 17 00:00:00 2001
From: Augusto Noronha <anoro...@apple.com>
Date: Thu, 7 Aug 2025 12:44:40 -0700
Subject: [PATCH] Revert "[NFC][lldb] Speed up lookup of shared modules
(#152054)"
This reverts commit 229d86026fa0e5d9412a0d5004532f0d9733aac6.
---
lldb/source/Core/ModuleList.cpp | 242 +-------------------------------
1 file changed, 7 insertions(+), 235 deletions(-)
diff --git a/lldb/source/Core/ModuleList.cpp b/lldb/source/Core/ModuleList.cpp
index 34caa474c1e32..d5ddc2b249e56 100644
--- a/lldb/source/Core/ModuleList.cpp
+++ b/lldb/source/Core/ModuleList.cpp
@@ -755,240 +755,11 @@ size_t ModuleList::GetIndexForModule(const Module
*module) const {
}
namespace {
-/// A wrapper around ModuleList for shared modules. Provides fast lookups for
-/// file-based ModuleSpec queries.
-class SharedModuleList {
-public:
- /// Finds all the modules matching the module_spec, and adds them to \p
- /// matching_module_list.
- void FindModules(const ModuleSpec &module_spec,
- ModuleList &matching_module_list) const {
- std::lock_guard<std::recursive_mutex> guard(GetMutex());
- // Try map first for performance - if found, skip expensive full list
- // search
- if (FindModulesInMap(module_spec, matching_module_list))
- return;
- m_list.FindModules(module_spec, matching_module_list);
- // Assert that modules were found in the list but not the map, it's
- // because the module_spec has no filename or the found module has a
- // different filename. For example, when searching by UUID and finding a
- // module with an alias.
- assert((matching_module_list.IsEmpty() ||
- module_spec.GetFileSpec().GetFilename().IsEmpty() ||
- module_spec.GetFileSpec().GetFilename() !=
- matching_module_list.GetModuleAtIndex(0)
- ->GetFileSpec()
- .GetFilename()) &&
- "Search by name not found in SharedModuleList's map");
- }
-
- ModuleSP FindModule(const Module *module_ptr) {
- if (!module_ptr)
- return ModuleSP();
-
- std::lock_guard<std::recursive_mutex> guard(GetMutex());
- if (ModuleSP result = FindModuleInMap(module_ptr))
- return result;
- return m_list.FindModule(module_ptr);
- }
-
- // UUID searches bypass map since UUIDs aren't indexed by filename.
- ModuleSP FindModule(const UUID &uuid) const {
- return m_list.FindModule(uuid);
- }
-
- void Append(const ModuleSP &module_sp, bool use_notifier) {
- if (!module_sp)
- return;
- std::lock_guard<std::recursive_mutex> guard(GetMutex());
- m_list.Append(module_sp, use_notifier);
- AddToMap(module_sp);
- }
-
- size_t RemoveOrphans(bool mandatory) {
- std::unique_lock<std::recursive_mutex> lock(GetMutex(), std::defer_lock);
- if (mandatory) {
- lock.lock();
- } else {
- if (!lock.try_lock())
- return 0;
- }
- size_t total_count = 0;
- size_t run_count;
- do {
- // Remove indexed orphans first, then remove non-indexed orphans. This
- // order is important because the shared count will be different if a
- // module is indexed or not.
- run_count = RemoveOrphansFromMapAndList();
- run_count += m_list.RemoveOrphans(mandatory);
- total_count += run_count;
- // Because removing orphans might make new orphans, remove from both
- // containers until a fixed-point is reached.
- } while (run_count != 0);
-
- return total_count;
- }
-
- bool Remove(const ModuleSP &module_sp, bool use_notifier = true) {
- if (!module_sp)
- return false;
- std::lock_guard<std::recursive_mutex> guard(GetMutex());
- RemoveFromMap(module_sp.get());
- return m_list.Remove(module_sp, use_notifier);
- }
-
- void ReplaceEquivalent(const ModuleSP &module_sp,
- llvm::SmallVectorImpl<lldb::ModuleSP> *old_modules) {
- std::lock_guard<std::recursive_mutex> guard(GetMutex());
- m_list.ReplaceEquivalent(module_sp, old_modules);
- ReplaceEquivalentInMap(module_sp);
- }
-
- bool RemoveIfOrphaned(const Module *module_ptr) {
- std::lock_guard<std::recursive_mutex> guard(GetMutex());
- RemoveFromMap(module_ptr, /*if_orphaned =*/true);
- return m_list.RemoveIfOrphaned(module_ptr);
- }
-
- std::recursive_mutex &GetMutex() const { return m_list.GetMutex(); }
-
-private:
- ModuleSP FindModuleInMap(const Module *module_ptr) {
- if (!module_ptr->GetFileSpec().GetFilename())
- return ModuleSP();
- ConstString name = module_ptr->GetFileSpec().GetFilename();
- auto it = m_name_to_modules.find(name);
- if (it == m_name_to_modules.end())
- return ModuleSP();
- const llvm::SmallVectorImpl<ModuleSP> &vector = it->second;
- for (auto &module_sp : vector) {
- if (module_sp.get() == module_ptr)
- return module_sp;
- }
- return ModuleSP();
- }
-
- bool FindModulesInMap(const ModuleSpec &module_spec,
- ModuleList &matching_module_list) const {
- auto it = m_name_to_modules.find(module_spec.GetFileSpec().GetFilename());
- if (it == m_name_to_modules.end())
- return false;
- const llvm::SmallVectorImpl<ModuleSP> &vector = it->second;
- bool found = false;
- for (auto &module_sp : vector) {
- if (module_sp->MatchesModuleSpec(module_spec)) {
- matching_module_list.Append(module_sp);
- found = true;
- }
- }
- return found;
- }
-
- void AddToMap(const ModuleSP &module_sp) {
- ConstString name = module_sp->GetFileSpec().GetFilename();
- if (name.IsEmpty())
- return;
- llvm::SmallVectorImpl<ModuleSP> &vec = m_name_to_modules[name];
- vec.push_back(module_sp);
- }
-
- void RemoveFromMap(const Module *module_ptr, bool if_orphaned = false) {
- ConstString name = module_ptr->GetFileSpec().GetFilename();
- if (m_name_to_modules.contains(name))
- return;
- llvm::SmallVectorImpl<ModuleSP> &vec = m_name_to_modules[name];
- for (auto *it = vec.begin(); it != vec.end(); ++it) {
- if (it->get() == module_ptr) {
- // use_count == 2 means only held by map and list (orphaned)
- if (!if_orphaned || it->use_count() == 2)
- vec.erase(it);
- break;
- }
- }
- }
-
- void ReplaceEquivalentInMap(const ModuleSP &module_sp) {
- RemoveEquivalentModulesFromMap(module_sp);
- AddToMap(module_sp);
- }
-
- void RemoveEquivalentModulesFromMap(const ModuleSP &module_sp) {
- ConstString name = module_sp->GetFileSpec().GetFilename();
- if (name.IsEmpty())
- return;
-
- auto it = m_name_to_modules.find(name);
- if (it == m_name_to_modules.end())
- return;
-
- // First remove any equivalent modules. Equivalent modules are modules
- // whose path, platform path and architecture match.
- ModuleSpec equivalent_module_spec(module_sp->GetFileSpec(),
- module_sp->GetArchitecture());
- equivalent_module_spec.GetPlatformFileSpec() =
- module_sp->GetPlatformFileSpec();
-
- llvm::SmallVectorImpl<ModuleSP> &vec = it->second;
- llvm::erase_if(vec, [&equivalent_module_spec](ModuleSP &element) {
- return element->MatchesModuleSpec(equivalent_module_spec);
- });
- }
-
- /// Remove orphans from the vector.
- ///
- /// Returns the removed orphans.
- ModuleList RemoveOrphansFromVector(llvm::SmallVectorImpl<ModuleSP> &vec) {
- ModuleList to_remove;
- for (int i = vec.size() - 1; i >= 0; --i) {
- ModuleSP module = vec[i];
- long kUseCountOrphaned = 2;
- long kUseCountLocalVariable = 1;
- // use_count == 3: map + list + local variable = orphaned.
- if (module.use_count() == kUseCountOrphaned + kUseCountLocalVariable) {
- to_remove.Append(module);
- vec.erase(vec.begin() + i);
- }
- }
- return to_remove;
- }
-
- /// Remove orphans that exist in both the map and list. This does not remove
- /// any orphans that exist exclusively on the list.
- ///
- /// This assumes that the mutex is locked.
- int RemoveOrphansFromMapAndList() {
- // Modules might hold shared pointers to other modules, so removing one
- // module might orphan other modules. Keep removing modules until
- // there are no further modules that can be removed.
- bool made_progress = true;
- int remove_count = 0;
- while (made_progress) {
- made_progress = false;
- for (auto &[name, vec] : m_name_to_modules) {
- if (vec.empty())
- continue;
- ModuleList to_remove = RemoveOrphansFromVector(vec);
- remove_count += to_remove.GetSize();
- made_progress = !to_remove.IsEmpty();
- m_list.Remove(to_remove);
- }
- }
- return remove_count;
- }
-
- ModuleList m_list;
-
- /// A hash map from a module's filename to all the modules that share that
- /// filename, for fast module lookups by name.
- llvm::DenseMap<ConstString, llvm::SmallVector<ModuleSP, 1>>
m_name_to_modules;
-};
-
struct SharedModuleListInfo {
- SharedModuleList module_list;
+ ModuleList module_list;
ModuleListProperties module_list_properties;
};
-} // namespace
-
+}
static SharedModuleListInfo &GetSharedModuleListInfo()
{
static SharedModuleListInfo *g_shared_module_list_info = nullptr;
@@ -1003,7 +774,7 @@ static SharedModuleListInfo &GetSharedModuleListInfo()
return *g_shared_module_list_info;
}
-static SharedModuleList &GetSharedModuleList() {
+static ModuleList &GetSharedModuleList() {
return GetSharedModuleListInfo().module_list;
}
@@ -1013,7 +784,7 @@ ModuleListProperties
&ModuleList::GetGlobalModuleListProperties() {
bool ModuleList::ModuleIsInCache(const Module *module_ptr) {
if (module_ptr) {
- SharedModuleList &shared_module_list = GetSharedModuleList();
+ ModuleList &shared_module_list = GetSharedModuleList();
return shared_module_list.FindModule(module_ptr).get() != nullptr;
}
return false;
@@ -1037,8 +808,9 @@ ModuleList::GetSharedModule(const ModuleSpec &module_spec,
ModuleSP &module_sp,
const FileSpecList *module_search_paths_ptr,
llvm::SmallVectorImpl<lldb::ModuleSP> *old_modules,
bool *did_create_ptr, bool always_create) {
- SharedModuleList &shared_module_list = GetSharedModuleList();
- std::lock_guard<std::recursive_mutex> guard(shared_module_list.GetMutex());
+ ModuleList &shared_module_list = GetSharedModuleList();
+ std::lock_guard<std::recursive_mutex> guard(
+ shared_module_list.m_modules_mutex);
char path[PATH_MAX];
Status error;
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
