Skip to content

Commit 595ea33

Browse files
committed
[NFC][lldb] Speed up lookup of shared modules
By profiling LLDB debugging a Swift application without a dSYM and a large amount of .o files, I identified that querying shared modules was the biggest bottleneck when running "frame variable", and Clang types need to be searched. One of the reasons for that slowness is that the shared module list can grow very large, and the search through it is O(n). To solve this issue, this patch adds a new hashmap to the shared module list whose key is the name of the module, and the value is all the modules that share that name. This should speed up any search where the query contains the module name. rdar://156753350
1 parent 9f7f3d6 commit 595ea33

File tree

1 file changed

+231
-7
lines changed

1 file changed

+231
-7
lines changed

lldb/source/Core/ModuleList.cpp

Lines changed: 231 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -755,11 +755,236 @@ size_t ModuleList::GetIndexForModule(const Module *module) const {
755755
}
756756

757757
namespace {
758+
/// A wrapper around ModuleList for shared modules. Provides fast lookups for
759+
/// file-based ModuleSpec queries.
760+
class SharedModuleList {
761+
public:
762+
/// Finds all the modules matching the module_spec, and adds them to \p
763+
/// matching_module_list.
764+
void FindModules(const ModuleSpec &module_spec,
765+
ModuleList &matching_module_list) const {
766+
std::lock_guard<std::recursive_mutex> guard(GetMutex());
767+
// Try index first for performance - if found, skip expensive full list search
768+
if (FindModulesInIndex(module_spec, matching_module_list))
769+
return;
770+
m_list.FindModules(module_spec, matching_module_list);
771+
// Assertion validates that if we found modules in the list but not the
772+
// index, it's because the module_spec has no filename or the found module
773+
// has a different filename (e.g., when searching by UUID and finding a
774+
// module with an alias)
775+
assert((matching_module_list.IsEmpty() ||
776+
module_spec.GetFileSpec().GetFilename().IsEmpty() ||
777+
module_spec.GetFileSpec().GetFilename() !=
778+
matching_module_list.GetModuleAtIndex(0)
779+
->GetFileSpec()
780+
.GetFilename()) &&
781+
"Search by name not found in SharedModuleList's index");
782+
}
783+
784+
ModuleSP FindModule(const Module *module_ptr) {
785+
if (!module_ptr)
786+
return ModuleSP();
787+
788+
std::lock_guard<std::recursive_mutex> guard(GetMutex());
789+
// Try index first, fallback to full list search
790+
if (ModuleSP result = FindModuleInIndex(module_ptr))
791+
return result;
792+
return m_list.FindModule(module_ptr);
793+
}
794+
795+
// UUID searches bypass index since UUIDs aren't indexed by filename
796+
ModuleSP FindModule(const UUID &uuid) const {
797+
return m_list.FindModule(uuid);
798+
}
799+
800+
void Append(const ModuleSP &module_sp, bool use_notifier) {
801+
if (!module_sp)
802+
return;
803+
std::lock_guard<std::recursive_mutex> guard(GetMutex());
804+
m_list.Append(module_sp, use_notifier);
805+
AddToIndex(module_sp);
806+
}
807+
808+
size_t RemoveOrphans(bool mandatory) {
809+
std::unique_lock<std::recursive_mutex> lock(GetMutex(), std::defer_lock);
810+
if (mandatory) {
811+
lock.lock();
812+
} else {
813+
// Skip orphan removal if mutex unavailable (non-blocking)
814+
if (!lock.try_lock())
815+
return 0;
816+
}
817+
size_t total_count = 0;
818+
size_t run_count;
819+
do {
820+
// Remove indexed orphans first, then remove non-indexed orphans. This
821+
// order is important because the shared count will be different if a
822+
// module if indexed or not.
823+
run_count = RemoveOrphansFromIndexAndList();
824+
run_count += m_list.RemoveOrphans(mandatory);
825+
total_count += run_count;
826+
// Because removing orphans might make new orphans, we must continuously
827+
// remove from both until both operations fail to remove new orphans.
828+
} while (run_count != 0);
829+
830+
return total_count;
831+
}
832+
833+
bool Remove(const ModuleSP &module_sp, bool use_notifier = true) {
834+
if (!module_sp)
835+
return false;
836+
std::lock_guard<std::recursive_mutex> guard(GetMutex());
837+
RemoveFromIndex(module_sp.get());
838+
bool success = m_list.Remove(module_sp, use_notifier);
839+
return success;
840+
}
841+
842+
void ReplaceEquivalent(const ModuleSP &module_sp,
843+
llvm::SmallVectorImpl<lldb::ModuleSP> *old_modules) {
844+
std::lock_guard<std::recursive_mutex> guard(GetMutex());
845+
m_list.ReplaceEquivalent(module_sp, old_modules);
846+
ReplaceEquivalentInIndex(module_sp);
847+
}
848+
849+
bool RemoveIfOrphaned(const Module *module_ptr) {
850+
std::lock_guard<std::recursive_mutex> guard(GetMutex());
851+
RemoveFromIndex(module_ptr, /*if_orphaned =*/true);
852+
bool result = m_list.RemoveIfOrphaned(module_ptr);
853+
return result;
854+
}
855+
856+
std::recursive_mutex &GetMutex() const { return m_list.GetMutex(); }
857+
858+
private:
859+
ModuleSP FindModuleInIndex(const Module *module_ptr) {
860+
if (!module_ptr->GetFileSpec().GetFilename())
861+
return ModuleSP();
862+
ConstString name = module_ptr->GetFileSpec().GetFilename();
863+
auto it = m_index.find(name);
864+
if (it != m_index.end()) {
865+
auto &vector = it->getSecond();
866+
for (auto &module_sp : vector)
867+
if (module_sp.get() == module_ptr)
868+
return module_sp;
869+
}
870+
return ModuleSP();
871+
}
872+
873+
bool FindModulesInIndex(const ModuleSpec &module_spec,
874+
ModuleList &matching_module_list) const {
875+
auto it = m_index.find(module_spec.GetFileSpec().GetFilename());
876+
if (it == m_index.end())
877+
return false;
878+
auto vector = it->getSecond();
879+
bool found = false;
880+
for (auto &module_sp : vector) {
881+
if (module_sp->MatchesModuleSpec(module_spec)) {
882+
matching_module_list.Append(module_sp);
883+
found = true;
884+
}
885+
}
886+
return found;
887+
}
888+
889+
void AddToIndex(const ModuleSP &module_sp) {
890+
auto name = module_sp->GetFileSpec().GetFilename();
891+
if (name.IsEmpty())
892+
return;
893+
auto &vec = m_index[name];
894+
vec.push_back(module_sp);
895+
}
896+
897+
void RemoveFromIndex(const Module *module_ptr, bool if_orphaned = false) {
898+
auto name = module_ptr->GetFileSpec().GetFilename();
899+
auto it = m_index.find(name);
900+
if (it == m_index.end())
901+
return;
902+
auto &vec = it->getSecond();
903+
for (auto *it = vec.begin(); it != vec.end(); ++it) {
904+
if (it->get() == module_ptr) {
905+
// use_count == 2 means only held by index and list (orphaned)
906+
if (!if_orphaned || it->use_count() == 2)
907+
vec.erase(it);
908+
break;
909+
}
910+
}
911+
}
912+
913+
void ReplaceEquivalentInIndex(const ModuleSP &module_sp) {
914+
RemoveEquivalentModulesFromIndex(module_sp);
915+
AddToIndex(module_sp);
916+
}
917+
918+
void RemoveEquivalentModulesFromIndex(const ModuleSP &module_sp) {
919+
auto name = module_sp->GetFileSpec().GetFilename();
920+
if (name.IsEmpty())
921+
return;
922+
923+
auto it = m_index.find(name);
924+
if (it == m_index.end())
925+
return;
926+
927+
// First remove any equivalent modules. Equivalent modules are modules
928+
// whose path, platform path and architecture match.
929+
ModuleSpec equivalent_module_spec(module_sp->GetFileSpec(),
930+
module_sp->GetArchitecture());
931+
equivalent_module_spec.GetPlatformFileSpec() =
932+
module_sp->GetPlatformFileSpec();
933+
934+
auto &vec = it->getSecond();
935+
// Iterate backwards to minimize element shifting during removal
936+
for (int i = vec.size() - 1; i >= 0; --i) {
937+
auto *it = vec.begin() + i;
938+
if ((*it)->MatchesModuleSpec(equivalent_module_spec))
939+
vec.erase(it);
940+
}
941+
}
942+
943+
/// Remove orphans from both the index and list, if orphaned.
944+
///
945+
/// This assumes that the mutex is locked.
946+
int RemoveOrphansFromIndexAndList() {
947+
// Modules might hold shared pointers to other modules, so removing one
948+
// module might make other modules orphans. Keep removing modules until
949+
// there are no further modules that can be removed.
950+
bool made_progress = true;
951+
int remove_count = 0;
952+
while (made_progress) {
953+
made_progress = false;
954+
for (auto &[name, vec] : m_index) {
955+
if (vec.empty())
956+
continue;
957+
ModuleList to_remove;
958+
// Iterate backwards to minimize element shifting during removal
959+
for (int i = vec.size() - 1; i >= 0; --i) {
960+
ModuleSP module = vec[i];
961+
// use_count == 3: index + list + local variable = orphaned
962+
if (module.use_count() == 3) {
963+
to_remove.Append(module);
964+
vec.erase(vec.begin() + i);
965+
remove_count += 1;
966+
made_progress = true;
967+
}
968+
}
969+
m_list.Remove(to_remove);
970+
}
971+
}
972+
return remove_count;
973+
}
974+
975+
ModuleList m_list;
976+
977+
/// A hash map from a module's filename to all the modules that share that
978+
/// filename, for fast module lookups by name
979+
llvm::DenseMap<ConstString, llvm::SmallVector<ModuleSP, 1>> m_index;
980+
};
981+
758982
struct SharedModuleListInfo {
759-
ModuleList module_list;
983+
SharedModuleList module_list;
760984
ModuleListProperties module_list_properties;
761985
};
762-
}
986+
} // namespace
987+
763988
static SharedModuleListInfo &GetSharedModuleListInfo()
764989
{
765990
static SharedModuleListInfo *g_shared_module_list_info = nullptr;
@@ -774,7 +999,7 @@ static SharedModuleListInfo &GetSharedModuleListInfo()
774999
return *g_shared_module_list_info;
7751000
}
7761001

777-
static ModuleList &GetSharedModuleList() {
1002+
static SharedModuleList &GetSharedModuleList() {
7781003
return GetSharedModuleListInfo().module_list;
7791004
}
7801005

@@ -784,7 +1009,7 @@ ModuleListProperties &ModuleList::GetGlobalModuleListProperties() {
7841009

7851010
bool ModuleList::ModuleIsInCache(const Module *module_ptr) {
7861011
if (module_ptr) {
787-
ModuleList &shared_module_list = GetSharedModuleList();
1012+
SharedModuleList &shared_module_list = GetSharedModuleList();
7881013
return shared_module_list.FindModule(module_ptr).get() != nullptr;
7891014
}
7901015
return false;
@@ -808,9 +1033,8 @@ ModuleList::GetSharedModule(const ModuleSpec &module_spec, ModuleSP &module_sp,
8081033
const FileSpecList *module_search_paths_ptr,
8091034
llvm::SmallVectorImpl<lldb::ModuleSP> *old_modules,
8101035
bool *did_create_ptr, bool always_create) {
811-
ModuleList &shared_module_list = GetSharedModuleList();
812-
std::lock_guard<std::recursive_mutex> guard(
813-
shared_module_list.m_modules_mutex);
1036+
SharedModuleList &shared_module_list = GetSharedModuleList();
1037+
std::lock_guard<std::recursive_mutex> guard(shared_module_list.GetMutex());
8141038
char path[PATH_MAX];
8151039

8161040
Status error;

0 commit comments

Comments
 (0)