[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 Oct 19 11:42:56 PDT 2023
https://github.com/chelcassanova updated https://github.com/llvm/llvm-project/pull/69516
>From 073ba299ab15c487bff28212563b5a103bdc5f60 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 1/5] [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 | 2 +-
lldb/include/lldb/Core/DebuggerEvents.h | 7 +++++--
lldb/include/lldb/Core/Progress.h | 10 +++++++++-
lldb/source/Core/Debugger.cpp | 16 +++++++++-------
lldb/source/Core/DebuggerEvents.cpp | 1 +
lldb/source/Core/Progress.cpp | 4 ++--
.../ObjectFile/Mach-O/ObjectFileMachO.cpp | 2 +-
.../SymbolFile/DWARF/ManualDWARFIndex.cpp | 2 +-
.../Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp | 4 ++--
lldb/source/Symbol/LocateSymbolFile.cpp | 2 +-
.../progress_reporting/TestProgressReporting.py | 11 +++++++----
11 files changed, 39 insertions(+), 22 deletions(-)
diff --git a/lldb/include/lldb/Core/Debugger.h b/lldb/include/lldb/Core/Debugger.h
index 5532cace606bfed..8e21502dac6dee2 100644
--- a/lldb/include/lldb/Core/Debugger.h
+++ b/lldb/include/lldb/Core/Debugger.h
@@ -618,7 +618,7 @@ class Debugger : public std::enable_shared_from_this<Debugger>,
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, bool is_discrete);
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..88455d8d60bb488 100644
--- 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, std::string title, std::string update,
- uint64_t completed, uint64_t total, bool debugger_specific)
+ uint64_t completed, uint64_t total, bool debugger_specific,
+ bool is_discrete)
: 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_is_discrete(is_discrete) {}
static llvm::StringRef GetFlavorString();
@@ -52,6 +53,7 @@ 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 IsDiscrete() const { return m_is_discrete; }
private:
/// The title of this progress event. The value is expected to remain stable
@@ -68,6 +70,7 @@ class ProgressEventData : public EventData {
uint64_t m_completed;
const uint64_t m_total;
const bool m_debugger_specific;
+ const bool m_is_discrete;
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..a48255fc88cf69b 100644
--- a/lldb/include/lldb/Core/Progress.h
+++ b/lldb/include/lldb/Core/Progress.h
@@ -69,8 +69,13 @@ class Progress {
///
/// @param [in] debugger An optional debugger pointer to specify that this
/// progress is to be reported only to specific debuggers.
+ ///
+ /// @param [in] is_discrete Boolean indicating whether or not
+ /// this progress report will happen once during a debug session or multiple
+ /// times as individual progress reports.
Progress(std::string title, uint64_t total = UINT64_MAX,
- lldb_private::Debugger *debugger = nullptr);
+ lldb_private::Debugger *debugger = nullptr,
+ bool is_discrete = false);
/// Destroy the progress object.
///
@@ -110,6 +115,9 @@ class Progress {
/// to ensure that we don't send progress updates after progress has
/// completed.
bool m_complete = false;
+ /// Set to true if the progress event is discrete; meaning it will happen
+ /// multiple times during a debug session as individual progress events
+ bool m_is_discrete = false;
};
} // namespace lldb_private
diff --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp
index 7ec1efc64fe9383..027b01cb2297e31 100644
--- a/lldb/source/Core/Debugger.cpp
+++ b/lldb/source/Core/Debugger.cpp
@@ -1402,22 +1402,23 @@ 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, bool is_discrete) {
// 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, is_discrete)));
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,
+ bool is_discrete) {
// 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 +1427,8 @@ 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,
+ /*is_discrete*/ is_discrete);
return;
}
// The progress event is not debugger specific, iterate over all debuggers
@@ -1436,7 +1438,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, is_discrete);
}
}
diff --git a/lldb/source/Core/DebuggerEvents.cpp b/lldb/source/Core/DebuggerEvents.cpp
index dd77fff349a64a7..6720d84131bfc39 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_discrete", progress_data->IsDiscrete());
return dictionary_sp;
}
diff --git a/lldb/source/Core/Progress.cpp b/lldb/source/Core/Progress.cpp
index 08be73f1470f349..543933af4c53fdd 100644
--- a/lldb/source/Core/Progress.cpp
+++ b/lldb/source/Core/Progress.cpp
@@ -17,7 +17,7 @@ using namespace lldb_private;
std::atomic<uint64_t> Progress::g_id(0);
Progress::Progress(std::string title, uint64_t total,
- lldb_private::Debugger *debugger)
+ lldb_private::Debugger *debugger, bool is_discrete)
: m_title(title), m_id(++g_id), m_completed(0), m_total(total) {
assert(total > 0);
if (debugger)
@@ -55,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_is_discrete);
}
}
diff --git a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
index 3e52c9e3c042811..f50a3be75a7f79b 100644
--- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -2219,7 +2219,7 @@ 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), UINT64_MAX, nullptr, true);
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..96c0cb1e69c7562 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);
+ total_progress, nullptr, true);
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..c4db5f1d1ee5b11 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -457,7 +457,7 @@ 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()), UINT64_MAX, nullptr, true);
m_index = AppleDWARFIndex::Create(
*GetObjectFile()->GetModule(), apple_names, apple_namespaces,
apple_types, apple_objc, m_context.getOrLoadStrData());
@@ -470,7 +470,7 @@ 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()), UINT64_MAX, nullptr, true);
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..ce1149370539a6e 100644
--- a/lldb/source/Symbol/LocateSymbolFile.cpp
+++ b/lldb/source/Symbol/LocateSymbolFile.cpp
@@ -269,7 +269,7 @@ Symbols::LocateExecutableSymbolFile(const ModuleSpec &module_spec,
Progress progress(llvm::formatv(
"Locating external symbol file for {0}",
- module_spec.GetFileSpec().GetFilename().AsCString("<Unknown>")));
+ module_spec.GetFileSpec().GetFilename().AsCString("<Unknown>")), UINT64_MAX, nullptr, true);
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..b71984772bb1fd1 100644
--- a/lldb/test/API/functionalities/progress_reporting/TestProgressReporting.py
+++ b/lldb/test/API/functionalities/progress_reporting/TestProgressReporting.py
@@ -25,8 +25,8 @@ def test_dwarf_symbol_loading_progress_report(self):
event = lldbutil.fetch_next_event(self, self.listener, self.broadcaster)
ret_args = lldb.SBDebugger.GetProgressFromEvent(event)
self.assertGreater(len(ret_args), 0)
- message = ret_args[0]
- self.assertGreater(len(message), 0)
+ title = ret_args[0]
+ self.assertGreater(len(title), 0)
def test_dwarf_symbol_loading_progress_report_structured_data(self):
"""Test that we are able to fetch dwarf symbol loading progress events
@@ -37,5 +37,8 @@ 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_discrete = progress_data.GetValueForKey("is_discrete")
+ self.assertTrue(is_discrete, "is_discrete should be true")
>From 7b9792b0f8739fe72fcf4d1fcf9efadd249da7bb Mon Sep 17 00:00:00 2001
From: Chelsea Cassanova <chelsea_cassanova at apple.com>
Date: Wed, 18 Oct 2023 14:34:08 -0700
Subject: [PATCH 2/5] [lldb][progress] Move position of `is_discrete`
Moves `is_discrete` to be the second parameter instead of the last when creating a progress
report so that the default arguments for `total` and `debugger` do not
have to be provided.
---
lldb/include/lldb/Core/Progress.h | 13 +++++++------
lldb/source/Core/Debugger.cpp | 3 +--
lldb/source/Core/Progress.cpp | 4 ++--
.../Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp | 2 +-
.../Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp | 2 +-
.../Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp | 4 ++--
lldb/source/Symbol/LocateSymbolFile.cpp | 2 +-
7 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/lldb/include/lldb/Core/Progress.h b/lldb/include/lldb/Core/Progress.h
index a48255fc88cf69b..226e6e22ce5542b 100644
--- a/lldb/include/lldb/Core/Progress.h
+++ b/lldb/include/lldb/Core/Progress.h
@@ -63,6 +63,10 @@ class Progress {
///
/// @param [in] title The title of this progress activity.
///
+ /// @param [in] is_discrete Boolean indicating whether or not
+ /// this progress report will happen once during a debug session or multiple
+ /// times as individual progress reports.
+ ///
/// @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.
@@ -70,12 +74,9 @@ class Progress {
/// @param [in] debugger An optional debugger pointer to specify that this
/// progress is to be reported only to specific debuggers.
///
- /// @param [in] is_discrete Boolean indicating whether or not
- /// this progress report will happen once during a debug session or multiple
- /// times as individual progress reports.
- Progress(std::string title, uint64_t total = UINT64_MAX,
- lldb_private::Debugger *debugger = nullptr,
- bool is_discrete = false);
+ Progress(std::string title, bool is_discrete = false,
+ uint64_t total = UINT64_MAX,
+ lldb_private::Debugger *debugger = nullptr);
/// Destroy the progress object.
///
diff --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp
index 027b01cb2297e31..7973e4a4a39e47a 100644
--- a/lldb/source/Core/Debugger.cpp
+++ b/lldb/source/Core/Debugger.cpp
@@ -1427,8 +1427,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_discrete*/ is_discrete);
+ /*is_debugger_specific*/ true, is_discrete);
return;
}
// The progress event is not debugger specific, iterate over all debuggers
diff --git a/lldb/source/Core/Progress.cpp b/lldb/source/Core/Progress.cpp
index 543933af4c53fdd..cf110ba732576d8 100644
--- a/lldb/source/Core/Progress.cpp
+++ b/lldb/source/Core/Progress.cpp
@@ -16,8 +16,8 @@ using namespace lldb_private;
std::atomic<uint64_t> Progress::g_id(0);
-Progress::Progress(std::string title, uint64_t total,
- lldb_private::Debugger *debugger, bool is_discrete)
+Progress::Progress(std::string title, bool is_discrete, uint64_t total,
+ lldb_private::Debugger *debugger)
: m_title(title), m_id(++g_id), m_completed(0), m_total(total) {
assert(total > 0);
if (debugger)
diff --git a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
index f50a3be75a7f79b..2d37ba871e1f278 100644
--- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -2219,7 +2219,7 @@ 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), UINT64_MAX, nullptr, true);
+ Progress progress(llvm::formatv("Parsing symbol table for {0}", file_name), true);
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 96c0cb1e69c7562..252736b1bd4e988 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, nullptr, true);
+ true, 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 c4db5f1d1ee5b11..696f305ff58eb39 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -457,7 +457,7 @@ 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()), UINT64_MAX, nullptr, true);
+ module_desc.GetData()), true);
m_index = AppleDWARFIndex::Create(
*GetObjectFile()->GetModule(), apple_names, apple_namespaces,
apple_types, apple_objc, m_context.getOrLoadStrData());
@@ -470,7 +470,7 @@ void SymbolFileDWARF::InitializeObject() {
LoadSectionData(eSectionTypeDWARFDebugNames, debug_names);
if (debug_names.GetByteSize() > 0) {
Progress progress(
- llvm::formatv("Loading DWARF5 index for {0}", module_desc.GetData()), UINT64_MAX, nullptr, true);
+ llvm::formatv("Loading DWARF5 index for {0}", module_desc.GetData()), true);
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 ce1149370539a6e..564772c98b1ba24 100644
--- a/lldb/source/Symbol/LocateSymbolFile.cpp
+++ b/lldb/source/Symbol/LocateSymbolFile.cpp
@@ -269,7 +269,7 @@ Symbols::LocateExecutableSymbolFile(const ModuleSpec &module_spec,
Progress progress(llvm::formatv(
"Locating external symbol file for {0}",
- module_spec.GetFileSpec().GetFilename().AsCString("<Unknown>")), UINT64_MAX, nullptr, true);
+ module_spec.GetFileSpec().GetFilename().AsCString("<Unknown>")), true);
FileSpecList debug_file_search_paths = default_search_paths;
>From c5ba61c82a9b38e7967744929c1473ef8b857887 Mon Sep 17 00:00:00 2001
From: Chelsea Cassanova <chelsea_cassanova at apple.com>
Date: Wed, 18 Oct 2023 14:50:19 -0700
Subject: [PATCH 3/5] [lldb][progress] Change variable name in test
Changes a variable name in the progress reporting test from title to message.
---
.../progress_reporting/TestProgressReporting.py | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lldb/test/API/functionalities/progress_reporting/TestProgressReporting.py b/lldb/test/API/functionalities/progress_reporting/TestProgressReporting.py
index b71984772bb1fd1..cda9f9c62a4278d 100644
--- a/lldb/test/API/functionalities/progress_reporting/TestProgressReporting.py
+++ b/lldb/test/API/functionalities/progress_reporting/TestProgressReporting.py
@@ -25,8 +25,8 @@ def test_dwarf_symbol_loading_progress_report(self):
event = lldbutil.fetch_next_event(self, self.listener, self.broadcaster)
ret_args = lldb.SBDebugger.GetProgressFromEvent(event)
self.assertGreater(len(ret_args), 0)
- title = ret_args[0]
- self.assertGreater(len(title), 0)
+ message = ret_args[0]
+ self.assertGreater(len(message), 0)
def test_dwarf_symbol_loading_progress_report_structured_data(self):
"""Test that we are able to fetch dwarf symbol loading progress events
>From bb5090bc4277fd2aa1e779c91623d42c92f74c09 Mon Sep 17 00:00:00 2001
From: Chelsea Cassanova <chelsea_cassanova at apple.com>
Date: Wed, 18 Oct 2023 16:31:09 -0700
Subject: [PATCH 4/5] Update test and initialize is_discrete
Updates `TestProgressReporting` to check that the `is_discrete` key
exists and is true. Also initializes `m_is_discrete` in `Progress`.
---
lldb/include/lldb/Core/Progress.h | 6 +++---
lldb/source/Core/Progress.cpp | 3 ++-
.../progress_reporting/TestProgressReporting.py | 7 ++++++-
3 files changed, 11 insertions(+), 5 deletions(-)
diff --git a/lldb/include/lldb/Core/Progress.h b/lldb/include/lldb/Core/Progress.h
index 226e6e22ce5542b..a5137fc1e455ce6 100644
--- a/lldb/include/lldb/Core/Progress.h
+++ b/lldb/include/lldb/Core/Progress.h
@@ -103,6 +103,9 @@ class Progress {
/// The title of the progress activity.
std::string m_title;
std::mutex m_mutex;
+ /// Set to true if the progress event is discrete; meaning it will happen
+ /// multiple times during a debug session as individual progress events
+ bool m_is_discrete = false;
/// A unique integer identifier for progress reporting.
const uint64_t m_id;
/// How much work ([0...m_total]) that has been completed.
@@ -116,9 +119,6 @@ class Progress {
/// to ensure that we don't send progress updates after progress has
/// completed.
bool m_complete = false;
- /// Set to true if the progress event is discrete; meaning it will happen
- /// multiple times during a debug session as individual progress events
- bool m_is_discrete = false;
};
} // namespace lldb_private
diff --git a/lldb/source/Core/Progress.cpp b/lldb/source/Core/Progress.cpp
index cf110ba732576d8..805f5f07e998bb3 100644
--- a/lldb/source/Core/Progress.cpp
+++ b/lldb/source/Core/Progress.cpp
@@ -18,7 +18,8 @@ std::atomic<uint64_t> Progress::g_id(0);
Progress::Progress(std::string title, bool is_discrete, uint64_t total,
lldb_private::Debugger *debugger)
- : m_title(title), m_id(++g_id), m_completed(0), m_total(total) {
+ : m_title(title), m_is_discrete(is_discrete), m_id(++g_id), m_completed(0),
+ m_total(total) {
assert(total > 0);
if (debugger)
m_debugger_id = debugger->GetID();
diff --git a/lldb/test/API/functionalities/progress_reporting/TestProgressReporting.py b/lldb/test/API/functionalities/progress_reporting/TestProgressReporting.py
index cda9f9c62a4278d..cd72c650145f2c4 100644
--- a/lldb/test/API/functionalities/progress_reporting/TestProgressReporting.py
+++ b/lldb/test/API/functionalities/progress_reporting/TestProgressReporting.py
@@ -41,4 +41,9 @@ def test_dwarf_symbol_loading_progress_report_structured_data(self):
self.assertGreater(len(title), 0)
is_discrete = progress_data.GetValueForKey("is_discrete")
- self.assertTrue(is_discrete, "is_discrete should be true")
+ self.assertTrue(
+ is_discrete.IsValid(), "ProgressEventData key 'is_discrete' does not exist."
+ )
+ self.assertTrue(
+ is_discrete, "ProgressEventData key 'is_discrete' should be true."
+ )
>From e10eaf72af5b4517fcb27a0bd37f6f58b160adfc Mon Sep 17 00:00:00 2001
From: Chelsea Cassanova <chelsea_cassanova at apple.com>
Date: Thu, 19 Oct 2023 11:39:39 -0700
Subject: [PATCH 5/5] Use enum instead of bool, use aggregate instead of
discrete
Adds an enum to Progress.h that progress reports will use instead of a
boolean to indicate the type of progress report being delivered. Also
uses the word 'aggregate' instead of 'discrete' as well as using an example
in the parameter description for `is_aggregate` in Progress.cpp to
better convey what the enum is supposed to indicate.
---
lldb/include/lldb/Core/Debugger.h | 8 +++++-
lldb/include/lldb/Core/DebuggerEvents.h | 8 +++---
lldb/include/lldb/Core/Progress.h | 27 ++++++++++++++-----
lldb/source/Core/Debugger.cpp | 12 +++++----
lldb/source/Core/DebuggerEvents.cpp | 2 +-
lldb/source/Core/Progress.cpp | 8 +++---
.../ObjectFile/Mach-O/ObjectFileMachO.cpp | 2 +-
.../SymbolFile/DWARF/ManualDWARFIndex.cpp | 2 +-
.../SymbolFile/DWARF/SymbolFileDWARF.cpp | 4 +--
lldb/source/Symbol/LocateSymbolFile.cpp | 8 +++---
.../TestProgressReporting.py | 7 ++---
11 files changed, 57 insertions(+), 31 deletions(-)
diff --git a/lldb/include/lldb/Core/Debugger.h b/lldb/include/lldb/Core/Debugger.h
index 8e21502dac6dee2..7f5feb1ecd3e000 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] is_aggregate
+ /// 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, bool is_discrete);
+ std::optional<lldb::user_id_t> debugger_id,
+ uint32_t is_aggregate);
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 88455d8d60bb488..6a015b7b8f68138 100644
--- a/lldb/include/lldb/Core/DebuggerEvents.h
+++ b/lldb/include/lldb/Core/DebuggerEvents.h
@@ -22,10 +22,10 @@ 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,
- bool is_discrete)
+ bool is_aggregate)
: 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_is_discrete(is_discrete) {}
+ m_debugger_specific(debugger_specific), m_is_aggregate(is_aggregate) {}
static llvm::StringRef GetFlavorString();
@@ -53,7 +53,7 @@ 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 IsDiscrete() const { return m_is_discrete; }
+ bool IsAggregate() const { return m_is_aggregate; }
private:
/// The title of this progress event. The value is expected to remain stable
@@ -70,7 +70,7 @@ class ProgressEventData : public EventData {
uint64_t m_completed;
const uint64_t m_total;
const bool m_debugger_specific;
- const bool m_is_discrete;
+ const bool m_is_aggregate;
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 a5137fc1e455ce6..4cb1ae4e287c064 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 ProgressReportType {
+ eAggregateProgressReport,
+ eNonAggregateProgressReport
+ };
/// Construct a progress object that will report information.
///
/// The constructor will create a unique progress reporting object and
@@ -63,9 +68,18 @@ class Progress {
///
/// @param [in] title The title of this progress activity.
///
- /// @param [in] is_discrete Boolean indicating whether or not
- /// this progress report will happen once during a debug session or multiple
- /// times as individual progress reports.
+ /// @param [in] is_discrete 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
@@ -74,7 +88,8 @@ class Progress {
/// @param [in] debugger An optional debugger pointer to specify that this
/// progress is to be reported only to specific debuggers.
///
- Progress(std::string title, bool is_discrete = false,
+ Progress(std::string title,
+ uint32_t is_aggregate = eNonAggregateProgressReport,
uint64_t total = UINT64_MAX,
lldb_private::Debugger *debugger = nullptr);
@@ -103,9 +118,9 @@ class Progress {
/// The title of the progress activity.
std::string m_title;
std::mutex m_mutex;
- /// Set to true if the progress event is discrete; meaning it will happen
+ /// Set to true if the progress event is aggregate; meaning it will happen
/// multiple times during a debug session as individual progress events
- bool m_is_discrete = false;
+ uint32_t m_is_aggregate = 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 7973e4a4a39e47a..5a97de8eff44496 100644
--- a/lldb/source/Core/Debugger.cpp
+++ b/lldb/source/Core/Debugger.cpp
@@ -1402,7 +1402,8 @@ 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_discrete) {
+ bool is_debugger_specific,
+ uint32_t is_aggregate) {
// Only deliver progress events if we have any progress listeners.
const uint32_t event_type = Debugger::eBroadcastBitProgress;
if (!debugger.GetBroadcaster().EventTypeHasListeners(event_type))
@@ -1410,7 +1411,7 @@ static void PrivateReportProgress(Debugger &debugger, uint64_t progress_id,
EventSP event_sp(new Event(
event_type, new ProgressEventData(progress_id, std::move(title),
std::move(details), completed, total,
- is_debugger_specific, is_discrete)));
+ is_debugger_specific, is_aggregate)));
debugger.GetBroadcaster().BroadcastEvent(event_sp);
}
@@ -1418,7 +1419,7 @@ 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,
- bool is_discrete) {
+ uint32_t is_aggregate) {
// 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
@@ -1427,7 +1428,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_discrete);
+ /*is_debugger_specific*/ true, is_aggregate);
return;
}
// The progress event is not debugger specific, iterate over all debuggers
@@ -1437,7 +1438,8 @@ 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, is_discrete);
+ total, /*is_debugger_specific*/ false,
+ is_aggregate);
}
}
diff --git a/lldb/source/Core/DebuggerEvents.cpp b/lldb/source/Core/DebuggerEvents.cpp
index 6720d84131bfc39..73797bc668a395f 100644
--- a/lldb/source/Core/DebuggerEvents.cpp
+++ b/lldb/source/Core/DebuggerEvents.cpp
@@ -67,7 +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_discrete", progress_data->IsDiscrete());
+ 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 805f5f07e998bb3..477daaa41cc810b 100644
--- a/lldb/source/Core/Progress.cpp
+++ b/lldb/source/Core/Progress.cpp
@@ -16,10 +16,10 @@ using namespace lldb_private;
std::atomic<uint64_t> Progress::g_id(0);
-Progress::Progress(std::string title, bool is_discrete, uint64_t total,
+Progress::Progress(std::string title, uint32_t is_aggregate, uint64_t total,
lldb_private::Debugger *debugger)
- : m_title(title), m_is_discrete(is_discrete), m_id(++g_id), m_completed(0),
- m_total(total) {
+ : m_title(title), m_is_aggregate(is_aggregate), m_id(++g_id),
+ m_completed(0), m_total(total) {
assert(total > 0);
if (debugger)
m_debugger_id = debugger->GetID();
@@ -56,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_is_discrete);
+ m_total, m_debugger_id, m_is_aggregate);
}
}
diff --git a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
index 2d37ba871e1f278..13a6ee0d8d4f162 100644
--- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -2219,7 +2219,7 @@ 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), true);
+ Progress progress(llvm::formatv("Parsing symbol table for {0}", file_name), Progress::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 252736b1bd4e988..84c9015b4a8f939 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()),
- true, total_progress);
+ Progress::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 696f305ff58eb39..d545f420ceaa84a 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -457,7 +457,7 @@ 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()), true);
+ module_desc.GetData()), Progress::eAggregateProgressReport);
m_index = AppleDWARFIndex::Create(
*GetObjectFile()->GetModule(), apple_names, apple_namespaces,
apple_types, apple_objc, m_context.getOrLoadStrData());
@@ -470,7 +470,7 @@ void SymbolFileDWARF::InitializeObject() {
LoadSectionData(eSectionTypeDWARFDebugNames, debug_names);
if (debug_names.GetByteSize() > 0) {
Progress progress(
- llvm::formatv("Loading DWARF5 index for {0}", module_desc.GetData()), true);
+ llvm::formatv("Loading DWARF5 index for {0}", module_desc.GetData()), Progress::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 564772c98b1ba24..59d0d484e4d3ace 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>")), true);
+ Progress progress(
+ llvm::formatv(
+ "Locating external symbol file for {0}",
+ module_spec.GetFileSpec().GetFilename().AsCString("<Unknown>")),
+ Progress::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 cd72c650145f2c4..634c0fb07180774 100644
--- a/lldb/test/API/functionalities/progress_reporting/TestProgressReporting.py
+++ b/lldb/test/API/functionalities/progress_reporting/TestProgressReporting.py
@@ -40,10 +40,11 @@ def test_dwarf_symbol_loading_progress_report_structured_data(self):
title = progress_data.GetValueForKey("title").GetStringValue(100)
self.assertGreater(len(title), 0)
- is_discrete = progress_data.GetValueForKey("is_discrete")
+ is_aggregate = progress_data.GetValueForKey("is_aggregate")
self.assertTrue(
- is_discrete.IsValid(), "ProgressEventData key 'is_discrete' does not exist."
+ is_aggregate.IsValid(),
+ "ProgressEventData key 'is_aggregate' does not exist.",
)
self.assertTrue(
- is_discrete, "ProgressEventData key 'is_discrete' should be true."
+ is_aggregate, "ProgressEventData key 'is_aggregate' should be true."
)
More information about the lldb-commits
mailing list