Skip to content

[lldb] Create dependent modules in parallel #114507

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Nov 2, 2024
Merged

[lldb] Create dependent modules in parallel #114507

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9a269fa
[lldb] Create dependent modules in parallel
JDevlieghere Nov 1, 2024
0bba5f7
Fix formatting
JDevlieghere Nov 1, 2024
535c27d
Don't lock for the duration fo GetDependentModules
JDevlieghere Nov 1, 2024
37243c1
Merge branch 'main' into parallel-dependents
JDevlieghere Nov 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 49 additions & 6 deletions lldb/source/Target/Target.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@

#include "llvm/ADT/ScopeExit.h"
#include "llvm/ADT/SetVector.h"
#include "llvm/Support/ThreadPool.h"

#include <memory>
#include <mutex>
Expand Down Expand Up @@ -1575,7 +1576,6 @@ void Target::SetExecutableModule(ModuleSP &executable_sp,
m_arch.GetSpec().GetTriple().getTriple());
}

FileSpecList dependent_files;
ObjectFile *executable_objfile = executable_sp->GetObjectFile();
bool load_dependents = true;
switch (load_dependent_files) {
Expand All @@ -1591,10 +1591,14 @@ void Target::SetExecutableModule(ModuleSP &executable_sp,
}

if (executable_objfile && load_dependents) {
// FileSpecList is not thread safe and needs to be synchronized.
FileSpecList dependent_files;
std::mutex dependent_files_mutex;

// ModuleList is thread safe.
ModuleList added_modules;
executable_objfile->GetDependentModules(dependent_files);
for (uint32_t i = 0; i < dependent_files.GetSize(); i++) {
FileSpec dependent_file_spec(dependent_files.GetFileSpecAtIndex(i));

auto GetDependentModules = [&](FileSpec dependent_file_spec) {
FileSpec platform_dependent_file_spec;
if (m_platform_sp)
m_platform_sp->GetFileWithUUID(dependent_file_spec, nullptr,
Expand All @@ -1608,9 +1612,48 @@ void Target::SetExecutableModule(ModuleSP &executable_sp,
if (image_module_sp) {
added_modules.AppendIfNeeded(image_module_sp, false);
ObjectFile *objfile = image_module_sp->GetObjectFile();
if (objfile)
objfile->GetDependentModules(dependent_files);
if (objfile) {
// Create a local copy of the dependent file list so we don't have
// to lock for the whole duration of GetDependentModules.
FileSpecList dependent_files_copy;
{
std::lock_guard<std::mutex> guard(dependent_files_mutex);
dependent_files_copy = dependent_files;
}

// Remember the size of the local copy so we can append only the
// modules that have been added by GetDependentModules.
const size_t previous_dependent_files =
dependent_files_copy.GetSize();

objfile->GetDependentModules(dependent_files_copy);

{
std::lock_guard<std::mutex> guard(dependent_files_mutex);
for (size_t i = previous_dependent_files;
i < dependent_files_copy.GetSize(); ++i)
dependent_files.AppendIfUnique(
dependent_files_copy.GetFileSpecAtIndex(i));
}
}
}
};

executable_objfile->GetDependentModules(dependent_files);

llvm::ThreadPoolTaskGroup task_group(Debugger::GetThreadPool());
for (uint32_t i = 0; i < dependent_files.GetSize(); i++) {
// Process all currently known dependencies in parallel in the innermost
// loop. This may create newly discovered dependencies to be appended to
// dependent_files. We'll deal with these files during the next
// iteration of the outermost loop.
{
std::lock_guard<std::mutex> guard(dependent_files_mutex);
for (; i < dependent_files.GetSize(); i++)
task_group.async(GetDependentModules,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

async inside a lock_guard looks dangerous. I assume you're sure this won't recurse into this function? :-)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't, but if that becomes a problem we can do the same trick that Jason suggested and iterate over a copy of the list here. For now that seems unnecessary but hopefully someone will see this comment if that ever changes :-)

dependent_files.GetFileSpecAtIndex(i));
}
task_group.wait();
}
ModulesDidLoad(added_modules);
}
Expand Down
Loading