[lldb] use one shared ThreadPool and task groups

[llunak]

Summary

As a preparation for parallelizing loading of symbols (D122975), it is necessary to use just one thread pool to avoid using a thread pool from inside a task of another thread pool.

The ThreadPool groups change is D123225 .

[tschuett]

You cannot access/find the global thread pool? Sorry, wrong diff.

[aganea]

lldb/source/Core/Debugger.cpp

1979

Ideally this should be explicitly created on the stack & passed along on the callstack or in a context, like MLIR does: https://github.com/llvm/llvm-project/blob/main/mlir/include/mlir/IR/MLIRContext.h#L48

lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp

126–134

Like suggested in D123225, wouldn't it be better if we had group.async(finalize_fn, &IndexSet::function_basenames)? Same for the waits: group.wait();.

[clayborg]

lldb/source/Core/Debugger.cpp

1979

We need one instance of the thread pool so we can't create on the stack.

Static is not great because of the C++ global destructor chain could try and use if "main" exits and we still have threads running. I would do something like the "g_debugger_list_ptr" where you create a static variable in Debugger.cpp:105 which is a pointer, and we initialize it inside of Debugger::Initialize() like we do for "g_debugger_list_ptr". Then the thread pool will not cause shutdown crashes when "main" exits before other threads.

[aganea]

lldb/source/Core/Debugger.cpp

1979

I meant having the ThreadPool in a context created by main() or the lib caller, before getting here in library/plugin code, and passed along. LLD does that too: https://github.com/llvm/llvm-project/blob/main/lld/ELF/Driver.cpp#L94 - the statics in Debugger.cpp seems like a workaround for that. In any case, it's better than having the static here.

[clayborg]

lldb/source/Core/Debugger.cpp

1979

In LLDB, the lldb_private::Debugger has static functions that hand out things that are needed for the debugger to do its work, so I like the static function here. We don't allow any llvm or clang code to make it through our public API (in lldb::SB* classes), so we do need code inside the debugger to be able to access global resources and the debugger class is where this should live.

We can also just have code like:

// NOTE: intentional leak to avoid issues with C++ destructor chain
static llvm::ThreadPool *g_thread_pool = nullptr;
static llvm::once_flag g_once_flag;
llvm::call_once(g_once_flag, []() {
  g_thread_pool = new llvm::ThreadPool(llvm::optimal_concurrency());
});
return *g_thread_pool;

[llunak]

lldb/source/Core/Debugger.cpp

1979

I've changed the code to initialize/cleanup from Debugger ctor/dtor.

[llunak]

ThreadPool object is now created/destroyed by Debugger class ctor/dtor.
Adapted to API changes from D123225.

[clayborg]

lldb/source/Core/Debugger.cpp

761

852

remove

1979

You can't do this as there can be more than one debugger. This needs to be a true global and use a once_flag.

Xcode creates one debugger per target window, that way each one has its own command interpreter etc;

[clayborg]

lldb/source/Core/Debugger.cpp

761

actually probably best to do it the way I originally described all inside of "llvm::ThreadPool &Debugger::GetThreadPool()" since multiple debuggers can exist within the same process.

1979

Probably best to do it as described above where everything is contained in this function due to multiple debuggers being possible, so we can't tie anything to the lifetime of any debugger and we need to not crash when the main thread exits first

[llunak]

lldb/source/Core/Debugger.cpp

1979

I see, I thought there could be just one Debugger instance. I'll use the call_once code from above then.

[llunak]

Changed to use std::call_once().

lldb/include/lldb/Core/Debugger.h

 #include "llvm/Support/Threading.h"
 
 #include <cassert>
 #include <cstddef>
 #include <cstdio>
 
 namespace llvm {
 class raw_ostream;
+class ThreadPool;
 }
 
 namespace lldb_private {
 class Address;
 class CommandInterpreter;
 class Process;
 class Stream;
 class SymbolContext;
public:
   // target with entities that will be copied over to new targets.
   Target &GetSelectedOrDummyTarget(bool prefer_dummy = false);
   Target &GetDummyTarget() { return *m_dummy_target_sp; }
 
   lldb::BroadcasterManagerSP GetBroadcasterManager() {
     return m_broadcaster_manager_sp;
   }
 
+  /// Shared thread poll. Use only with ThreadPoolTaskGroup.
+  static llvm::ThreadPool &GetThreadPool();
+
   /// Report warning events.
   ///
   /// Progress events will be delivered to any debuggers that have listeners
   /// for the eBroadcastBitError.
   ///
   /// \param[in] message
   ///   The warning message to be reported.
   ///

lldb/source/Core/Debugger.cpp

 
 #include "llvm/ADT/None.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/iterator.h"
 #include "llvm/Support/DynamicLibrary.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Process.h"
+#include "llvm/Support/ThreadPool.h"
 #include "llvm/Support/Threading.h"
 #include "llvm/Support/raw_ostream.h"
 
 #include <cstdio>
 #include <cstdlib>
 #include <cstring>
 #include <list>
 #include <memory>

 class Address;
 }
 
 using namespace lldb;
 using namespace lldb_private;
 
 static lldb::user_id_t g_unique_id = 1;
 static size_t g_debugger_event_thread_stack_bytes = 8 * 1024 * 1024;
+static std::unique_ptr<llvm::ThreadPool> g_threadpool;
 
 #pragma mark Static Functions
 
 typedef std::vector<DebuggerSP> DebuggerList;
 static std::recursive_mutex *g_debugger_list_mutex_ptr =
     nullptr; // NOTE: intentional leak to avoid issues with C++ destructor chain
 static DebuggerList *g_debugger_list_ptr =
     nullptr; // NOTE: intentional leak to avoid issues with C++ destructor chain
: UserID(g_unique_id++),
           std::make_unique<CommandInterpreter>(*this, false)),
       m_io_handler_stack(), m_instance_name(), m_loaded_plugins(),
       m_event_handler_thread(), m_io_handler_thread(),
       m_sync_broadcaster(nullptr, "lldb.debugger.sync"),
       m_broadcaster(m_broadcaster_manager_sp,
                     GetStaticBroadcasterClass().AsCString()),
       m_forward_listener_sp(), m_clear_once() {
   m_instance_name.SetString(llvm::formatv("debugger_{0}", GetID()).str());
+  g_threadpool = std::make_unique<llvm::ThreadPool>(llvm::optimal_concurrency());
   if (log_callback)
     m_log_callback_stream_sp =
         std::make_shared<StreamCallback>(log_callback, baton);
   m_command_interpreter_up->Initialize();
   // Always add our default platform to the platform list
   PlatformSP default_platform_sp(Platform::GetHostPlatform());
   assert(default_platform_sp);
   m_platform_list.Append(default_platform_sp, true);
llvm::call_once(m_clear_once, [this]() {
     m_broadcaster_manager_sp->Clear();
 
     // Close the input file _before_ we close the input read communications
     // class as it does NOT own the input file, our m_input_file does.
     m_terminal_state.Clear();
     GetInputFile().Close();
 
     m_command_interpreter_up->Clear();
+    g_threadpool.reset();
   });
 }
 
 bool Debugger::GetCloseInputOnEOF() const {
   //    return m_input_comm.GetCloseOnEOF();
   return false;
 }
 
if (!repl_sp) {
     return err;
   }
 
   repl_sp->SetCompilerOptions(repl_options);
   repl_sp->RunLoop();
 
   return err;
 }
+
+llvm::ThreadPool &Debugger::GetThreadPool() {
+  assert(g_threadpool);
+  return *g_threadpool;
+}

lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp

 //===-- ManualDWARFIndex.cpp ----------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "Plugins/SymbolFile/DWARF/ManualDWARFIndex.h"
 #include "Plugins/Language/ObjC/ObjCLanguage.h"
 #include "Plugins/SymbolFile/DWARF/DWARFDebugInfo.h"
 #include "Plugins/SymbolFile/DWARF/DWARFDeclContext.h"
 #include "Plugins/SymbolFile/DWARF/LogChannelDWARF.h"
 #include "Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h"
 #include "lldb/Core/DataFileCache.h"
+#include "lldb/Core/Debugger.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Core/Progress.h"
 #include "lldb/Symbol/ObjectFile.h"
 #include "lldb/Utility/DataEncoder.h"
 #include "lldb/Utility/DataExtractor.h"
 #include "lldb/Utility/Stream.h"
 #include "lldb/Utility/Timer.h"
 #include "llvm/Support/FormatVariadic.h"
void ManualDWARFIndex::Index() {
 
   auto extract_fn = [&](size_t cu_idx) {
     clear_cu_dies[cu_idx] = units_to_index[cu_idx]->ExtractDIEsScoped();
     progress.Increment();
   };
 
   // Share one thread pool across operations to avoid the overhead of
   // recreating the threads.
-  llvm::ThreadPool pool(llvm::optimal_concurrency(units_to_index.size()));
+  llvm::ThreadPoolTaskGroup task_group(Debugger::GetThreadPool());
 
   // Create a task runner that extracts dies for each DWARF unit in a
   // separate thread.
   // First figure out which units didn't have their DIEs already
   // parsed and remember this.  If no DIEs were parsed prior to this index
   // function call, we are going to want to clear the CU dies after we are
   // done indexing to make sure we don't pull in all DWARF dies, but we need
   // to wait until all units have been indexed in case a DIE in one
   // unit refers to another and the indexes accesses those DIEs.
   for (size_t i = 0; i < units_to_index.size(); ++i)
-    pool.async(extract_fn, i);
-  pool.wait();
+    task_group.async(extract_fn, i);
+  task_group.wait();
 
   // Now create a task runner that can index each DWARF unit in a
   // separate thread so we can index quickly.
   for (size_t i = 0; i < units_to_index.size(); ++i)
-    pool.async(parser_fn, i);
-  pool.wait();
+    task_group.async(parser_fn, i);
+  task_group.wait();
 
   auto finalize_fn = [this, &sets, &progress](NameToDIE(IndexSet::*index)) {
     NameToDIE &result = m_set.*index;
     for (auto &set : sets)
       result.Append(set.*index);
     result.Finalize();
     progress.Increment();
   };
 
-  pool.async(finalize_fn, &IndexSet::function_basenames);
-  pool.async(finalize_fn, &IndexSet::function_fullnames);
-  pool.async(finalize_fn, &IndexSet::function_methods);
-  pool.async(finalize_fn, &IndexSet::function_selectors);
-  pool.async(finalize_fn, &IndexSet::objc_class_selectors);
-  pool.async(finalize_fn, &IndexSet::globals);
-  pool.async(finalize_fn, &IndexSet::types);
-  pool.async(finalize_fn, &IndexSet::namespaces);
-  pool.wait();
+  task_group.async(finalize_fn, &IndexSet::function_basenames);
+  task_group.async(finalize_fn, &IndexSet::function_fullnames);
+  task_group.async(finalize_fn, &IndexSet::function_methods);
+  task_group.async(finalize_fn, &IndexSet::function_selectors);
+  task_group.async(finalize_fn, &IndexSet::objc_class_selectors);
+  task_group.async(finalize_fn, &IndexSet::globals);
+  task_group.async(finalize_fn, &IndexSet::types);
+  task_group.async(finalize_fn, &IndexSet::namespaces);
+  task_group.wait();
 
   SaveToCache();
 }
 
 void ManualDWARFIndex::IndexUnit(DWARFUnit &unit, SymbolFileDWARFDwo *dwp,
                                  IndexSet &set) {
   Log *log = GetLog(DWARFLog::Lookups);