[Lldb-commits] [lldb] c308534 - [lldb] Rate limit progress reports -- different approach [WIP-ish]

Pavel Labath via lldb-commits lldb-commits at lists.llvm.org
Fri Jun 16 00:09:23 PDT 2023


Author: Pavel Labath
Date: 2023-06-16T08:28:29+02:00
New Revision: c30853460da7446f92bc1e516f9cbe2c5df6e136

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

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

Have the Progress class spawn a thread to periodically send progress
reports.

The reporting period could be made configurable, but for now I've
hardcoded it to 100ms. (This is the main WIP part)

It could be argued that creating a thread for progress reporting adds
overhead, but I would counter that by saying "If the task is so fast
that creating a thread noticably slows it down, then it really doesn't
need progress reporting".

For me, this speeds up DWARF indexing by about 1.5% (which is only
slightly above the error bars), but I expect it will have a much bigger
impact in situations where printing a single progress update takes a
nontrivial amount of time.

Differential Revision: https://reviews.llvm.org/D152364

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 b2b8781a43b05..3bafd3d4b34ea 100644
--- a/lldb/include/lldb/Core/Progress.h
+++ b/lldb/include/lldb/Core/Progress.h
@@ -9,9 +9,11 @@
 #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>
 
@@ -92,24 +94,26 @@ class Progress {
   void Increment(uint64_t amount = 1, std::string update = {});
 
 private:
-  void ReportProgress(std::string update = {});
+  void SendPeriodicReports(std::shared_future<void> done);
+  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.
-  uint64_t m_completed;
+  std::atomic<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;
-  /// 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;
+
+  std::mutex m_update_mutex;
+  std::string m_update;
+
+  std::promise<void> m_stop_reporting_thread;
+  HostThread m_reporting_thread;
 };
 
 } // namespace lldb_private

diff  --git a/lldb/source/Core/Progress.cpp b/lldb/source/Core/Progress.cpp
index 08be73f1470f3..a531b9b70437e 100644
--- a/lldb/source/Core/Progress.cpp
+++ b/lldb/source/Core/Progress.cpp
@@ -9,6 +9,8 @@
 #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;
@@ -22,39 +24,67 @@ Progress::Progress(std::string title, uint64_t total,
   assert(total > 0);
   if (debugger)
     m_debugger_id = debugger->GetID();
-  std::lock_guard<std::mutex> guard(m_mutex);
-  ReportProgress();
+
+  // 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: {}");
+  }
 }
 
 Progress::~Progress() {
-  // 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();
+  m_stop_reporting_thread.set_value();
+  if (m_reporting_thread.IsJoinable()) {
+    m_reporting_thread.Join(nullptr);
   }
 }
 
-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);
+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;
   }
+
+  Debugger::ReportProgress(m_id, m_title, "", m_total, m_total, m_debugger_id);
 }
 
-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);
+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);
   }
+  m_completed += amount;
 }


        


More information about the lldb-commits mailing list