[Lldb-commits] [lldb] 129eb5b - [lldb] Add the ability to provide a message to a progress event update

Jonas Devlieghere via lldb-commits lldb-commits at lists.llvm.org
Sun Feb 12 11:19:48 PST 2023


Author: Jonas Devlieghere
Date: 2023-02-12T11:17:58-08:00
New Revision: 129eb5bcab91a12ed3c4712279f201834ae2d8e1

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

LOG: [lldb] Add the ability to provide a message to a progress event update

Consider the following example as motivation. Say you have to load
symbols for 3 dynamic libraries: `libFoo`, `libBar` and `libBaz`.
Currently, there are two ways to report process for this operation:

 1. As 3 separate progress instances. In this case you create a progress
    instance with the message "Loading symbols: libFoo", "Loading
    symbols: libBar", and "Loading symbols: libBaz" respectively. Each
    progress event gets a unique ID and therefore cannot be correlated
    by the consumer.

 2. As 1 progress instance with 3 units of work. The title would be
    "Loading symbols" and you call Progress::Increment for each of the
    libraries. The 3 progress events share the same ID and can easily be
    correlated, however, in the current design, there's no way to
    include the name of the libraries.

The second approach is preferred when the amount of work is known in
advance, because determinate progress can be reported (i.e. x out of y
operations completed). An additional benefit is that the progress
consumer can decide to ignore certain progress updates by their ID if
they are deemed to noisy, which isn't trivial for the first approach due
to the use of different progress IDs.

This patch adds the ability to add a message (detail) to a progress
event update. For the example described above, progress can now be
displayed as shown:

  [1/3] Loading symbols: libFoo
  [2/3] Loading symbols: libBar
  [3/3] Loading symbols: libBaz

Differential revision: https://reviews.llvm.org/D143690

Added: 
    

Modified: 
    lldb/include/lldb/Core/Debugger.h
    lldb/include/lldb/Core/DebuggerEvents.h
    lldb/include/lldb/Core/Progress.h
    lldb/source/API/SBDebugger.cpp
    lldb/source/Core/Debugger.cpp
    lldb/source/Core/DebuggerEvents.cpp
    lldb/source/Core/Progress.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/include/lldb/Core/Debugger.h b/lldb/include/lldb/Core/Debugger.h
index dd3e6c061fcf6..9f83a37bc75cc 100644
--- a/lldb/include/lldb/Core/Debugger.h
+++ b/lldb/include/lldb/Core/Debugger.h
@@ -484,8 +484,9 @@ class Debugger : public std::enable_shared_from_this<Debugger>,
   ///   debugger identifier that this progress should be delivered to. If this
   ///   optional parameter does not have a value, the progress will be
   ///   delivered to all debuggers.
-  static void ReportProgress(uint64_t progress_id, const std::string &message,
-                             uint64_t completed, uint64_t total,
+  static void ReportProgress(uint64_t progress_id, std::string title,
+                             std::string details, uint64_t completed,
+                             uint64_t total,
                              std::optional<lldb::user_id_t> debugger_id);
 
   static void ReportDiagnosticImpl(DiagnosticEventData::Type type,

diff  --git a/lldb/include/lldb/Core/DebuggerEvents.h b/lldb/include/lldb/Core/DebuggerEvents.h
index 0dade0f9629a0..f9ae67efac323 100644
--- a/lldb/include/lldb/Core/DebuggerEvents.h
+++ b/lldb/include/lldb/Core/DebuggerEvents.h
@@ -21,10 +21,11 @@ class Stream;
 
 class ProgressEventData : public EventData {
 public:
-  ProgressEventData(uint64_t progress_id, const std::string &message,
+  ProgressEventData(uint64_t progress_id, std::string title, std::string update,
                     uint64_t completed, uint64_t total, bool debugger_specific)
-      : m_message(message), m_id(progress_id), m_completed(completed),
-        m_total(total), m_debugger_specific(debugger_specific) {}
+      : m_title(std::move(title)), m_details(std::move(update)),
+        m_id(progress_id), m_completed(completed), m_total(total),
+        m_debugger_specific(debugger_specific) {}
 
   static ConstString GetFlavorString();
 
@@ -41,12 +42,30 @@ class ProgressEventData : public EventData {
   bool IsFinite() const { return m_total != UINT64_MAX; }
   uint64_t GetCompleted() const { return m_completed; }
   uint64_t GetTotal() const { return m_total; }
-  const std::string &GetMessage() const { return m_message; }
+  std::string GetMessage() const {
+    std::string message = m_title;
+    if (!m_details.empty()) {
+      message.append(": ");
+      message.append(m_details);
+    }
+    return message;
+  }
+  const std::string &GetTitle() const { return m_title; }
+  const std::string &GetDetails() const { return m_details; }
   bool IsDebuggerSpecific() const { return m_debugger_specific; }
 
 private:
-  std::string m_message;
+  /// The title of this progress event. The value is expected to remain stable
+  /// for a given progress ID.
+  std::string m_title;
+
+  /// Details associated with this progress event update. The value is expected
+  /// to change between progress events.
+  std::string m_details;
+
+  /// Unique ID used to associate progress events.
   const uint64_t m_id;
+
   uint64_t m_completed;
   const uint64_t m_total;
   const bool m_debugger_specific;

diff  --git a/lldb/include/lldb/Core/Progress.h b/lldb/include/lldb/Core/Progress.h
index 48078705ae6b8..b2b8781a43b05 100644
--- a/lldb/include/lldb/Core/Progress.h
+++ b/lldb/include/lldb/Core/Progress.h
@@ -87,10 +87,12 @@ class Progress {
   /// anything nor send any progress updates.
   ///
   /// @param [in] amount The amount to increment m_completed by.
-  void Increment(uint64_t amount = 1);
+  ///
+  /// @param [in] an optional message associated with this update.
+  void Increment(uint64_t amount = 1, std::string update = {});
 
 private:
-  void ReportProgress();
+  void ReportProgress(std::string update = {});
   static std::atomic<uint64_t> g_id;
   /// The title of the progress activity.
   std::string m_title;

diff  --git a/lldb/source/API/SBDebugger.cpp b/lldb/source/API/SBDebugger.cpp
index 2fa93e47a44cf..641c84f0bd159 100644
--- a/lldb/source/API/SBDebugger.cpp
+++ b/lldb/source/API/SBDebugger.cpp
@@ -157,6 +157,7 @@ const char *SBDebugger::GetProgressFromEvent(const lldb::SBEvent &event,
                                              uint64_t &total,
                                              bool &is_debugger_specific) {
   LLDB_INSTRUMENT_VA(event);
+
   const ProgressEventData *progress_data =
       ProgressEventData::GetEventDataFromEvent(event.get());
   if (progress_data == nullptr)
@@ -165,7 +166,8 @@ const char *SBDebugger::GetProgressFromEvent(const lldb::SBEvent &event,
   completed = progress_data->GetCompleted();
   total = progress_data->GetTotal();
   is_debugger_specific = progress_data->IsDebuggerSpecific();
-  return progress_data->GetMessage().c_str();
+  ConstString message(progress_data->GetMessage());
+  return message.AsCString();
 }
 
 lldb::SBStructuredData

diff  --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp
index 8a8a01c70ce66..9c09da429d401 100644
--- a/lldb/source/Core/Debugger.cpp
+++ b/lldb/source/Core/Debugger.cpp
@@ -1286,7 +1286,7 @@ void Debugger::SetLoggingCallback(lldb::LogOutputCallback log_callback,
 }
 
 static void PrivateReportProgress(Debugger &debugger, uint64_t progress_id,
-                                  const std::string &message,
+                                  std::string title, std::string details,
                                   uint64_t completed, uint64_t total,
                                   bool is_debugger_specific) {
   // Only deliver progress events if we have any progress listeners.
@@ -1294,13 +1294,15 @@ static void PrivateReportProgress(Debugger &debugger, uint64_t progress_id,
   if (!debugger.GetBroadcaster().EventTypeHasListeners(event_type))
     return;
   EventSP event_sp(new Event(
-      event_type, new ProgressEventData(progress_id, message, completed, total,
-                                        is_debugger_specific)));
+      event_type,
+      new ProgressEventData(progress_id, std::move(title), std::move(details),
+                            completed, total, is_debugger_specific)));
   debugger.GetBroadcaster().BroadcastEvent(event_sp);
 }
 
-void Debugger::ReportProgress(uint64_t progress_id, const std::string &message,
-                              uint64_t completed, uint64_t total,
+void Debugger::ReportProgress(uint64_t progress_id, std::string title,
+                              std::string details, uint64_t completed,
+                              uint64_t total,
                               std::optional<lldb::user_id_t> debugger_id) {
   // Check if this progress is for a specific debugger.
   if (debugger_id) {
@@ -1308,8 +1310,9 @@ void Debugger::ReportProgress(uint64_t progress_id, const std::string &message,
     // still exists.
     DebuggerSP debugger_sp = FindDebuggerWithID(*debugger_id);
     if (debugger_sp)
-      PrivateReportProgress(*debugger_sp, progress_id, message, completed,
-                            total, /*is_debugger_specific*/ true);
+      PrivateReportProgress(*debugger_sp, progress_id, std::move(title),
+                            std::move(details), completed, total,
+                            /*is_debugger_specific*/ true);
     return;
   }
   // The progress event is not debugger specific, iterate over all debuggers
@@ -1318,8 +1321,8 @@ void Debugger::ReportProgress(uint64_t progress_id, const std::string &message,
     std::lock_guard<std::recursive_mutex> guard(*g_debugger_list_mutex_ptr);
     DebuggerList::iterator pos, end = g_debugger_list_ptr->end();
     for (pos = g_debugger_list_ptr->begin(); pos != end; ++pos)
-      PrivateReportProgress(*(*pos), progress_id, message, completed, total,
-                            /*is_debugger_specific*/ false);
+      PrivateReportProgress(*(*pos), progress_id, title, details, completed,
+                            total, /*is_debugger_specific*/ false);
   }
 }
 

diff  --git a/lldb/source/Core/DebuggerEvents.cpp b/lldb/source/Core/DebuggerEvents.cpp
index fd459f899abaa..1c38e9bbc22f8 100644
--- a/lldb/source/Core/DebuggerEvents.cpp
+++ b/lldb/source/Core/DebuggerEvents.cpp
@@ -33,7 +33,9 @@ ConstString ProgressEventData::GetFlavor() const {
 }
 
 void ProgressEventData::Dump(Stream *s) const {
-  s->Printf(" id = %" PRIu64 ", message = \"%s\"", m_id, m_message.c_str());
+  s->Printf(" id = %" PRIu64 ", title = \"%s\"", m_id, m_title.c_str());
+  if (!m_details.empty())
+    s->Printf(", details = \"%s\"", m_details.c_str());
   if (m_completed == 0 || m_completed == m_total)
     s->Printf(", type = %s", m_completed == 0 ? "start" : "end");
   else
@@ -58,6 +60,8 @@ ProgressEventData::GetAsStructuredData(const Event *event_ptr) {
     return {};
 
   auto dictionary_sp = std::make_shared<StructuredData::Dictionary>();
+  dictionary_sp->AddStringItem("title", progress_data->GetTitle());
+  dictionary_sp->AddStringItem("details", progress_data->GetDetails());
   dictionary_sp->AddStringItem("message", progress_data->GetMessage());
   dictionary_sp->AddIntegerItem("progress_id", progress_data->GetID());
   dictionary_sp->AddIntegerItem("completed", progress_data->GetCompleted());

diff  --git a/lldb/source/Core/Progress.cpp b/lldb/source/Core/Progress.cpp
index c54e7774adf3a..08be73f1470f3 100644
--- a/lldb/source/Core/Progress.cpp
+++ b/lldb/source/Core/Progress.cpp
@@ -36,7 +36,7 @@ Progress::~Progress() {
   }
 }
 
-void Progress::Increment(uint64_t amount) {
+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
@@ -45,16 +45,16 @@ void Progress::Increment(uint64_t amount) {
       m_completed = m_total;
     else
       m_completed += amount;
-    ReportProgress();
+    ReportProgress(update);
   }
 }
 
-void Progress::ReportProgress() {
+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, m_completed, m_total,
-                             m_debugger_id);
+    Debugger::ReportProgress(m_id, m_title, std::move(update), m_completed,
+                             m_total, m_debugger_id);
   }
 }


        


More information about the lldb-commits mailing list