https://github.com/chelcassanova updated
https://github.com/llvm/llvm-project/pull/69516
>From 06e9b990b3513443e563a91b33ceab07fdbc952b Mon Sep 17 00:00:00 2001
From: Chelsea Cassanova <chelsea_cassan...@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 | 37 ++++++++++++++++---
lldb/source/Core/Debugger.cpp | 17 +++++----
lldb/source/Core/DebuggerEvents.cpp | 1 +
lldb/source/Core/Progress.cpp | 18 +++++----
.../ObjectFile/Mach-O/ObjectFileMachO.cpp | 3 +-
.../SymbolFile/DWARF/ManualDWARFIndex.cpp | 2 +-
.../SymbolFile/DWARF/SymbolFileDWARF.cpp | 6 ++-
.../TestProgressReporting.py | 13 ++++++-
10 files changed, 88 insertions(+), 28 deletions(-)
diff --git a/lldb/include/lldb/Core/Debugger.h
b/lldb/include/lldb/Core/Debugger.h
index e4ee94809cf1a..395ac09a965e0 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 982b22229701f..dc933c47dcf53 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 b2b8781a43b05..363aff3b2162e 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] 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;
diff --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp
index 21f71e449ca5e..e53bd264b334f 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 dd77fff349a64..73797bc668a39 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 ea3f874916a99..7e1330b569320 100644
--- a/lldb/source/Core/Progress.cpp
+++ b/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);
}
}
diff --git a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
index 24f3939a8f2ba..253ceddc7b24e 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 16ff5f7d4842c..afd71592148c9 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 b8b2eb58a8bd8..64eeec7311e4e 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 0e72770e35036..634c0fb071807 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."
+ )
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
