[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 10:07:06 PST 2023
https://github.com/chelcassanova updated https://github.com/llvm/llvm-project/pull/69516
>From 004e0e6a8a09b34bdc694ca7eb5ef02483693e5f 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 ++--
lldb/source/Symbol/LocateSymbolFile.cpp | 8 ++++--
.../TestProgressReporting.py | 13 +++++++--
11 files changed, 82 insertions(+), 24 deletions(-)
diff --git a/lldb/include/lldb/Core/Debugger.h b/lldb/include/lldb/Core/Debugger.h
index 5532cace606bfed..462c033e88af623 100644
--- a/lldb/include/lldb/Core/Debugger.h
+++ b/lldb/include/lldb/Core/Debugger.h
@@ -615,10 +615,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 7ec1efc64fe9383..b247ebbcaf37a19 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"
@@ -1402,22 +1403,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
@@ -1426,7 +1429,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
@@ -1436,7 +1439,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 08be73f1470f349..00d3d1a3d98145b 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();
@@ -55,6 +56,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 3e52c9e3c042811..ccbb28f24c8f101 100644
--- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -2219,7 +2219,8 @@ void ObjectFileMachO::ParseSymtab(Symtab &symtab) {
const FileSpec &file = m_file ? m_file : module_sp->GetFileSpec();
const char *file_name = file.GetFilename().AsCString("<Unknown>");
LLDB_SCOPED_TIMERF("ObjectFileMachO::ParseSymtab () module = %s", 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 57b962ff60df03d..3054e4b3e70a588 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
@@ -76,7 +76,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 02037e7403cd9ad..9397792605e7aaa 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -457,7 +457,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());
@@ -470,7 +471,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/source/Symbol/LocateSymbolFile.cpp b/lldb/source/Symbol/LocateSymbolFile.cpp
index 66ee7589ac60499..2071fbad1ab2eff 100644
--- a/lldb/source/Symbol/LocateSymbolFile.cpp
+++ b/lldb/source/Symbol/LocateSymbolFile.cpp
@@ -267,9 +267,11 @@ Symbols::LocateExecutableSymbolFile(const ModuleSpec &module_spec,
FileSystem::Instance().Exists(symbol_file_spec))
return symbol_file_spec;
- Progress progress(llvm::formatv(
- "Locating external symbol file for {0}",
- module_spec.GetFileSpec().GetFilename().AsCString("<Unknown>")));
+ Progress progress(
+ llvm::formatv(
+ "Locating external symbol file for {0}",
+ module_spec.GetFileSpec().GetFilename().AsCString("<Unknown>")),
+ Progress::ProgressReportType::eAggregateProgressReport);
FileSpecList debug_file_search_paths = default_search_paths;
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