[Lldb-commits] [lldb] [lldb] Implement coalescing of disjoint progress events (PR #84854)

[Greg Clayton via lldb-commits]

================
@@ -75,45 +81,86 @@ void Progress::ReportProgress() {
   }
 }
 
-ProgressManager::ProgressManager() : m_progress_category_map() {}
+ProgressManager::ProgressManager()
+    : m_alarm(std::chrono::milliseconds(100)), m_entries() {}
 
 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;
+  std::lock_guard<std::mutex> lock(m_entries_mutex);
+  llvm::StringRef key = progress_data.title;
+
+  // This is a new progress event.
+  if (!m_entries.contains(key)) {
     ReportProgress(progress_data, EventType::Begin);
+    Entry &entry = m_entries[key];
+    entry.data = progress_data;
+    entry.refcount = 1;
+    return;
+  }
+
+  // This is an existing progress event.
+  Entry &entry = m_entries[key];
+
+  // The progress event was scheduled to be deleted but a new one came in before
+  // the timer expired.
+  if (entry.refcount == 0) {
+    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);
+    }
+    entry.refcount = 1;
+    entry.handle = Alarm::INVALID_HANDLE;
+  } else {
+    entry.refcount++;
   }
----------------
clayborg wrote:

One idea for this code could be:
```
if (entry.refcount++ == 1) {
  // This entry was scheduled to be expired, but no longer is.
  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);
  }
  entry.handle = Alarm::INVALID_HANDLE;
}
```
This just removes the need to manually set `entry.refcount = 1;` or to separately increment refcount in the else body

https://github.com/llvm/llvm-project/pull/84854