[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

diff --git a/lldb/include/lldb/Core/Debugger.h b/lldb/include/lldb/Core/Debugger.h
index dd3e6c0..9f83a37 100644 (file)
--- a/lldb/include/lldb/Core/Debugger.h
+++ b/lldb/include/lldb/Core/Debugger.h
@@ -484,8 +484,9 @@ protected:
   ///   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 0dade0f..f9ae67e 100644 (file)
--- 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 @@ public:
   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 4807870..b2b8781 100644 (file)
--- a/lldb/include/lldb/Core/Progress.h
+++ b/lldb/include/lldb/Core/Progress.h
@@ -87,10 +87,12 @@ public:
   /// 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 2fa93e4..641c84f 100644 (file)
--- 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 8a8a01c..9c09da4 100644 (file)
--- 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 fd459f8..1c38e9b 100644 (file)
--- 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 c54e777..08be73f 100644 (file)
--- 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);
   }
 }