[lldb][progress] Add discrete boolean flag to progress reports

This commit adds a boolean flag `is_discrete` is to progress reports in
LLDB. The flag is set to false by default and indicates if a progress event is discrete, i.e. an
operation that has no clear start and end and can happen multiple times
during the course of a debug session. Operations that happen
in this manner will report multiple individual progress events as they
happen, so this flag gives the functionality to group multiple progress
events so they can be reported in a less haphazard manner.

Author: chelcassanova

lldb/include/lldb/Core/Debugger.h

@@ -618,10 +618,16 @@ 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.
+  ///
+  ///  \param [in] report_type
+  ///   Indicates whether the operation for which this progress reporting is
+  ///   reporting on will happen as an aggregate of multiple individual
+  ///   progress reports or not.
   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);
+                             std::optional<lldb::user_id_t> debugger_id,
+                             Progress::ProgressReportType report_type);
 
   static void ReportDiagnosticImpl(DiagnosticEventData::Type type,
                                    std::string message,

lldb/include/lldb/Core/DebuggerEvents.h

@@ -7,6 +7,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "lldb/Core/ModuleSpec.h"
+#include "lldb/Core/Progress.h"
 #include "lldb/Utility/Event.h"
 #include "lldb/Utility/StructuredData.h"
 
@@ -21,10 +22,11 @@ class Stream;
 class ProgressEventData : public EventData {
 public:
   ProgressEventData(uint64_t progress_id, std::string title, std::string update,
-                    uint64_t completed, uint64_t total, bool debugger_specific)
+                    uint64_t completed, uint64_t total, bool debugger_specific,
+                    Progress::ProgressReportType report_type)
       : 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) {}
+        m_debugger_specific(debugger_specific), m_report_type(report_type) {}
 
   static llvm::StringRef GetFlavorString();
 
@@ -52,6 +54,10 @@ class ProgressEventData : public EventData {
   const std::string &GetTitle() const { return m_title; }
   const std::string &GetDetails() const { return m_details; }
   bool IsDebuggerSpecific() const { return m_debugger_specific; }
+  bool IsAggregate() const {
+    return m_report_type ==
+           Progress::ProgressReportType::eAggregateProgressReport;
+  }
 
 private:
   /// The title of this progress event. The value is expected to remain stable
@@ -68,6 +74,7 @@ class ProgressEventData : public EventData {
   uint64_t m_completed;
   const uint64_t m_total;
   const bool m_debugger_specific;
+  const Progress::ProgressReportType m_report_type;
   ProgressEventData(const ProgressEventData &) = delete;
   const ProgressEventData &operator=(const ProgressEventData &) = delete;
 };

lldb/include/lldb/Core/Progress.h

@@ -55,6 +55,11 @@ namespace lldb_private {
 
 class Progress {
 public:
+  /// Enum that indicates the type of progress report
+  enum class ProgressReportType {
+    eAggregateProgressReport,
+    eNonAggregateProgressReport
+  };
   /// Construct a progress object that will report information.
   ///
   /// The constructor will create a unique progress reporting object and
@@ -63,13 +68,30 @@ class Progress {
   ///
   /// @param [in] title The title of this progress activity.
   ///
-  /// @param [in] total The total units of work to be done if specified, if
-  /// set to UINT64_MAX then an indeterminate progress indicator should be
+  /// @param [in] report_type Enum value indicating how the progress is being
+  /// reported. Progress reports considered "aggregate" are reports done for
+  /// operations that may happen multiple times during a debug session.
+  ///
+  /// For example, when a debug session is first started it needs to parse the
+  /// symbol tables for all files that were initially included and this
+  /// operation will deliver progress reports. If new symbol tables need to be
+  /// parsed later during the session then these will also be parsed and deliver
+  /// more progress reports. This type of operation would use the
+  /// eAggregateProgressReport enum. Using this enum would allow these progress
+  /// reports to be grouped together as one, even though their reports are
+  /// happening individually.
+  ///
+  /// @param [in] total Optional total units of work to be done if specified, if
+  /// unset then an indeterminate progress indicator should be
   /// displayed.
   ///
   /// @param [in] debugger An optional debugger pointer to specify that this
   /// progress is to be reported only to specific debuggers.
-  Progress(std::string title, uint64_t total = UINT64_MAX,
+  ///
+  Progress(std::string title,
+           ProgressReportType report_type =
+               ProgressReportType::eNonAggregateProgressReport,
+           std::optional<uint64_t> total = std::nullopt,
            lldb_private::Debugger *debugger = nullptr);
 
   /// Destroy the progress object.
@@ -97,12 +119,17 @@ class Progress {
   /// The title of the progress activity.
   std::string m_title;
   std::mutex m_mutex;
+  /// Set to eAggregateProgressReport if the progress event is aggregate;
+  /// meaning it will happen multiple times during a debug session as individual
+  /// progress events
+  ProgressReportType m_report_type =
+      ProgressReportType::eNonAggregateProgressReport;
   /// 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;
-  /// Total amount of work, UINT64_MAX for non deterministic progress.
-  const uint64_t m_total;
+  /// Total amount of work, use a std::nullopt for non deterministic progress.
+  const std::optional<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;

lldb/source/Core/Debugger.cpp

@@ -15,6 +15,7 @@
 #include "lldb/Core/ModuleList.h"
 #include "lldb/Core/ModuleSpec.h"
 #include "lldb/Core/PluginManager.h"
+#include "lldb/Core/Progress.h"
 #include "lldb/Core/StreamAsynchronousIO.h"
 #include "lldb/DataFormatters/DataVisualization.h"
 #include "lldb/Expression/REPL.h"
@@ -1421,22 +1422,24 @@ void Debugger::SetDestroyCallback(
 static void PrivateReportProgress(Debugger &debugger, uint64_t progress_id,
                                   std::string title, std::string details,
                                   uint64_t completed, uint64_t total,
-                                  bool is_debugger_specific) {
+                                  bool is_debugger_specific,
+                                  Progress::ProgressReportType report_type) {
   // Only deliver progress events if we have any progress listeners.
   const uint32_t event_type = Debugger::eBroadcastBitProgress;
   if (!debugger.GetBroadcaster().EventTypeHasListeners(event_type))
     return;
   EventSP event_sp(new Event(
-      event_type,
-      new ProgressEventData(progress_id, std::move(title), std::move(details),
-                            completed, total, is_debugger_specific)));
+      event_type, new ProgressEventData(progress_id, std::move(title),
+                                        std::move(details), completed, total,
+                                        is_debugger_specific, report_type)));
   debugger.GetBroadcaster().BroadcastEvent(event_sp);
 }
 
 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) {
+                              std::optional<lldb::user_id_t> debugger_id,
+                              Progress::ProgressReportType report_type) {
   // Check if this progress is for a specific debugger.
   if (debugger_id) {
     // It is debugger specific, grab it and deliver the event if the debugger
@@ -1445,7 +1448,7 @@ void Debugger::ReportProgress(uint64_t progress_id, std::string title,
     if (debugger_sp)
       PrivateReportProgress(*debugger_sp, progress_id, std::move(title),
                             std::move(details), completed, total,
-                            /*is_debugger_specific*/ true);
+                            /*is_debugger_specific*/ true, report_type);
     return;
   }
   // The progress event is not debugger specific, iterate over all debuggers
@@ -1455,7 +1458,7 @@ void Debugger::ReportProgress(uint64_t progress_id, std::string title,
     DebuggerList::iterator pos, end = g_debugger_list_ptr->end();
     for (pos = g_debugger_list_ptr->begin(); pos != end; ++pos)
       PrivateReportProgress(*(*pos), progress_id, title, details, completed,
-                            total, /*is_debugger_specific*/ false);
+                            total, /*is_debugger_specific*/ false, report_type);
   }
 }
 

lldb/source/Core/DebuggerEvents.cpp

@@ -67,6 +67,7 @@ ProgressEventData::GetAsStructuredData(const Event *event_ptr) {
   dictionary_sp->AddIntegerItem("total", progress_data->GetTotal());
   dictionary_sp->AddBooleanItem("debugger_specific",
                                 progress_data->IsDebuggerSpecific());
+  dictionary_sp->AddBooleanItem("is_aggregate", progress_data->IsAggregate());
 
   return dictionary_sp;
 }

lldb/source/Core/Progress.cpp

@@ -11,14 +11,18 @@
 #include "lldb/Core/Debugger.h"
 #include "lldb/Utility/StreamString.h"
 
+#include <optional>
+
 using namespace lldb;
 using namespace lldb_private;
 
 std::atomic<uint64_t> Progress::g_id(0);
 
-Progress::Progress(std::string title, uint64_t total,
+Progress::Progress(std::string title, ProgressReportType report_type,
+                   std::optional<uint64_t> total,
                    lldb_private::Debugger *debugger)
-    : m_title(title), m_id(++g_id), m_completed(0), m_total(total) {
+    : m_title(title), m_report_type(report_type), m_id(++g_id), m_completed(0),
+      m_total(total) {
   assert(total > 0);
   if (debugger)
     m_debugger_id = debugger->GetID();
@@ -31,7 +35,7 @@ Progress::~Progress() {
   // 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;
+    m_completed = m_total.value();
   ReportProgress();
 }
 
@@ -40,8 +44,8 @@ void Progress::Increment(uint64_t amount, std::string update) {
     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;
+    if (amount > (m_total.value() - m_completed))
+      m_completed = m_total.value();
     else
       m_completed += amount;
     ReportProgress(update);
@@ -52,8 +56,8 @@ 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;
+    m_complete = m_completed == m_total.value();
     Debugger::ReportProgress(m_id, m_title, std::move(update), m_completed,
-                             m_total, m_debugger_id);
+                             m_total.value(), m_debugger_id, m_report_type);
   }
 }

lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp

@@ -2225,7 +2225,8 @@ void ObjectFileMachO::ParseSymtab(Symtab &symtab) {
   const char *file_name = file.GetFilename().AsCString("<Unknown>");
   LLDB_SCOPED_TIMERF("ObjectFileMachO::ParseSymtab () module = %s", file_name);
   LLDB_LOG(log, "Parsing symbol table for {0}", file_name);
-  Progress progress(llvm::formatv("Parsing symbol table for {0}", file_name));
+  Progress progress(llvm::formatv("Parsing symbol table for {0}", file_name),
+                    Progress::ProgressReportType::eAggregateProgressReport);
 
   llvm::MachO::symtab_command symtab_load_command = {0, 0, 0, 0, 0, 0};
   llvm::MachO::linkedit_data_command function_starts_load_command = {0, 0, 0, 0};

lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp

@@ -77,7 +77,7 @@ void ManualDWARFIndex::Index() {
   const uint64_t total_progress = units_to_index.size() * 2 + 8;
   Progress progress(
       llvm::formatv("Manually indexing DWARF for {0}", module_desc.GetData()),
-      total_progress);
+      Progress::ProgressReportType::eAggregateProgressReport, total_progress);
 
   std::vector<IndexSet> sets(units_to_index.size());
 

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

@@ -472,7 +472,8 @@ void SymbolFileDWARF::InitializeObject() {
     if (apple_names.GetByteSize() > 0 || apple_namespaces.GetByteSize() > 0 ||
         apple_types.GetByteSize() > 0 || apple_objc.GetByteSize() > 0) {
       Progress progress(llvm::formatv("Loading Apple DWARF index for {0}",
-                                      module_desc.GetData()));
+                                      module_desc.GetData()),
+                        Progress::ProgressReportType::eAggregateProgressReport);
       m_index = AppleDWARFIndex::Create(
           *GetObjectFile()->GetModule(), apple_names, apple_namespaces,
           apple_types, apple_objc, m_context.getOrLoadStrData());
@@ -485,7 +486,8 @@ void SymbolFileDWARF::InitializeObject() {
     LoadSectionData(eSectionTypeDWARFDebugNames, debug_names);
     if (debug_names.GetByteSize() > 0) {
       Progress progress(
-          llvm::formatv("Loading DWARF5 index for {0}", module_desc.GetData()));
+          llvm::formatv("Loading DWARF5 index for {0}", module_desc.GetData()),
+          Progress::ProgressReportType::eAggregateProgressReport);
       llvm::Expected<std::unique_ptr<DebugNamesDWARFIndex>> index_or =
           DebugNamesDWARFIndex::Create(*GetObjectFile()->GetModule(),
                                        debug_names,

lldb/test/API/functionalities/progress_reporting/TestProgressReporting.py

@@ -37,5 +37,14 @@ def test_dwarf_symbol_loading_progress_report_structured_data(self):
 
         event = lldbutil.fetch_next_event(self, self.listener, self.broadcaster)
         progress_data = lldb.SBDebugger.GetProgressDataFromEvent(event)
-        message = progress_data.GetValueForKey("message").GetStringValue(100)
-        self.assertGreater(len(message), 0)
+        title = progress_data.GetValueForKey("title").GetStringValue(100)
+        self.assertGreater(len(title), 0)
+
+        is_aggregate = progress_data.GetValueForKey("is_aggregate")
+        self.assertTrue(
+            is_aggregate.IsValid(),
+            "ProgressEventData key 'is_aggregate' does not exist.",
+        )
+        self.assertTrue(
+            is_aggregate, "ProgressEventData key 'is_aggregate' should be true."
+        )