[Lldb-commits] [lldb] [lldb][progress] Add discrete boolean flag to progress reports (PR #69516)

Chelsea Cassanova via lldb-commits lldb-commits at lists.llvm.org
Thu Nov 30 14:13:01 PST 2023


https://github.com/chelcassanova updated https://github.com/llvm/llvm-project/pull/69516

>From 417d99ff038517fed596ebf6605e203110e88e6f Mon Sep 17 00:00:00 2001
From: Chelsea Cassanova <chelsea_cassanova at apple.com>
Date: Wed, 18 Oct 2023 13:07:51 -0700
Subject: [PATCH] [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.
---
 lldb/include/lldb/Core/Debugger.h             |  8 +++++-
 lldb/include/lldb/Core/DebuggerEvents.h       | 11 ++++++--
 lldb/include/lldb/Core/Progress.h             | 28 ++++++++++++++++++-
 lldb/source/Core/Debugger.cpp                 | 17 ++++++-----
 lldb/source/Core/DebuggerEvents.cpp           |  1 +
 lldb/source/Core/Progress.cpp                 |  9 +++---
 .../ObjectFile/Mach-O/ObjectFileMachO.cpp     |  3 +-
 .../SymbolFile/DWARF/ManualDWARFIndex.cpp     |  2 +-
 .../SymbolFile/DWARF/SymbolFileDWARF.cpp      |  6 ++--
 .../TestProgressReporting.py                  | 13 +++++++--
 10 files changed, 77 insertions(+), 21 deletions(-)

diff --git a/lldb/include/lldb/Core/Debugger.h b/lldb/include/lldb/Core/Debugger.h
index e4ee94809cf1a09..395ac09a965e05b 100644
--- a/lldb/include/lldb/Core/Debugger.h
+++ b/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,
diff --git a/lldb/include/lldb/Core/DebuggerEvents.h b/lldb/include/lldb/Core/DebuggerEvents.h
index 982b22229701f89..dc933c47dcf53f0 100644
--- a/lldb/include/lldb/Core/DebuggerEvents.h
+++ b/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;
 };
diff --git a/lldb/include/lldb/Core/Progress.h b/lldb/include/lldb/Core/Progress.h
index b2b8781a43b0591..41343ce890ed887 100644
--- a/lldb/include/lldb/Core/Progress.h
+++ b/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] 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 dSYMs are added 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 The total units of work to be done if specified, if
   /// set to UINT64_MAX 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,
+           uint64_t total = UINT64_MAX,
            lldb_private::Debugger *debugger = nullptr);
 
   /// Destroy the progress object.
@@ -97,6 +119,10 @@ class Progress {
   /// The title of the progress activity.
   std::string m_title;
   std::mutex m_mutex;
+  /// Set to true 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.
diff --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp
index 21f71e449ca5ed0..e53bd264b334f00 100644
--- a/lldb/source/Core/Debugger.cpp
+++ b/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);
   }
 }
 
diff --git a/lldb/source/Core/DebuggerEvents.cpp b/lldb/source/Core/DebuggerEvents.cpp
index dd77fff349a64a7..73797bc668a395f 100644
--- a/lldb/source/Core/DebuggerEvents.cpp
+++ b/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;
 }
diff --git a/lldb/source/Core/Progress.cpp b/lldb/source/Core/Progress.cpp
index ea3f874916a999f..f4e4fde9d43ddfa 100644
--- a/lldb/source/Core/Progress.cpp
+++ b/lldb/source/Core/Progress.cpp
@@ -16,9 +16,10 @@ using namespace lldb_private;
 
 std::atomic<uint64_t> Progress::g_id(0);
 
-Progress::Progress(std::string title, uint64_t total,
-                   lldb_private::Debugger *debugger)
-    : m_title(title), m_id(++g_id), m_completed(0), m_total(total) {
+Progress::Progress(std::string title, ProgressReportType report_type,
+                   uint64_t total, lldb_private::Debugger *debugger)
+    : 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();
@@ -54,6 +55,6 @@ void Progress::ReportProgress(std::string update) {
     // complete.
     m_complete = m_completed == m_total;
     Debugger::ReportProgress(m_id, m_title, std::move(update), m_completed,
-                             m_total, m_debugger_id);
+                             m_total, m_debugger_id, m_report_type);
   }
 }
diff --git a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
index 24f3939a8f2ba5a..253ceddc7b24e58 100644
--- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ b/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};
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp b/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
index 16ff5f7d4842cae..afd71592148c9af 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
+++ b/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());
 
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index b8b2eb58a8bd85c..64eeec7311e4e82 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/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,
diff --git a/lldb/test/API/functionalities/progress_reporting/TestProgressReporting.py b/lldb/test/API/functionalities/progress_reporting/TestProgressReporting.py
index 0e72770e350366d..634c0fb07180774 100644
--- a/lldb/test/API/functionalities/progress_reporting/TestProgressReporting.py
+++ b/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."
+        )



More information about the lldb-commits mailing list