[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

1976

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

1976

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

1976

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

1976

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

1976

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

760

850

remove

1976

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

760

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.

1976

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

1976

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>
: 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());
   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();
   });
 }
 
 bool Debugger::GetCloseInputOnEOF() const {
   //    return m_input_comm.GetCloseOnEOF();
   return false;
 }
 
 void Debugger::SetCloseInputOnEOF(bool b) {
if (!repl_sp) {
     return err;
   }
 
   repl_sp->SetCompilerOptions(repl_options);
   repl_sp->RunLoop();
 
   return err;
 }
+
+llvm::ThreadPool &Debugger::GetThreadPool() {
+  // 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;
+}

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);