[Lldb-commits] [lldb] afe8f20 - Revert "[lldb] Rate limit progress reports -- different approach [WIP-ish]"

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2023-06-16T09:09:56+02:00
New Revision: afe8f20bb8dd1808cae542eb7ba0fc11a7886918

URL: https://github.com/llvm/llvm-project/commit/afe8f20bb8dd1808cae542eb7ba0fc11a7886918
DIFF: https://github.com/llvm/llvm-project/commit/afe8f20bb8dd1808cae542eb7ba0fc11a7886918.diff

LOG: Revert "[lldb] Rate limit progress reports -- different approach [WIP-ish]"

This reverts commit c30853460da7446f92bc1e516f9cbe2c5df6e136, which I
pushed accidentally -- sorry.

Added: 
    

Modified: 
    lldb/include/lldb/Core/Progress.h
    lldb/source/Core/Progress.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/include/lldb/Core/Progress.h b/lldb/include/lldb/Core/Progress.h
index 3bafd3d4b34ea..b2b8781a43b05 100644
--- a/lldb/include/lldb/Core/Progress.h
+++ b/lldb/include/lldb/Core/Progress.h
@@ -9,11 +9,9 @@
 #ifndef LLDB_CORE_PROGRESS_H
 #define LLDB_CORE_PROGRESS_H
 
-#include "lldb/Host/HostThread.h"
 #include "lldb/Utility/ConstString.h"
 #include "lldb/lldb-types.h"
 #include <atomic>
-#include <future>
 #include <mutex>
 #include <optional>
 
@@ -94,26 +92,24 @@ class Progress {
   void Increment(uint64_t amount = 1, std::string update = {});
 
 private:
-  void SendPeriodicReports(std::shared_future<void> done);
-  void ReportProgress(std::string update);
+  void ReportProgress(std::string update = {});
   static std::atomic<uint64_t> g_id;
   /// The title of the progress activity.
   std::string m_title;
+  std::mutex m_mutex;
   /// A unique integer identifier for progress reporting.
   const uint64_t m_id;
   /// How much work ([0...m_total]) that has been completed.
-  std::atomic<uint64_t> m_completed;
+  uint64_t m_completed;
   /// Total amount of work, UINT64_MAX for non deterministic progress.
   const uint64_t m_total;
   /// The optional debugger ID to report progress to. If this has no value then
   /// all debuggers will receive this event.
   std::optional<lldb::user_id_t> m_debugger_id;
-
-  std::mutex m_update_mutex;
-  std::string m_update;
-
-  std::promise<void> m_stop_reporting_thread;
-  HostThread m_reporting_thread;
+  /// Set to true when progress has been reported where m_completed == m_total
+  /// to ensure that we don't send progress updates after progress has
+  /// completed.
+  bool m_complete = false;
 };
 
 } // namespace lldb_private

diff  --git a/lldb/source/Core/Progress.cpp b/lldb/source/Core/Progress.cpp
index a531b9b70437e..08be73f1470f3 100644
--- a/lldb/source/Core/Progress.cpp
+++ b/lldb/source/Core/Progress.cpp
@@ -9,8 +9,6 @@
 #include "lldb/Core/Progress.h"
 
 #include "lldb/Core/Debugger.h"
-#include "lldb/Host/ThreadLauncher.h"
-#include "lldb/Utility/LLDBLog.h"
 #include "lldb/Utility/StreamString.h"
 
 using namespace lldb;
@@ -24,67 +22,39 @@ Progress::Progress(std::string title, uint64_t total,
   assert(total > 0);
   if (debugger)
     m_debugger_id = debugger->GetID();
-
-  // Using a shared_future because std::function needs to be copyable.
-  if (llvm::Expected<HostThread> reporting_thread =
-          ThreadLauncher::LaunchThread(
-              "<lldb.progress>",
-              [this, future = std::shared_future<void>(
-                         m_stop_reporting_thread.get_future())]() {
-                SendPeriodicReports(future);
-                return lldb::thread_result_t();
-              })) {
-    m_reporting_thread = std::move(*reporting_thread);
-  } else {
-    LLDB_LOG_ERROR(GetLog(LLDBLog::Host), reporting_thread.takeError(),
-                   "failed to launch host thread: {}");
-  }
+  std::lock_guard<std::mutex> guard(m_mutex);
+  ReportProgress();
 }
 
 Progress::~Progress() {
-  m_stop_reporting_thread.set_value();
-  if (m_reporting_thread.IsJoinable()) {
-    m_reporting_thread.Join(nullptr);
+  // Make sure to always report progress completed when this object is
+  // destructed so it indicates the progress dialog/activity should go away.
+  std::lock_guard<std::mutex> guard(m_mutex);
+  if (!m_completed) {
+    m_completed = m_total;
+    ReportProgress();
   }
 }
 
-void Progress::SendPeriodicReports(std::shared_future<void> done) {
-  uint64_t last_completed = 0;
-  Debugger::ReportProgress(m_id, m_title, "", last_completed, m_total,
-                           m_debugger_id);
-
-  while (last_completed != m_total &&
-         done.wait_for(std::chrono::milliseconds(100)) ==
-             std::future_status::timeout) {
-    uint64_t current_completed = m_completed.load();
-    if (current_completed == last_completed)
-      continue;
-
-    if (current_completed == m_total ||
-        current_completed < last_completed /*overflow*/) {
-      break;
-    }
-
-    std::string current_update;
-    {
-      std::lock_guard<std::mutex> guard(m_update_mutex);
-      current_update = std::move(m_update);
-      m_update.clear();
-    }
-    Debugger::ReportProgress(m_id, m_title, std::move(current_update),
-                             current_completed, m_total, m_debugger_id);
-    last_completed = current_completed;
+void Progress::Increment(uint64_t amount, std::string update) {
+  if (amount > 0) {
+    std::lock_guard<std::mutex> guard(m_mutex);
+    // Watch out for unsigned overflow and make sure we don't increment too
+    // much and exceed m_total.
+    if (amount > (m_total - m_completed))
+      m_completed = m_total;
+    else
+      m_completed += amount;
+    ReportProgress(update);
   }
-
-  Debugger::ReportProgress(m_id, m_title, "", m_total, m_total, m_debugger_id);
 }
 
-void Progress::Increment(uint64_t amount, std::string update) {
-  if (amount == 0)
-    return;
-  if (!update.empty()) {
-    std::lock_guard<std::mutex> guard(m_update_mutex);
-    m_update = std::move(update);
+void Progress::ReportProgress(std::string update) {
+  if (!m_complete) {
+    // Make sure we only send one notification that indicates the progress is
+    // complete.
+    m_complete = m_completed == m_total;
+    Debugger::ReportProgress(m_id, m_title, std::move(update), m_completed,
+                             m_total, m_debugger_id);
   }
-  m_completed += amount;
 }