Skip to content

Commit 2c9d142

Browse files
committed
Revert "[NFC][lldb] Speed up lookup of shared modules (llvm#152054)"
This reverts commit 229d860.
1 parent ca52d9b commit 2c9d142

File tree

1 file changed

+7
-235
lines changed

1 file changed

+7
-235
lines changed

lldb/source/Core/ModuleList.cpp

Lines changed: 7 additions & 235 deletions
Original file line numberDiff line numberDiff line change
@@ -755,240 +755,11 @@ 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 map first for performance - if found, skip expensive full list
768-
// search
769-
if (FindModulesInMap(module_spec, matching_module_list))
770-
return;
771-
m_list.FindModules(module_spec, matching_module_list);
772-
// Assert that modules were found in the list but not the map, it's
773-
// because the module_spec has no filename or the found module has a
774-
// different filename. For example, 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 map");
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-
if (ModuleSP result = FindModuleInMap(module_ptr))
791-
return result;
792-
return m_list.FindModule(module_ptr);
793-
}
794-
795-
// UUID searches bypass map 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-
AddToMap(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-
if (!lock.try_lock())
814-
return 0;
815-
}
816-
size_t total_count = 0;
817-
size_t run_count;
818-
do {
819-
// Remove indexed orphans first, then remove non-indexed orphans. This
820-
// order is important because the shared count will be different if a
821-
// module is indexed or not.
822-
run_count = RemoveOrphansFromMapAndList();
823-
run_count += m_list.RemoveOrphans(mandatory);
824-
total_count += run_count;
825-
// Because removing orphans might make new orphans, remove from both
826-
// containers until a fixed-point is reached.
827-
} while (run_count != 0);
828-
829-
return total_count;
830-
}
831-
832-
bool Remove(const ModuleSP &module_sp, bool use_notifier = true) {
833-
if (!module_sp)
834-
return false;
835-
std::lock_guard<std::recursive_mutex> guard(GetMutex());
836-
RemoveFromMap(module_sp.get());
837-
return m_list.Remove(module_sp, use_notifier);
838-
}
839-
840-
void ReplaceEquivalent(const ModuleSP &module_sp,
841-
llvm::SmallVectorImpl<lldb::ModuleSP> *old_modules) {
842-
std::lock_guard<std::recursive_mutex> guard(GetMutex());
843-
m_list.ReplaceEquivalent(module_sp, old_modules);
844-
ReplaceEquivalentInMap(module_sp);
845-
}
846-
847-
bool RemoveIfOrphaned(const Module *module_ptr) {
848-
std::lock_guard<std::recursive_mutex> guard(GetMutex());
849-
RemoveFromMap(module_ptr, /*if_orphaned =*/true);
850-
return m_list.RemoveIfOrphaned(module_ptr);
851-
}
852-
853-
std::recursive_mutex &GetMutex() const { return m_list.GetMutex(); }
854-
855-
private:
856-
ModuleSP FindModuleInMap(const Module *module_ptr) {
857-
if (!module_ptr->GetFileSpec().GetFilename())
858-
return ModuleSP();
859-
ConstString name = module_ptr->GetFileSpec().GetFilename();
860-
auto it = m_name_to_modules.find(name);
861-
if (it == m_name_to_modules.end())
862-
return ModuleSP();
863-
const llvm::SmallVectorImpl<ModuleSP> &vector = it->second;
864-
for (auto &module_sp : vector) {
865-
if (module_sp.get() == module_ptr)
866-
return module_sp;
867-
}
868-
return ModuleSP();
869-
}
870-
871-
bool FindModulesInMap(const ModuleSpec &module_spec,
872-
ModuleList &matching_module_list) const {
873-
auto it = m_name_to_modules.find(module_spec.GetFileSpec().GetFilename());
874-
if (it == m_name_to_modules.end())
875-
return false;
876-
const llvm::SmallVectorImpl<ModuleSP> &vector = it->second;
877-
bool found = false;
878-
for (auto &module_sp : vector) {
879-
if (module_sp->MatchesModuleSpec(module_spec)) {
880-
matching_module_list.Append(module_sp);
881-
found = true;
882-
}
883-
}
884-
return found;
885-
}
886-
887-
void AddToMap(const ModuleSP &module_sp) {
888-
ConstString name = module_sp->GetFileSpec().GetFilename();
889-
if (name.IsEmpty())
890-
return;
891-
llvm::SmallVectorImpl<ModuleSP> &vec = m_name_to_modules[name];
892-
vec.push_back(module_sp);
893-
}
894-
895-
void RemoveFromMap(const Module *module_ptr, bool if_orphaned = false) {
896-
ConstString name = module_ptr->GetFileSpec().GetFilename();
897-
if (m_name_to_modules.contains(name))
898-
return;
899-
llvm::SmallVectorImpl<ModuleSP> &vec = m_name_to_modules[name];
900-
for (auto *it = vec.begin(); it != vec.end(); ++it) {
901-
if (it->get() == module_ptr) {
902-
// use_count == 2 means only held by map and list (orphaned)
903-
if (!if_orphaned || it->use_count() == 2)
904-
vec.erase(it);
905-
break;
906-
}
907-
}
908-
}
909-
910-
void ReplaceEquivalentInMap(const ModuleSP &module_sp) {
911-
RemoveEquivalentModulesFromMap(module_sp);
912-
AddToMap(module_sp);
913-
}
914-
915-
void RemoveEquivalentModulesFromMap(const ModuleSP &module_sp) {
916-
ConstString name = module_sp->GetFileSpec().GetFilename();
917-
if (name.IsEmpty())
918-
return;
919-
920-
auto it = m_name_to_modules.find(name);
921-
if (it == m_name_to_modules.end())
922-
return;
923-
924-
// First remove any equivalent modules. Equivalent modules are modules
925-
// whose path, platform path and architecture match.
926-
ModuleSpec equivalent_module_spec(module_sp->GetFileSpec(),
927-
module_sp->GetArchitecture());
928-
equivalent_module_spec.GetPlatformFileSpec() =
929-
module_sp->GetPlatformFileSpec();
930-
931-
llvm::SmallVectorImpl<ModuleSP> &vec = it->second;
932-
llvm::erase_if(vec, [&equivalent_module_spec](ModuleSP &element) {
933-
return element->MatchesModuleSpec(equivalent_module_spec);
934-
});
935-
}
936-
937-
/// Remove orphans from the vector.
938-
///
939-
/// Returns the removed orphans.
940-
ModuleList RemoveOrphansFromVector(llvm::SmallVectorImpl<ModuleSP> &vec) {
941-
ModuleList to_remove;
942-
for (int i = vec.size() - 1; i >= 0; --i) {
943-
ModuleSP module = vec[i];
944-
long kUseCountOrphaned = 2;
945-
long kUseCountLocalVariable = 1;
946-
// use_count == 3: map + list + local variable = orphaned.
947-
if (module.use_count() == kUseCountOrphaned + kUseCountLocalVariable) {
948-
to_remove.Append(module);
949-
vec.erase(vec.begin() + i);
950-
}
951-
}
952-
return to_remove;
953-
}
954-
955-
/// Remove orphans that exist in both the map and list. This does not remove
956-
/// any orphans that exist exclusively on the list.
957-
///
958-
/// This assumes that the mutex is locked.
959-
int RemoveOrphansFromMapAndList() {
960-
// Modules might hold shared pointers to other modules, so removing one
961-
// module might orphan other modules. Keep removing modules until
962-
// there are no further modules that can be removed.
963-
bool made_progress = true;
964-
int remove_count = 0;
965-
while (made_progress) {
966-
made_progress = false;
967-
for (auto &[name, vec] : m_name_to_modules) {
968-
if (vec.empty())
969-
continue;
970-
ModuleList to_remove = RemoveOrphansFromVector(vec);
971-
remove_count += to_remove.GetSize();
972-
made_progress = !to_remove.IsEmpty();
973-
m_list.Remove(to_remove);
974-
}
975-
}
976-
return remove_count;
977-
}
978-
979-
ModuleList m_list;
980-
981-
/// A hash map from a module's filename to all the modules that share that
982-
/// filename, for fast module lookups by name.
983-
llvm::DenseMap<ConstString, llvm::SmallVector<ModuleSP, 1>> m_name_to_modules;
984-
};
985-
986758
struct SharedModuleListInfo {
987-
SharedModuleList module_list;
759+
ModuleList module_list;
988760
ModuleListProperties module_list_properties;
989761
};
990-
} // namespace
991-
762+
}
992763
static SharedModuleListInfo &GetSharedModuleListInfo()
993764
{
994765
static SharedModuleListInfo *g_shared_module_list_info = nullptr;
@@ -1003,7 +774,7 @@ static SharedModuleListInfo &GetSharedModuleListInfo()
1003774
return *g_shared_module_list_info;
1004775
}
1005776

1006-
static SharedModuleList &GetSharedModuleList() {
777+
static ModuleList &GetSharedModuleList() {
1007778
return GetSharedModuleListInfo().module_list;
1008779
}
1009780

@@ -1013,7 +784,7 @@ ModuleListProperties &ModuleList::GetGlobalModuleListProperties() {
1013784

1014785
bool ModuleList::ModuleIsInCache(const Module *module_ptr) {
1015786
if (module_ptr) {
1016-
SharedModuleList &shared_module_list = GetSharedModuleList();
787+
ModuleList &shared_module_list = GetSharedModuleList();
1017788
return shared_module_list.FindModule(module_ptr).get() != nullptr;
1018789
}
1019790
return false;
@@ -1037,8 +808,9 @@ ModuleList::GetSharedModule(const ModuleSpec &module_spec, ModuleSP &module_sp,
1037808
const FileSpecList *module_search_paths_ptr,
1038809
llvm::SmallVectorImpl<lldb::ModuleSP> *old_modules,
1039810
bool *did_create_ptr, bool always_create) {
1040-
SharedModuleList &shared_module_list = GetSharedModuleList();
1041-
std::lock_guard<std::recursive_mutex> guard(shared_module_list.GetMutex());
811+
ModuleList &shared_module_list = GetSharedModuleList();
812+
std::lock_guard<std::recursive_mutex> guard(
813+
shared_module_list.m_modules_mutex);
1042814
char path[PATH_MAX];
1043815

1044816
Status error;

0 commit comments

Comments
 (0)