[Lldb-commits] [lldb] Parallelize module loading in POSIX dyld code (PR #130912)
via lldb-commits
lldb-commits at lists.llvm.org
Thu Mar 13 14:39:22 PDT 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-lldb
Author: Tom Yang (zhyty)
<details>
<summary>Changes</summary>
This patch improves LLDB launch time on Linux machines for **preload scenarios**, particularly for executables with a lot of shared library dependencies (or modules). Specifically:
* Launching a binary with `target.preload-symbols = true`
* Attaching to a process with `target.preload-symbols = true`.
It's completely controlled by a new flag added in the first commit `plugin.dynamic-loader.posix-dyld.parallel-module-load`, which *defaults to false*.
Some rough numbers to showcase perf improvement, run on a very beefy machine:
* Executable with ~5600 modules: baseline 45s, improvement 15s
* Executable with ~3800 modules: baseline 25s, improvement 10s
* Executable with ~6650 modules: baseline 67s, improvement 20s
* Executable with ~12500 modules: baseline 185s, improvement 85s
* Executable with ~14700 modules: baseline 235s, improvement 120s
A lot of targets we deal with have a *ton* of modules, and unfortunately we're unable to convince other folks to reduce the number of modules, so performance improvements like this can be very impactful for user experience.
This patch achieves the performance improvement by parallelizing `DynamicLoaderPOSIXDYLD::RefreshModules` for the launch scenario, and `DynamicLoaderPOSIXDYLD::LoadAllCurrentModules` for the attach scenario. The commits have some context on their specific changes as well -- hopefully this helps the review.
# More context on implementation
We discovered the bottlenecks by via `perf record -g -p <lldb's pid>` on a Linux machine. With an executable known to have 1000s of shared library dependencies, I ran
```
(lldb) b main
(lldb) r
# taking a while
```
and showed the resulting perf trace (snippet shown)
```
Samples: 85K of event 'cycles:P', Event count (approx.): 54615855812
Children Self Command Shared Object Symbol
- 93.54% 0.00% intern-state libc.so.6 [.] clone3
clone3
start_thread
lldb_private::HostNativeThreadBase::ThreadCreateTrampoline(void*) r
std::_Function_handler<void* (), lldb_private::Process::StartPrivateStateThread(bool)::$_0>::_M_invoke(std::_Any_data const&)
lldb_private::Process::RunPrivateStateThread(bool) n
- lldb_private::Process::HandlePrivateEvent(std::shared_ptr<lldb_private::Event>&)
- 93.54% lldb_private::Process::ShouldBroadcastEvent(lldb_private::Event*)
- 93.54% lldb_private::ThreadList::ShouldStop(lldb_private::Event*)
- lldb_private::Thread::ShouldStop(lldb_private::Event*) *
- 93.53% lldb_private::StopInfoBreakpoint::ShouldStopSynchronous(lldb_private::Event*) t
- 93.52% lldb_private::BreakpointSite::ShouldStop(lldb_private::StoppointCallbackContext*) i
lldb_private::BreakpointLocationCollection::ShouldStop(lldb_private::StoppointCallbackContext*) k
lldb_private::BreakpointLocation::ShouldStop(lldb_private::StoppointCallbackContext*) b
lldb_private::BreakpointOptions::InvokeCallback(lldb_private::StoppointCallbackContext*, unsigned long, unsigned long) i
DynamicLoaderPOSIXDYLD::RendezvousBreakpointHit(void*, lldb_private::StoppointCallbackContext*, unsigned long, unsigned lo
- DynamicLoaderPOSIXDYLD::RefreshModules() O
- 93.42% DynamicLoaderPOSIXDYLD::RefreshModules()::$_0::operator()(DYLDRendezvous::SOEntry const&) const u
- 93.40% DynamicLoaderPOSIXDYLD::LoadModuleAtAddress(lldb_private::FileSpec const&, unsigned long, unsigned long, bools
- lldb_private::DynamicLoader::LoadModuleAtAddress(lldb_private::FileSpec const&, unsigned long, unsigned long, boos
- 83.90% lldb_private::DynamicLoader::FindModuleViaTarget(lldb_private::FileSpec const&) o
- 83.01% lldb_private::Target::GetOrCreateModule(lldb_private::ModuleSpec const&, bool, lldb_private::Status*
- 77.89% lldb_private::Module::PreloadSymbols()
- 44.06% lldb_private::Symtab::PreloadSymbols()
- 43.66% lldb_private::Symtab::InitNameIndexes()
...
```
We saw that majority of time was spent in `RefreshModules`, with the main culprit within it `LoadModuleAtAddress` which eventually calls `PreloadSymbols`.
At first, `DynamicLoaderPOSIXDYLD::LoadModuleAtAddress` appears fairly independent -- most of it deals with different files and then getting or creating Modules from these files. The portions that aren't independent seem to deal with ModuleLists, which appear concurrency safe. There were members of `DynamicLoaderPOSIXDYLD` I had to synchronize though: namely `m_loaded_modules` which `DynamicLoaderPOSIXDYLD` maintains to map its loaded modules to their link addresses. Without synchronizing this, I ran into SEGFAULTS and other issues when running `check-lldb`. I also locked the assignment and comparison of `m_interpreter_module`, which may be unnecessary.
# Alternate implementations
When creating this patch, another implementation I considered was directly background-ing the call to `Module::PreloadSymbol` in `Target::GetOrCreateModule`. It would have the added benefit of working across platforms generically, and appeared to be concurrency safe. It was done via `Debugger::GetThreadPool().async` directly. However, there were a ton of concurrency issues, so I abandoned that approach for now.
# Testing
With the feature active, I tested via `ninja check-lldb` on both Debug and Release builds several times (~5 or 6 altogether?), and didn't spot additional failing or flaky tests.
I also tested manually on several different binaries, some with around 14000 modules, but just basic operations: launching, reaching main, setting breakpoint, stepping, showing some backtraces.
I've also tested with the flag off just to make sure things behave properly synchronously.
---
Full diff: https://github.com/llvm/llvm-project/pull/130912.diff
4 Files Affected:
- (modified) lldb/source/Plugins/DynamicLoader/POSIX-DYLD/CMakeLists.txt (+12)
- (modified) lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp (+153-39)
- (modified) lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h (+15-4)
- (added) lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLDProperties.td (+8)
``````````diff
diff --git a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/CMakeLists.txt b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/CMakeLists.txt
index c1e00b2dd444f..532bfb20ea0f1 100644
--- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/CMakeLists.txt
+++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/CMakeLists.txt
@@ -1,3 +1,11 @@
+lldb_tablegen(DynamicLoaderPOSIXDYLDProperties.inc -gen-lldb-property-defs
+ SOURCE DynamicLoaderPOSIXDYLDProperties.td
+ TARGET LLDBPluginDynamicLoaderPOSIXDYLDPropertiesGen)
+
+lldb_tablegen(DynamicLoaderPOSIXDYLDPropertiesEnum.inc -gen-lldb-property-enum-defs
+ SOURCE DynamicLoaderPOSIXDYLDProperties.td
+ TARGET LLDBPluginDynamicLoaderPOSIXDYLDPropertiesEnumGen)
+
add_lldb_library(lldbPluginDynamicLoaderPosixDYLD PLUGIN
DYLDRendezvous.cpp
DynamicLoaderPOSIXDYLD.cpp
@@ -13,3 +21,7 @@ add_lldb_library(lldbPluginDynamicLoaderPosixDYLD PLUGIN
LINK_COMPONENTS
Support
)
+
+add_dependencies(lldbPluginDynamicLoaderPosixDYLD
+ LLDBPluginDynamicLoaderPOSIXDYLDPropertiesGen
+ LLDBPluginDynamicLoaderPOSIXDYLDPropertiesEnumGen)
diff --git a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
index 53ba11ac21bd3..f6f6057f830fe 100644
--- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
+++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
@@ -10,6 +10,7 @@
#include "DynamicLoaderPOSIXDYLD.h"
#include "lldb/Breakpoint/BreakpointLocation.h"
+#include "lldb/Core/Debugger.h"
#include "lldb/Core/Module.h"
#include "lldb/Core/ModuleSpec.h"
#include "lldb/Core/PluginManager.h"
@@ -25,6 +26,7 @@
#include "lldb/Utility/LLDBLog.h"
#include "lldb/Utility/Log.h"
#include "lldb/Utility/ProcessInfo.h"
+#include "llvm/Support/ThreadPool.h"
#include <memory>
#include <optional>
@@ -34,9 +36,56 @@ using namespace lldb_private;
LLDB_PLUGIN_DEFINE_ADV(DynamicLoaderPOSIXDYLD, DynamicLoaderPosixDYLD)
+namespace {
+
+#define LLDB_PROPERTIES_dynamicloaderposixdyld
+#include "DynamicLoaderPOSIXDYLDProperties.inc"
+
+enum {
+#define LLDB_PROPERTIES_dynamicloaderposixdyld
+#include "DynamicLoaderPOSIXDYLDPropertiesEnum.inc"
+};
+
+class PluginProperties : public Properties {
+public:
+ static llvm::StringRef GetSettingName() {
+ return DynamicLoaderPOSIXDYLD::GetPluginNameStatic();
+ }
+
+ PluginProperties() : Properties() {
+ m_collection_sp = std::make_shared<OptionValueProperties>(GetSettingName());
+ m_collection_sp->Initialize(g_dynamicloaderposixdyld_properties);
+ }
+
+ ~PluginProperties() override = default;
+
+ bool GetParallelModuleLoad() const {
+ const uint32_t idx = ePropertyParallelModuleLoad;
+ return GetPropertyAtIndexAs<bool>(idx, true);
+ }
+};
+
+} // namespace
+
+static PluginProperties &GetGlobalPluginProperties() {
+ static PluginProperties g_settings;
+ return g_settings;
+}
+
void DynamicLoaderPOSIXDYLD::Initialize() {
PluginManager::RegisterPlugin(GetPluginNameStatic(),
- GetPluginDescriptionStatic(), CreateInstance);
+ GetPluginDescriptionStatic(), CreateInstance,
+ &DebuggerInitialize);
+}
+
+void DynamicLoaderPOSIXDYLD::DebuggerInitialize(Debugger &debugger) {
+ if (!PluginManager::GetSettingForDynamicLoaderPlugin(
+ debugger, PluginProperties::GetSettingName())) {
+ const bool is_global_setting = true;
+ PluginManager::CreateSettingForDynamicLoaderPlugin(
+ debugger, GetGlobalPluginProperties().GetValueProperties(),
+ "Properties for the POSIX dynamic loader plug-in.", is_global_setting);
+ }
}
void DynamicLoaderPOSIXDYLD::Terminate() {}
@@ -184,16 +233,37 @@ void DynamicLoaderPOSIXDYLD::DidLaunch() {
Status DynamicLoaderPOSIXDYLD::CanLoadImage() { return Status(); }
+void DynamicLoaderPOSIXDYLD::SetLoadedModule(const ModuleSP &module_sp,
+ addr_t link_map_addr) {
+ std::unique_lock<std::shared_mutex> lock(m_loaded_modules_rw_mutex);
+ m_loaded_modules[module_sp] = link_map_addr;
+}
+
+void DynamicLoaderPOSIXDYLD::UnloadModule(const ModuleSP &module_sp) {
+ std::unique_lock<std::shared_mutex> lock(m_loaded_modules_rw_mutex);
+ m_loaded_modules.erase(module_sp);
+}
+
+std::optional<lldb::addr_t>
+DynamicLoaderPOSIXDYLD::GetLoadedModuleLinkAddr(const ModuleSP &module_sp) {
+ std::shared_lock<std::shared_mutex> lock(m_loaded_modules_rw_mutex);
+ auto it = m_loaded_modules.find(module_sp);
+ if (it != m_loaded_modules.end())
+ return it->second;
+ return std::nullopt;
+}
+
void DynamicLoaderPOSIXDYLD::UpdateLoadedSections(ModuleSP module,
addr_t link_map_addr,
addr_t base_addr,
bool base_addr_is_offset) {
- m_loaded_modules[module] = link_map_addr;
+ SetLoadedModule(module, link_map_addr);
+
UpdateLoadedSectionsCommon(module, base_addr, base_addr_is_offset);
}
void DynamicLoaderPOSIXDYLD::UnloadSections(const ModuleSP module) {
- m_loaded_modules.erase(module);
+ UnloadModule(module);
UnloadSectionsCommon(module);
}
@@ -401,7 +471,7 @@ void DynamicLoaderPOSIXDYLD::RefreshModules() {
// The rendezvous class doesn't enumerate the main module, so track that
// ourselves here.
ModuleSP executable = GetTargetExecutable();
- m_loaded_modules[executable] = m_rendezvous.GetLinkMapAddress();
+ SetLoadedModule(executable, m_rendezvous.GetLinkMapAddress());
DYLDRendezvous::iterator I;
DYLDRendezvous::iterator E;
@@ -423,34 +493,66 @@ void DynamicLoaderPOSIXDYLD::RefreshModules() {
E = m_rendezvous.end();
m_initial_modules_added = true;
}
- for (; I != E; ++I) {
- // Don't load a duplicate copy of ld.so if we have already loaded it
- // earlier in LoadInterpreterModule. If we instead loaded then unloaded it
- // later, the section information for ld.so would be removed. That
- // information is required for placing breakpoints on Arm/Thumb systems.
- if ((m_interpreter_module.lock() != nullptr) &&
- (I->base_addr == m_interpreter_base))
- continue;
-
- ModuleSP module_sp =
- LoadModuleAtAddress(I->file_spec, I->link_addr, I->base_addr, true);
- if (!module_sp.get())
- continue;
-
- if (module_sp->GetObjectFile()->GetBaseAddress().GetLoadAddress(
- &m_process->GetTarget()) == m_interpreter_base) {
- ModuleSP interpreter_sp = m_interpreter_module.lock();
- if (m_interpreter_module.lock() == nullptr) {
- m_interpreter_module = module_sp;
- } else if (module_sp == interpreter_sp) {
- // Module already loaded.
- continue;
- }
- }
- loaded_modules.AppendIfNeeded(module_sp);
- new_modules.Append(module_sp);
+ std::mutex interpreter_module_mutex;
+ // We should be able to take SOEntry as reference since the data
+ // exists for the duration of this call in `m_rendezvous`.
+ auto load_module_fn =
+ [this, &loaded_modules, &new_modules,
+ &interpreter_module_mutex](const DYLDRendezvous::SOEntry &so_entry) {
+ // Don't load a duplicate copy of ld.so if we have already loaded it
+ // earlier in LoadInterpreterModule. If we instead loaded then
+ // unloaded it later, the section information for ld.so would be
+ // removed. That information is required for placing breakpoints on
+ // Arm/Thumb systems.
+ {
+ // `m_interpreter_module` may be modified by another thread at the
+ // same time, so we guard the access here.
+ std::lock_guard<std::mutex> lock(interpreter_module_mutex);
+ if ((m_interpreter_module.lock() != nullptr) &&
+ (so_entry.base_addr == m_interpreter_base))
+ return;
+ }
+
+ ModuleSP module_sp = LoadModuleAtAddress(
+ so_entry.file_spec, so_entry.link_addr, so_entry.base_addr, true);
+ if (!module_sp.get())
+ return;
+
+ {
+ // `m_interpreter_module` may be modified by another thread at the
+ // same time, so we guard the access here.
+ std::lock_guard<std::mutex> lock(interpreter_module_mutex);
+ // Set the interpreter module, if this is the interpreter.
+ if (module_sp->GetObjectFile()->GetBaseAddress().GetLoadAddress(
+ &m_process->GetTarget()) == m_interpreter_base) {
+ ModuleSP interpreter_sp = m_interpreter_module.lock();
+ if (m_interpreter_module.lock() == nullptr) {
+ m_interpreter_module = module_sp;
+ } else if (module_sp == interpreter_sp) {
+ // Module already loaded.
+ return;
+ }
+ }
+ }
+
+ loaded_modules.AppendIfNeeded(module_sp);
+ new_modules.Append(module_sp);
+ };
+
+ // Loading modules in parallel tends to be faster, but is still unstable.
+ // Once it's stable, we can remove this setting and remove the serial
+ // approach.
+ if (GetGlobalPluginProperties().GetParallelModuleLoad()) {
+ llvm::ThreadPoolTaskGroup task_group(Debugger::GetThreadPool());
+ for (; I != E; ++I)
+ task_group.async(load_module_fn, *I);
+ task_group.wait();
+ } else {
+ for (; I != E; ++I)
+ load_module_fn(*I);
}
+
m_process->GetTarget().ModulesDidLoad(new_modules);
}
@@ -636,7 +738,7 @@ void DynamicLoaderPOSIXDYLD::LoadAllCurrentModules() {
// The rendezvous class doesn't enumerate the main module, so track that
// ourselves here.
ModuleSP executable = GetTargetExecutable();
- m_loaded_modules[executable] = m_rendezvous.GetLinkMapAddress();
+ SetLoadedModule(executable, m_rendezvous.GetLinkMapAddress());
std::vector<FileSpec> module_names;
for (I = m_rendezvous.begin(), E = m_rendezvous.end(); I != E; ++I)
@@ -644,19 +746,31 @@ void DynamicLoaderPOSIXDYLD::LoadAllCurrentModules() {
m_process->PrefetchModuleSpecs(
module_names, m_process->GetTarget().GetArchitecture().GetTriple());
- for (I = m_rendezvous.begin(), E = m_rendezvous.end(); I != E; ++I) {
- ModuleSP module_sp =
- LoadModuleAtAddress(I->file_spec, I->link_addr, I->base_addr, true);
+ auto load_module_fn = [this, &module_list,
+ &log](const DYLDRendezvous::SOEntry &so_entry) {
+ ModuleSP module_sp = LoadModuleAtAddress(
+ so_entry.file_spec, so_entry.link_addr, so_entry.base_addr, true);
if (module_sp.get()) {
LLDB_LOG(log, "LoadAllCurrentModules loading module: {0}",
- I->file_spec.GetFilename());
+ so_entry.file_spec.GetFilename());
module_list.Append(module_sp);
} else {
Log *log = GetLog(LLDBLog::DynamicLoader);
LLDB_LOGF(
log,
"DynamicLoaderPOSIXDYLD::%s failed loading module %s at 0x%" PRIx64,
- __FUNCTION__, I->file_spec.GetPath().c_str(), I->base_addr);
+ __FUNCTION__, so_entry.file_spec.GetPath().c_str(),
+ so_entry.base_addr);
+ }
+ };
+ if (GetGlobalPluginProperties().GetParallelModuleLoad()) {
+ llvm::ThreadPoolTaskGroup task_group(Debugger::GetThreadPool());
+ for (I = m_rendezvous.begin(), E = m_rendezvous.end(); I != E; ++I)
+ task_group.async(load_module_fn, *I);
+ task_group.wait();
+ } else {
+ for (I = m_rendezvous.begin(), E = m_rendezvous.end(); I != E; ++I) {
+ load_module_fn(*I);
}
}
@@ -728,15 +842,15 @@ DynamicLoaderPOSIXDYLD::GetThreadLocalData(const lldb::ModuleSP module_sp,
const lldb::ThreadSP thread,
lldb::addr_t tls_file_addr) {
Log *log = GetLog(LLDBLog::DynamicLoader);
- auto it = m_loaded_modules.find(module_sp);
- if (it == m_loaded_modules.end()) {
+ std::optional<addr_t> link_map_addr_opt = GetLoadedModuleLinkAddr(module_sp);
+ if (!link_map_addr_opt.has_value()) {
LLDB_LOGF(
log, "GetThreadLocalData error: module(%s) not found in loaded modules",
module_sp->GetObjectName().AsCString());
return LLDB_INVALID_ADDRESS;
}
- addr_t link_map = it->second;
+ addr_t link_map = link_map_addr_opt.value();
if (link_map == LLDB_INVALID_ADDRESS || link_map == 0) {
LLDB_LOGF(log,
"GetThreadLocalData error: invalid link map address=0x%" PRIx64,
diff --git a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h
index bde334aaca40b..183a157654ba2 100644
--- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h
+++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h
@@ -28,6 +28,8 @@ class DynamicLoaderPOSIXDYLD : public lldb_private::DynamicLoader {
static void Initialize();
+ static void DebuggerInitialize(lldb_private::Debugger &debugger);
+
static void Terminate();
static llvm::StringRef GetPluginNameStatic() { return "posix-dyld"; }
@@ -93,10 +95,6 @@ class DynamicLoaderPOSIXDYLD : public lldb_private::DynamicLoader {
/// Contains the pointer to the interpret module, if loaded.
std::weak_ptr<lldb_private::Module> m_interpreter_module;
- /// Loaded module list. (link map for each module)
- std::map<lldb::ModuleWP, lldb::addr_t, std::owner_less<lldb::ModuleWP>>
- m_loaded_modules;
-
/// Returns true if the process is for a core file.
bool IsCoreFile() const;
@@ -180,6 +178,19 @@ class DynamicLoaderPOSIXDYLD : public lldb_private::DynamicLoader {
DynamicLoaderPOSIXDYLD(const DynamicLoaderPOSIXDYLD &) = delete;
const DynamicLoaderPOSIXDYLD &
operator=(const DynamicLoaderPOSIXDYLD &) = delete;
+
+ /// Loaded module list. (link map for each module)
+ /// This may be accessed in a multi-threaded context. Use the accessor methods
+ /// to access `m_loaded_modules` safely.
+ std::map<lldb::ModuleWP, lldb::addr_t, std::owner_less<lldb::ModuleWP>>
+ m_loaded_modules;
+ std::shared_mutex m_loaded_modules_rw_mutex;
+
+ void SetLoadedModule(const lldb::ModuleSP &module_sp,
+ lldb::addr_t link_map_addr);
+ void UnloadModule(const lldb::ModuleSP &module_sp);
+ std::optional<lldb::addr_t>
+ GetLoadedModuleLinkAddr(const lldb::ModuleSP &module_sp);
};
#endif // LLDB_SOURCE_PLUGINS_DYNAMICLOADER_POSIX_DYLD_DYNAMICLOADERPOSIXDYLD_H
diff --git a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLDProperties.td b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLDProperties.td
new file mode 100644
index 0000000000000..68e3448196cec
--- /dev/null
+++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLDProperties.td
@@ -0,0 +1,8 @@
+
+include "../../../../include/lldb/Core/PropertiesBase.td"
+
+let Definition = "dynamicloaderposixdyld" in {
+ def ParallelModuleLoad: Property<"parallel-module-load", "Boolean">,
+ DefaultFalse,
+ Desc<"Enable experimental loading of modules in parallel for the POSIX dynamic loader.">;
+}
``````````
</details>
https://github.com/llvm/llvm-project/pull/130912
More information about the lldb-commits
mailing list