Author: Jonas Devlieghere Date: 2024-11-02T09:38:10-07:00 New Revision: b360dfd5031e76c97257ef1b3e90385bf297e8ab
URL: https://github.com/llvm/llvm-project/commit/b360dfd5031e76c97257ef1b3e90385bf297e8ab DIFF: https://github.com/llvm/llvm-project/commit/b360dfd5031e76c97257ef1b3e90385bf297e8ab.diff LOG: [lldb] Create dependent modules in parallel (#114507) Create dependent modules in parallel in Target::SetExecutableModule. This change was inspired by #110646 which takes the same approach when attaching. Jason suggested we could use the same approach when you create a target in LLDB. I used Slack for benchmarking, which loads 902 images. ``` Benchmark 1: ./bin/lldb /Applications/Slack.app/Contents/MacOS/Slack Time (mean ± σ): 1.225 s ± 0.003 s [User: 3.977 s, System: 1.521 s] Range (min … max): 1.220 s … 1.229 s 10 runs Benchmark 2: ./bin/lldb /Applications/Slack.app/Contents/MacOS/Slack Time (mean ± σ): 3.253 s ± 0.037 s [User: 3.013 s, System: 0.248 s] Range (min … max): 3.211 s … 3.310 s 10 runs ``` We see about a 2x speedup, which matches what Jason saw for the attach scenario. I also ran this under TSan to confirm this doesn't introduce any races or deadlocks. Added: Modified: lldb/source/Target/Target.cpp Removed: ################################################################################ diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp index 199efae8a728cc..8cd3fa8af6bae1 100644 --- a/lldb/source/Target/Target.cpp +++ b/lldb/source/Target/Target.cpp @@ -68,6 +68,7 @@ #include "llvm/ADT/ScopeExit.h" #include "llvm/ADT/SetVector.h" +#include "llvm/Support/ThreadPool.h" #include <memory> #include <mutex> @@ -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) { @@ -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, @@ -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, + dependent_files.GetFileSpecAtIndex(i)); } + task_group.wait(); } ModulesDidLoad(added_modules); } _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
