Initiate loading of shared libraries in parallel

[scott.smith]

Summary

This change forks a thread for each shared library, so that as much work as possible can be done in parallel.

[scott.smith]

This comment was removed by scott.smith.

[scott.smith]

update to use a private llvm::ThreadPool. I chose this over a 2nd global "TaskPool" because if the threads are going to be short lived, there isn't much point in having a global pool rather than a short-lived instantiated one.

[zturner]

source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp

537–557

I'm still unclear why we're still going down this route of adding a very specialized instance of parallelism to this one place, that is probably going to be less efficient than having a global thread pool (as now you will have to spin up and destroy the entire thread pool every time you go to load shared libraries).

So, rather than keep repeating the same thing over and over, I went and looked into this a little bit. It turns out LLD has a large library of parallel algorithms in lld/Include/lld/Parallel.h. There is almost nothing specific to LLD in any of these algorithms however, and they should probably be moved to llvm support. Upon doing so, you will be able to write this code as:

std::vector<ModuleSP> Modules(m_rendevous.size());
llvm::parallel_for_each(make_integer_range(0, m_rendevous.size()), [this, &Modules](int I)
  {
    auto &R = m_rendevous[I];
    Modules[I] = LoadModuleAtAddress(R.file_spec, R.link_addr, R.base_addr, true);
  });
for (const auto &M : Modules)
   module_list.Append(M);

Please look into making this change.

[zturner]

source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp

537–557

Note that this is, of course, exactly why I've been saying all along that we should not even be discussing this here. Had the discussion happened over on the LLVM side the first time I asked about this, someone could have mentioned this without me having to spend time looking into it.

[scott.smith]

source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp

537–557

As long as those routines allow you to recursively call those routines, then great, go for it.

The spinning up/tearing down of threads is not expensive, at least in my measured use case.

[zturner]

source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp

537–557

As long as those routines allow you to recursively call those routines, then great, go for it.

Right, which is why I've asked at least 4 times for it to be looked into. That doesn't mean mean use the first thing you see and check it in right away, it means please see if there is some way to use existing code to solve the problem. If it turns out this doesn't allow recursive use, then a followup would be to understand why not, and if it is hard to change.

So again, please look into this and see if it will work for your needs.

[zturner]

D32826

[scott.smith]

source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp

537–557

It seems there are two possible long term fixes:

1. Separate thread pool
2. Make parallel_for support recursive use.

Personally I'm in favor of 2. @ruiu brought it up as a preferred goal too. The q is then how to proceed in the short term:

1. Wait for parallel_for to land in llvm, make it support recursion (hopefully Windows ConcRT supports it too?), then convert this change and commit.
2. Change TaskMapOverInt to take a ThreadPool, then have it use a private ThreadPool for now, then easily convert to the #1 above later.
3. Change TaskMapOverInt to support recursion now (which will also require changes to lldb::TaskPool, IMO), then do #1 above once it lands. Advantage is we prove it works, which hopefully makes moving it upstream easier.

Any preferences on how to proceed?

[clayborg]

I would suggest adding Pavel and Tamas, or anyone that has contributed to DynamicLoaderPOSIXDYLD as this will affect them.

[labath]

I'm out of office this week. Could you hold until I get back? Hopefully we will see some development on the llvm/lld front in the meanwhile.

source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h

public:
   //------------------------------------------------------------------
   // PluginInterface protocol
   //------------------------------------------------------------------
   lldb_private::ConstString GetPluginName() override;
 
   uint32_t GetPluginVersion() override;
 
 protected:
+  /// Mutex to protect various global variables during parallel shared library
+  /// loading.
+  std::recursive_mutex m_mutex;
+
   /// Runtime linker rendezvous structure.
   DYLDRendezvous m_rendezvous;
 
   /// Virtual load address of the inferior process.
   lldb::addr_t m_load_offset;
 
   /// Virtual entry address of the inferior process.
   lldb::addr_t m_entry_point;

source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp

 #include "lldb/Target/Platform.h"
 #include "lldb/Target/Process.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Target/Thread.h"
 #include "lldb/Target/ThreadPlanRunToAddress.h"
 #include "lldb/Utility/Log.h"
 
 // C++ Includes
+#include <thread>
+
 // C Includes
 
 using namespace lldb;
 using namespace lldb_private;
 
 void DynamicLoaderPOSIXDYLD::Initialize() {
   PluginManager::RegisterPlugin(GetPluginNameStatic(),
                                 GetPluginDescriptionStatic(), CreateInstance);
if (log) {
                               : "<null>",
                     m_process ? m_process->GetID() : LLDB_INVALID_PROCESS_ID);
       }
     }
   }
 }
 
 void DynamicLoaderPOSIXDYLD::DidLaunch() {
+  std::lock_guard<std::recursive_mutex> guard(m_mutex);
   Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_DYNAMIC_LOADER));
   if (log)
     log->Printf("DynamicLoaderPOSIXDYLD::%s()", __FUNCTION__);
 
   ModuleSP executable;
   addr_t load_offset;
 
   m_auxv.reset(new AuxVector(m_process));

 }
 
 Error DynamicLoaderPOSIXDYLD::CanLoadImage() { return Error(); }
 
 void DynamicLoaderPOSIXDYLD::UpdateLoadedSections(ModuleSP module,
                                                   addr_t link_map_addr,
                                                   addr_t base_addr,
                                                   bool base_addr_is_offset) {
+  std::lock_guard<std::recursive_mutex> guard(m_mutex);
   m_loaded_modules[module] = link_map_addr;
   UpdateLoadedSectionsCommon(module, base_addr, base_addr_is_offset);
 }
 
 void DynamicLoaderPOSIXDYLD::UnloadSections(const ModuleSP module) {
+  std::lock_guard<std::recursive_mutex> guard(m_mutex);
   m_loaded_modules.erase(module);
 
   UnloadSectionsCommon(module);
 }
 
 void DynamicLoaderPOSIXDYLD::ProbeEntry() {
+  std::lock_guard<std::recursive_mutex> guard(m_mutex);
   Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_DYNAMIC_LOADER));
 
   const addr_t entry = GetEntryPoint();
   if (entry == LLDB_INVALID_ADDRESS) {
     if (log)
       log->Printf(
           "DynamicLoaderPOSIXDYLD::%s pid %" PRIu64
           " GetEntryPoint() returned no address, not setting entry breakpoint",
bool DynamicLoaderPOSIXDYLD::EntryBreakpointHit(
   }
 
   dyld_instance->LoadAllCurrentModules();
   dyld_instance->SetRendezvousBreakpoint();
   return false; // Continue running.
 }
 
 void DynamicLoaderPOSIXDYLD::SetRendezvousBreakpoint() {
+  std::lock_guard<std::recursive_mutex> guard(m_mutex);
   Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_DYNAMIC_LOADER));
 
   addr_t break_addr = m_rendezvous.GetBreakAddress();
   Target &target = m_process->GetTarget();
 
   if (m_dyld_bid == LLDB_INVALID_BREAK_ID) {
     if (log)
       log->Printf("DynamicLoaderPOSIXDYLD::%s pid %" PRIu64
bool DynamicLoaderPOSIXDYLD::RendezvousBreakpointHit(
     user_id_t break_loc_id) {
   assert(baton && "null baton");
   if (!baton)
     return false;
 
   Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_DYNAMIC_LOADER));
   DynamicLoaderPOSIXDYLD *const dyld_instance =
       static_cast<DynamicLoaderPOSIXDYLD *>(baton);
+  std::lock_guard<std::recursive_mutex> guard(dyld_instance->m_mutex);
   if (log)
     log->Printf("DynamicLoaderPOSIXDYLD::%s called for pid %" PRIu64,
                 __FUNCTION__,
                 dyld_instance->m_process ? dyld_instance->m_process->GetID()
                                          : LLDB_INVALID_PROCESS_ID);
 
   dyld_instance->RefreshModules();
 
   // Return true to stop the target, false to just let the target run.
   const bool stop_when_images_change = dyld_instance->GetStopWhenImagesChange();
   if (log)
     log->Printf("DynamicLoaderPOSIXDYLD::%s pid %" PRIu64
                 " stop_when_images_change=%s",
                 __FUNCTION__,
                 dyld_instance->m_process ? dyld_instance->m_process->GetID()
                                          : LLDB_INVALID_PROCESS_ID,
                 stop_when_images_change ? "true" : "false");
   return stop_when_images_change;
 }
 
 void DynamicLoaderPOSIXDYLD::RefreshModules() {
+  std::lock_guard<std::recursive_mutex> guard(m_mutex);
   if (!m_rendezvous.Resolve())
     return;
 
   DYLDRendezvous::iterator I;
   DYLDRendezvous::iterator E;
 
   ModuleList &loaded_modules = m_process->GetTarget().GetImages();
 
if (m_rendezvous.ModulesDidUnload()) {
     loaded_modules.Remove(old_modules);
     m_process->GetTarget().ModulesDidUnload(old_modules, false);
   }
 }
 
 ThreadPlanSP
 DynamicLoaderPOSIXDYLD::GetStepThroughTrampolinePlan(Thread &thread,
                                                      bool stop) {
+  std::lock_guard<std::recursive_mutex> guard(m_mutex);
   ThreadPlanSP thread_plan_sp;
 
   StackFrame *frame = thread.GetStackFrameAtIndex(0).get();
   const SymbolContext &context = frame->GetSymbolContext(eSymbolContextSymbol);
   Symbol *sym = context.symbol;
 
   if (sym == NULL || !sym->IsTrampoline())
     return thread_plan_sp;
void DynamicLoaderPOSIXDYLD::LoadAllCurrentModules() {
   }
 
   std::vector<FileSpec> module_names;
   for (I = m_rendezvous.begin(), E = m_rendezvous.end(); I != E; ++I)
     module_names.push_back(I->file_spec);
   m_process->PrefetchModuleSpecs(
       module_names, m_process->GetTarget().GetArchitecture().GetTriple());
 
+  struct loader {
+    std::thread t;
+    ModuleSP m;
+    DYLDRendezvous::iterator I;
+  };
+  std::list<loader> loaders;
   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);
-    if (module_sp.get()) {
-      module_list.Append(module_sp);
+    loaders.push_back(loader());
+    auto * last = &loaders.back();
+    last->I = I;
+    last->t = std::thread([this, last]()
+    {
+      last->m = LoadModuleAtAddress(
+        last->I->file_spec, last->I->link_addr, last->I->base_addr, true);
+    });
+  }
+  for (auto & l : loaders) {
+    l.t.join();
+    if (l.m.get()) {
+      module_list.Append(l.m);
     } else {
       Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_DYNAMIC_LOADER));
       if (log)
         log->Printf(
             "DynamicLoaderPOSIXDYLD::%s failed loading module %s at 0x%" PRIx64,
-            __FUNCTION__, I->file_spec.GetCString(), I->base_addr);
+            __FUNCTION__, l.I->file_spec.GetCString(), l.I->base_addr);
     }
   }
 
   m_process->GetTarget().ModulesDidLoad(module_list);
 }
 
 addr_t DynamicLoaderPOSIXDYLD::ComputeLoadOffset() {
+  std::lock_guard<std::recursive_mutex> guard(m_mutex);
   addr_t virt_entry;
 
   if (m_load_offset != LLDB_INVALID_ADDRESS)
     return m_load_offset;
 
   if ((virt_entry = GetEntryPoint()) == LLDB_INVALID_ADDRESS)
     return LLDB_INVALID_ADDRESS;
 
addr_t DynamicLoaderPOSIXDYLD::ComputeLoadOffset() {
   if (!file_entry.IsValid())
     return LLDB_INVALID_ADDRESS;
 
   m_load_offset = virt_entry - file_entry.GetFileAddress();
   return m_load_offset;
 }
 
 void DynamicLoaderPOSIXDYLD::EvalVdsoStatus() {
+  std::lock_guard<std::recursive_mutex> guard(m_mutex);
   AuxVector::iterator I = m_auxv->FindEntry(AuxVector::AT_SYSINFO_EHDR);
 
   if (I != m_auxv->end())
     m_vdso_base = I->value;
 }
 
 addr_t DynamicLoaderPOSIXDYLD::GetEntryPoint() {
+  std::lock_guard<std::recursive_mutex> guard(m_mutex);
   if (m_entry_point != LLDB_INVALID_ADDRESS)
     return m_entry_point;
 
   if (m_auxv.get() == NULL)
     return LLDB_INVALID_ADDRESS;
 
   AuxVector::iterator I = m_auxv->FindEntry(AuxVector::AT_ENTRY);
 
addr_t DynamicLoaderPOSIXDYLD::GetEntryPoint() {
 
   return m_entry_point;
 }
 
 lldb::addr_t
 DynamicLoaderPOSIXDYLD::GetThreadLocalData(const lldb::ModuleSP module_sp,
                                            const lldb::ThreadSP thread,
                                            lldb::addr_t tls_file_addr) {
+  std::lock_guard<std::recursive_mutex> guard(m_mutex);
   auto it = m_loaded_modules.find(module_sp);
   if (it == m_loaded_modules.end())
     return LLDB_INVALID_ADDRESS;
 
   addr_t link_map = it->second;
   if (link_map == LLDB_INVALID_ADDRESS)
     return LLDB_INVALID_ADDRESS;
 
DynamicLoaderPOSIXDYLD::GetThreadLocalData(const lldb::ModuleSP module_sp,
   if (tls_block == LLDB_INVALID_ADDRESS)
     return LLDB_INVALID_ADDRESS;
   else
     return tls_block + tls_file_addr;
 }
 
 void DynamicLoaderPOSIXDYLD::ResolveExecutableModule(
     lldb::ModuleSP &module_sp) {
+  std::lock_guard<std::recursive_mutex> guard(m_mutex);
   Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_DYNAMIC_LOADER));
 
   if (m_process == nullptr)
     return;
 
   auto &target = m_process->GetTarget();
   const auto platform_sp = target.GetPlatform();
 
if (error.Fail()) {
     return;
   }
 
   target.SetExecutableModule(module_sp, false);
 }
 
 bool DynamicLoaderPOSIXDYLD::AlwaysRelyOnEHUnwindInfo(
     lldb_private::SymbolContext &sym_ctx) {
+  std::lock_guard<std::recursive_mutex> guard(m_mutex);
   ModuleSP module_sp;
   if (sym_ctx.symbol)
     module_sp = sym_ctx.symbol->GetAddressRef().GetModule();
   if (!module_sp && sym_ctx.function)
     module_sp =
         sym_ctx.function->GetAddressRange().GetBaseAddress().GetModule();
   if (!module_sp)
     return false;
 
   return module_sp->GetFileSpec().GetPath() == "[vdso]";
 }