Skip to content

Commit bf53a9c

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 bf53a9c

File tree

1 file changed

+232
-7
lines changed

1 file changed

+232
-7
lines changed

lldb/source/Core/ModuleList.cpp

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

777-
static ModuleList &GetSharedModuleList() {
1003+
static SharedModuleList &GetSharedModuleList() {
7781004
return GetSharedModuleListInfo().module_list;
7791005
}
7801006

@@ -784,7 +1010,7 @@ ModuleListProperties &ModuleList::GetGlobalModuleListProperties() {
7841010

7851011
bool ModuleList::ModuleIsInCache(const Module *module_ptr) {
7861012
if (module_ptr) {
787-
ModuleList &shared_module_list = GetSharedModuleList();
1013+
SharedModuleList &shared_module_list = GetSharedModuleList();
7881014
return shared_module_list.FindModule(module_ptr).get() != nullptr;
7891015
}
7901016
return false;
@@ -808,9 +1034,8 @@ ModuleList::GetSharedModule(const ModuleSpec &module_spec, ModuleSP &module_sp,
8081034
const FileSpecList *module_search_paths_ptr,
8091035
llvm::SmallVectorImpl<lldb::ModuleSP> *old_modules,
8101036
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);
1037+
SharedModuleList &shared_module_list = GetSharedModuleList();
1038+
std::lock_guard<std::recursive_mutex> guard(shared_module_list.GetMutex());
8141039
char path[PATH_MAX];
8151040

8161041
Status error;

0 commit comments

Comments
 (0)