🍒 Progress Report Cherrypicks

lldb/include/lldb/Core/Progress.h

@@ -9,6 +9,7 @@
 #ifndef LLDB_CORE_PROGRESS_H
 #define LLDB_CORE_PROGRESS_H
 
+#include "lldb/Host/Alarm.h"
 #include "lldb/lldb-forward.h"
 #include "lldb/lldb-types.h"
 #include "llvm/ADT/StringMap.h"
@@ -150,14 +151,45 @@ class ProgressManager {
   void Increment(const Progress::ProgressData &);
   void Decrement(const Progress::ProgressData &);
 
+  static void Initialize();
+  static void Terminate();
+  static bool Enabled();
   static ProgressManager &Instance();
 
-  static void ReportProgress(const Progress::ProgressData &);
+protected:
+  enum class EventType {
+    Begin,
+    End,
+  };
+  static void ReportProgress(const Progress::ProgressData &progress_data,
+                             EventType type);
 
-private:
-  llvm::StringMap<std::pair<uint64_t, Progress::ProgressData>>
-      m_progress_category_map;
-  std::mutex m_progress_map_mutex;
+  static std::optional<ProgressManager> &InstanceImpl();
+
+  /// Helper function for reporting progress when the alarm in the corresponding
+  /// entry in the map expires.
+  void Expire(llvm::StringRef key);
+
+  /// Entry used for bookkeeping.
+  struct Entry {
+    /// Reference count used for overlapping events.
+    uint64_t refcount = 0;
+
+    /// Data used to emit progress events.
+    Progress::ProgressData data;
+
+    /// Alarm handle used when the refcount reaches zero.
+    Alarm::Handle handle = Alarm::INVALID_HANDLE;
+  };
+
+  /// Map used for bookkeeping.
+  llvm::StringMap<Entry> m_entries;
+
+  /// Mutex to provide the map.
+  std::mutex m_entries_mutex;
+
+  /// Alarm instance to coalesce progress events.
+  Alarm m_alarm;
 };
 
 } // namespace lldb_private

lldb/include/lldb/Host/Alarm.h

@@ -0,0 +1,115 @@
+//===-- Alarm.h -------------------------------------------------*- C++ -*-===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLDB_HOST_ALARM_H
+#define LLDB_HOST_ALARM_H
+
+#include "lldb/Host/HostThread.h"
+#include "lldb/lldb-types.h"
+#include "llvm/Support/Chrono.h"
+
+#include <condition_variable>
+#include <mutex>
+
+namespace lldb_private {
+
+/// \class Alarm abstraction that enables scheduling a callback function after a
+/// specified timeout. Creating an alarm for a callback returns a Handle that
+/// can be used to restart or cancel the alarm.
+class Alarm {
+public:
+  using Handle = uint64_t;
+  using Callback = std::function<void()>;
+  using TimePoint = llvm::sys::TimePoint<>;
+  using Duration = std::chrono::milliseconds;
+
+  Alarm(Duration timeout, bool run_callback_on_exit = false);
+  ~Alarm();
+
+  /// Create an alarm for the given callback. The alarm will expire and the
+  /// callback will be called after the timeout.
+  ///
+  /// \returns
+  ///   Handle which can be used to restart or cancel the alarm.
+  Handle Create(Callback callback);
+
+  /// Restart the alarm for the given Handle. The alarm will expire and the
+  /// callback will be called after the timeout.
+  ///
+  /// \returns
+  ///   True if the alarm was successfully restarted. False if there is no alarm
+  ///   for the given Handle or the alarm already expired.
+  bool Restart(Handle handle);
+
+  /// Cancel the alarm for the given Handle. The alarm and its handle will be
+  /// removed.
+  ///
+  /// \returns
+  ///   True if the alarm was successfully canceled and the Handle removed.
+  ///   False if there is no alarm for the given Handle or the alarm already
+  ///   expired.
+  bool Cancel(Handle handle);
+
+  static constexpr Handle INVALID_HANDLE = 0;
+
+private:
+  /// Helper functions to start, stop and check the status of the alarm thread.
+  /// @{
+  void StartAlarmThread();
+  void StopAlarmThread();
+  bool AlarmThreadRunning();
+  /// @}
+
+  /// Return an unique, monotonically increasing handle.
+  static Handle GetNextUniqueHandle();
+
+  /// Helper to compute the next time the alarm thread needs to wake up.
+  TimePoint GetNextExpiration() const;
+
+  /// Alarm entry.
+  struct Entry {
+    Handle handle;
+    Callback callback;
+    TimePoint expiration;
+
+    Entry(Callback callback, TimePoint expiration);
+    bool operator==(const Entry &rhs) { return handle == rhs.handle; }
+  };
+
+  /// List of alarm entries.
+  std::vector<Entry> m_entries;
+
+  /// Timeout between when an alarm is created and when it fires.
+  Duration m_timeout;
+
+  /// The alarm thread.
+  /// @{
+  HostThread m_alarm_thread;
+  lldb::thread_result_t AlarmThread();
+  /// @}
+
+  /// Synchronize access between the alarm thread and the main thread.
+  std::mutex m_alarm_mutex;
+
+  /// Condition variable used to wake up the alarm thread.
+  std::condition_variable m_alarm_cv;
+
+  /// Flag to signal the alarm thread that something changed and we need to
+  /// recompute the next alarm.
+  bool m_recompute_next_alarm = false;
+
+  /// Flag to signal the alarm thread to exit.
+  bool m_exit = false;
+
+  /// Flag to signal we should run all callbacks on exit.
+  bool m_run_callbacks_on_exit = false;
+};
+
+} // namespace lldb_private
+
+#endif // LLDB_HOST_ALARM_H

lldb/source/API/SystemInitializerFull.cpp

@@ -10,6 +10,7 @@
 #include "lldb/API/SBCommandInterpreter.h"
 #include "lldb/Core/Debugger.h"
 #include "lldb/Core/PluginManager.h"
+#include "lldb/Core/Progress.h"
 #include "lldb/Host/Config.h"
 #include "lldb/Host/Host.h"
 #include "lldb/Initialization/SystemInitializerCommon.h"
@@ -57,6 +58,7 @@ llvm::Error SystemInitializerFull::Initialize() {
   llvm::InitializeAllAsmPrinters();
   llvm::InitializeAllTargetMCs();
   llvm::InitializeAllDisassemblers();
+
   // Initialize the command line parser in LLVM. This usually isn't necessary
   // as we aren't dealing with command line options here, but otherwise some
   // other code in Clang/LLVM might be tempted to call this function from a
@@ -65,13 +67,16 @@ llvm::Error SystemInitializerFull::Initialize() {
   const char *arg0 = "lldb";
   llvm::cl::ParseCommandLineOptions(1, &arg0);
 
+  // Initialize the progress manager.
+  ProgressManager::Initialize();
+
 #define LLDB_PLUGIN(p) LLDB_PLUGIN_INITIALIZE(p);
 #include "Plugins/Plugins.def"
 
   // Initialize plug-ins in core LLDB
   ProcessTrace::Initialize();
 
-  // Scan for any system or user LLDB plug-ins
+  // Scan for any system or user LLDB plug-ins.
   PluginManager::Initialize();
 
   // The process settings need to know about installed plug-ins, so the
@@ -87,15 +92,18 @@ llvm::Error SystemInitializerFull::Initialize() {
 void SystemInitializerFull::Terminate() {
   Debugger::SettingsTerminate();
 
-  // Terminate plug-ins in core LLDB
+  // Terminate plug-ins in core LLDB.
   ProcessTrace::Terminate();
 
-  // Terminate and unload and loaded system or user LLDB plug-ins
+  // Terminate and unload and loaded system or user LLDB plug-ins.
   PluginManager::Terminate();
 
 #define LLDB_PLUGIN(p) LLDB_PLUGIN_TERMINATE(p);
 #include "Plugins/Plugins.def"
 
+  // Terminate the progress manager.
+  ProgressManager::Terminate();
+
   // Now shutdown the common parts, in reverse order.
   SystemInitializerCommon::Terminate();
 }

lldb/source/Core/Progress.cpp

@@ -35,7 +35,10 @@ Progress::Progress(std::string title, std::string details,
 
   std::lock_guard<std::mutex> guard(m_mutex);
   ReportProgress();
-  ProgressManager::Instance().Increment(m_progress_data);
+
+  // Report to the ProgressManager if that subsystem is enabled.
+  if (ProgressManager::Enabled())
+    ProgressManager::Instance().Increment(m_progress_data);
 }
 
 Progress::~Progress() {
@@ -45,7 +48,10 @@ Progress::~Progress() {
   if (!m_completed)
     m_completed = m_total;
   ReportProgress();
-  ProgressManager::Instance().Decrement(m_progress_data);
+
+  // Report to the ProgressManager if that subsystem is enabled.
+  if (ProgressManager::Enabled())
+    ProgressManager::Instance().Decrement(m_progress_data);
 }
 
 void Progress::Increment(uint64_t amount,
@@ -75,55 +81,113 @@ void Progress::ReportProgress() {
   }
 }
 
-ProgressManager::ProgressManager() : m_progress_category_map() {}
+ProgressManager::ProgressManager()
+    : m_entries(), m_alarm(std::chrono::milliseconds(100)) {}
 
 ProgressManager::~ProgressManager() {}
 
+void ProgressManager::Initialize() {
+  assert(!InstanceImpl() && "Already initialized.");
+  InstanceImpl().emplace();
+}
+
+void ProgressManager::Terminate() {
+  assert(InstanceImpl() && "Already terminated.");
+  InstanceImpl().reset();
+}
+
+bool ProgressManager::Enabled() { return InstanceImpl().operator bool(); }
+
 ProgressManager &ProgressManager::Instance() {
-  static std::once_flag g_once_flag;
-  static ProgressManager *g_progress_manager = nullptr;
-  std::call_once(g_once_flag, []() {
-    // NOTE: known leak to avoid global destructor chain issues.
-    g_progress_manager = new ProgressManager();
-  });
-  return *g_progress_manager;
+  assert(InstanceImpl() && "ProgressManager must be initialized");
+  return *InstanceImpl();
+}
+
+std::optional<ProgressManager> &ProgressManager::InstanceImpl() {
+  static std::optional<ProgressManager> g_progress_manager;
+  return g_progress_manager;
 }
 
 void ProgressManager::Increment(const Progress::ProgressData &progress_data) {
-  std::lock_guard<std::mutex> lock(m_progress_map_mutex);
-  // If the current category exists in the map then it is not an initial report,
-  // therefore don't broadcast to the category bit. Also, store the current
-  // progress data in the map so that we have a note of the ID used for the
-  // initial progress report.
-  if (!m_progress_category_map.contains(progress_data.title)) {
-    m_progress_category_map[progress_data.title].second = progress_data;
-    ReportProgress(progress_data);
+  std::lock_guard<std::mutex> lock(m_entries_mutex);
+
+  llvm::StringRef key = progress_data.title;
+  bool new_entry = !m_entries.contains(key);
+  Entry &entry = m_entries[progress_data.title];
+
+  if (new_entry) {
+    // This is a new progress event. Report progress and store the progress
+    // data.
+    ReportProgress(progress_data, EventType::Begin);
+    entry.data = progress_data;
+  } else if (entry.refcount == 0) {
+    // This is an existing entry that was scheduled to be deleted but a new one
+    // came in before the timer expired.
+    assert(entry.handle != Alarm::INVALID_HANDLE);
+
+    if (!m_alarm.Cancel(entry.handle)) {
+      // The timer expired before we had a chance to cancel it. We have to treat
+      // this as an entirely new progress event.
+      ReportProgress(progress_data, EventType::Begin);
+    }
+    // Clear the alarm handle.
+    entry.handle = Alarm::INVALID_HANDLE;
   }
-  m_progress_category_map[progress_data.title].first++;
+
+  // Regardless of how we got here, we need to bump the reference count.
+  entry.refcount++;
 }
 
 void ProgressManager::Decrement(const Progress::ProgressData &progress_data) {
-  std::lock_guard<std::mutex> lock(m_progress_map_mutex);
-  auto pos = m_progress_category_map.find(progress_data.title);
+  std::lock_guard<std::mutex> lock(m_entries_mutex);
+  llvm::StringRef key = progress_data.title;
 
-  if (pos == m_progress_category_map.end())
+  if (!m_entries.contains(key))
     return;
 
-  if (pos->second.first <= 1) {
-    ReportProgress(pos->second.second);
-    m_progress_category_map.erase(progress_data.title);
-  } else {
-    --pos->second.first;
+  Entry &entry = m_entries[key];
+  entry.refcount--;
+
+  if (entry.refcount == 0) {
+    assert(entry.handle == Alarm::INVALID_HANDLE);
+
+    // Copy the key to a std::string so we can pass it by value to the lambda.
+    // The underlying StringRef will not exist by the time the callback is
+    // called.
+    std::string key_str = std::string(key);
+
+    // Start a timer. If it expires before we see another progress event, it
+    // will be reported.
+    entry.handle = m_alarm.Create([=]() { Expire(key_str); });
   }
 }
 
 void ProgressManager::ReportProgress(
-    const Progress::ProgressData &progress_data) {
+    const Progress::ProgressData &progress_data, EventType type) {
   // The category bit only keeps track of when progress report categories have
   // started and ended, so clear the details and reset other fields when
   // broadcasting to it since that bit doesn't need that information.
-  Debugger::ReportProgress(
-      progress_data.progress_id, progress_data.title, "",
-      Progress::kNonDeterministicTotal, Progress::kNonDeterministicTotal,
-      progress_data.debugger_id, Debugger::eBroadcastBitProgressCategory);
+  const uint64_t completed =
+      (type == EventType::Begin) ? 0 : Progress::kNonDeterministicTotal;
+  Debugger::ReportProgress(progress_data.progress_id, progress_data.title, "",
+                           completed, Progress::kNonDeterministicTotal,
+                           progress_data.debugger_id,
+                           Debugger::eBroadcastBitProgressCategory);
+}
+
+void ProgressManager::Expire(llvm::StringRef key) {
+  std::lock_guard<std::mutex> lock(m_entries_mutex);
+
+  // This shouldn't happen but be resilient anyway.
+  if (!m_entries.contains(key))
+    return;
+
+  // A new event came in and the alarm fired before we had a chance to restart
+  // it.
+  if (m_entries[key].refcount != 0)
+    return;
+
+  // We're done with this entry.
+  ReportProgress(m_entries[key].data, EventType::End);
+  m_entries.erase(key);
 }