[Lldb-commits] [lldb] [lldb][Progress] Separate title and details (PR #77547)
Chelsea Cassanova via lldb-commits
lldb-commits at lists.llvm.org
Tue Jan 9 21:49:09 PST 2024
https://github.com/chelcassanova updated https://github.com/llvm/llvm-project/pull/77547
>From 44a3cdca21bc9c2aa24eeaf5d82c8b8af382bfa7 Mon Sep 17 00:00:00 2001
From: Chelsea Cassanova <chelsea_cassanova at apple.com>
Date: Tue, 9 Jan 2024 18:32:06 -0800
Subject: [PATCH 1/2] [lldb][Progress] Separate title and details
Per this RFC:
https://discourse.llvm.org/t/rfc-improve-lldb-progress-reporting/75717
on improving progress reports, this commit separates the title field and
details field so that the title specifies the category that the progress
report falls under. The details field is added as a part of the
constructor for progress reports and by default is an empty string. Also
updates the test to check for details being correctly reported from the
event structured data dictionary.
---
lldb/include/lldb/Core/DebuggerEvents.h | 4 ++--
lldb/include/lldb/Core/Progress.h | 3 ++-
lldb/source/Core/Progress.cpp | 7 ++++---
lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp | 2 +-
lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp | 4 +---
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp | 6 ++----
.../Plugins/SymbolLocator/Default/SymbolLocatorDefault.cpp | 4 +---
.../progress_reporting/TestProgressReporting.py | 2 ++
8 files changed, 15 insertions(+), 17 deletions(-)
diff --git a/lldb/include/lldb/Core/DebuggerEvents.h b/lldb/include/lldb/Core/DebuggerEvents.h
index 982b22229701f8..1fadb96a4b565d 100644
--- a/lldb/include/lldb/Core/DebuggerEvents.h
+++ b/lldb/include/lldb/Core/DebuggerEvents.h
@@ -20,9 +20,9 @@ class Stream;
class ProgressEventData : public EventData {
public:
- ProgressEventData(uint64_t progress_id, std::string title, std::string update,
+ ProgressEventData(uint64_t progress_id, std::string title, std::string details,
uint64_t completed, uint64_t total, bool debugger_specific)
- : m_title(std::move(title)), m_details(std::move(update)),
+ : m_title(std::move(title)), m_details(std::move(details)),
m_id(progress_id), m_completed(completed), m_total(total),
m_debugger_specific(debugger_specific) {}
diff --git a/lldb/include/lldb/Core/Progress.h b/lldb/include/lldb/Core/Progress.h
index b2b8781a43b059..1ea6df01a4bd4a 100644
--- a/lldb/include/lldb/Core/Progress.h
+++ b/lldb/include/lldb/Core/Progress.h
@@ -69,7 +69,7 @@ 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, uint64_t total = UINT64_MAX,
+ Progress(std::string title, std::string details = {}, uint64_t total = UINT64_MAX,
lldb_private::Debugger *debugger = nullptr);
/// Destroy the progress object.
@@ -96,6 +96,7 @@ class Progress {
static std::atomic<uint64_t> g_id;
/// The title of the progress activity.
std::string m_title;
+ std::string m_details;
std::mutex m_mutex;
/// A unique integer identifier for progress reporting.
const uint64_t m_id;
diff --git a/lldb/source/Core/Progress.cpp b/lldb/source/Core/Progress.cpp
index ea3f874916a999..8c99b561373362 100644
--- a/lldb/source/Core/Progress.cpp
+++ b/lldb/source/Core/Progress.cpp
@@ -16,9 +16,9 @@ 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, std::string details, uint64_t total,
lldb_private::Debugger *debugger)
- : m_title(title), m_id(++g_id), m_completed(0), m_total(total) {
+ : m_title(title), m_details(details), m_id(++g_id), m_completed(0), m_total(total) {
assert(total > 0);
if (debugger)
m_debugger_id = debugger->GetID();
@@ -38,6 +38,7 @@ Progress::~Progress() {
void Progress::Increment(uint64_t amount, std::string update) {
if (amount > 0) {
std::lock_guard<std::mutex> guard(m_mutex);
+ m_details = update;
// Watch out for unsigned overflow and make sure we don't increment too
// much and exceed m_total.
if (amount > (m_total - m_completed))
@@ -53,7 +54,7 @@ void Progress::ReportProgress(std::string update) {
// Make sure we only send one notification that indicates the progress is
// complete.
m_complete = m_completed == m_total;
- Debugger::ReportProgress(m_id, m_title, std::move(update), m_completed,
+ Debugger::ReportProgress(m_id, m_title, m_details, m_completed,
m_total, m_debugger_id);
}
}
diff --git a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
index 182a9f2afaeb2e..f451bac598b874 100644
--- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -2225,7 +2225,7 @@ 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("Parsing symbol table", file_name);
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 16ff5f7d4842ca..52555908865b0b 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
@@ -75,9 +75,7 @@ void ManualDWARFIndex::Index() {
// Include 2 passes per unit to index for extracting DIEs from the unit and
// indexing the unit, and then 8 extra entries for finalizing each index set.
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 progress("Manually indexing DWARF", module_desc.GetData(), 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 1a16b70f42fe1f..1ea14366ad7dd7 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -519,8 +519,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()));
+ Progress progress("Loading Apple DWARF index", module_desc.GetData());
m_index = AppleDWARFIndex::Create(
*GetObjectFile()->GetModule(), apple_names, apple_namespaces,
apple_types, apple_objc, m_context.getOrLoadStrData());
@@ -532,8 +531,7 @@ void SymbolFileDWARF::InitializeObject() {
DWARFDataExtractor debug_names;
LoadSectionData(eSectionTypeDWARFDebugNames, debug_names);
if (debug_names.GetByteSize() > 0) {
- Progress progress(
- llvm::formatv("Loading DWARF5 index for {0}", module_desc.GetData()));
+ Progress progress("Loading DWARF5 index", module_desc.GetData());
llvm::Expected<std::unique_ptr<DebugNamesDWARFIndex>> index_or =
DebugNamesDWARFIndex::Create(*GetObjectFile()->GetModule(),
debug_names,
diff --git a/lldb/source/Plugins/SymbolLocator/Default/SymbolLocatorDefault.cpp b/lldb/source/Plugins/SymbolLocator/Default/SymbolLocatorDefault.cpp
index ed014f99fdb5e7..52ae7adb8d3d97 100644
--- a/lldb/source/Plugins/SymbolLocator/Default/SymbolLocatorDefault.cpp
+++ b/lldb/source/Plugins/SymbolLocator/Default/SymbolLocatorDefault.cpp
@@ -98,9 +98,7 @@ std::optional<FileSpec> SymbolLocatorDefault::LocateExecutableSymbolFile(
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("Locating external symbol file", module_spec.GetFileSpec().GetFilename().AsCString("<Unknown>"));
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 0e72770e350366..9af53845ca1b77 100644
--- a/lldb/test/API/functionalities/progress_reporting/TestProgressReporting.py
+++ b/lldb/test/API/functionalities/progress_reporting/TestProgressReporting.py
@@ -39,3 +39,5 @@ def test_dwarf_symbol_loading_progress_report_structured_data(self):
progress_data = lldb.SBDebugger.GetProgressDataFromEvent(event)
message = progress_data.GetValueForKey("message").GetStringValue(100)
self.assertGreater(len(message), 0)
+ details = progress_data.GetValueForKey("details").GetStringValue(100)
+ self.assertGreater(len(details), 0)
>From 75de25c88f13130db8e8b806fa928b6d858daef5 Mon Sep 17 00:00:00 2001
From: Chelsea Cassanova <chelsea_cassanova at apple.com>
Date: Tue, 9 Jan 2024 21:47:36 -0800
Subject: [PATCH 2/2] fixup! [lldb][Progress] Separate title and details
Formatting
---
lldb/include/lldb/Core/DebuggerEvents.h | 5 +++--
lldb/include/lldb/Core/Progress.h | 3 ++-
lldb/source/Core/Progress.cpp | 7 ++++---
lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp | 3 ++-
.../Plugins/SymbolLocator/Default/SymbolLocatorDefault.cpp | 4 +++-
5 files changed, 14 insertions(+), 8 deletions(-)
diff --git a/lldb/include/lldb/Core/DebuggerEvents.h b/lldb/include/lldb/Core/DebuggerEvents.h
index 1fadb96a4b565d..4a27766e94e3aa 100644
--- a/lldb/include/lldb/Core/DebuggerEvents.h
+++ b/lldb/include/lldb/Core/DebuggerEvents.h
@@ -20,8 +20,9 @@ class Stream;
class ProgressEventData : public EventData {
public:
- ProgressEventData(uint64_t progress_id, std::string title, std::string details,
- uint64_t completed, uint64_t total, bool debugger_specific)
+ ProgressEventData(uint64_t progress_id, std::string title,
+ std::string details, uint64_t completed, uint64_t total,
+ bool debugger_specific)
: m_title(std::move(title)), m_details(std::move(details)),
m_id(progress_id), m_completed(completed), m_total(total),
m_debugger_specific(debugger_specific) {}
diff --git a/lldb/include/lldb/Core/Progress.h b/lldb/include/lldb/Core/Progress.h
index 1ea6df01a4bd4a..fd5b8463ccc5c2 100644
--- a/lldb/include/lldb/Core/Progress.h
+++ b/lldb/include/lldb/Core/Progress.h
@@ -69,7 +69,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, std::string details = {}, uint64_t total = UINT64_MAX,
+ Progress(std::string title, std::string details = {},
+ uint64_t total = UINT64_MAX,
lldb_private::Debugger *debugger = nullptr);
/// Destroy the progress object.
diff --git a/lldb/source/Core/Progress.cpp b/lldb/source/Core/Progress.cpp
index 8c99b561373362..6d27763f85acbf 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, std::string details, uint64_t total,
lldb_private::Debugger *debugger)
- : m_title(title), m_details(details), m_id(++g_id), m_completed(0), m_total(total) {
+ : m_title(title), m_details(details), m_id(++g_id), m_completed(0),
+ m_total(total) {
assert(total > 0);
if (debugger)
m_debugger_id = debugger->GetID();
@@ -54,7 +55,7 @@ void Progress::ReportProgress(std::string update) {
// Make sure we only send one notification that indicates the progress is
// complete.
m_complete = m_completed == m_total;
- Debugger::ReportProgress(m_id, m_title, m_details, m_completed,
- m_total, m_debugger_id);
+ Debugger::ReportProgress(m_id, m_title, m_details, m_completed, m_total,
+ m_debugger_id);
}
}
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp b/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
index 52555908865b0b..92275600f99caa 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
@@ -75,7 +75,8 @@ void ManualDWARFIndex::Index() {
// Include 2 passes per unit to index for extracting DIEs from the unit and
// indexing the unit, and then 8 extra entries for finalizing each index set.
const uint64_t total_progress = units_to_index.size() * 2 + 8;
- Progress progress("Manually indexing DWARF", module_desc.GetData(), total_progress);
+ Progress progress("Manually indexing DWARF", module_desc.GetData(),
+ total_progress);
std::vector<IndexSet> sets(units_to_index.size());
diff --git a/lldb/source/Plugins/SymbolLocator/Default/SymbolLocatorDefault.cpp b/lldb/source/Plugins/SymbolLocator/Default/SymbolLocatorDefault.cpp
index 52ae7adb8d3d97..6f0126b16cdc00 100644
--- a/lldb/source/Plugins/SymbolLocator/Default/SymbolLocatorDefault.cpp
+++ b/lldb/source/Plugins/SymbolLocator/Default/SymbolLocatorDefault.cpp
@@ -98,7 +98,9 @@ std::optional<FileSpec> SymbolLocatorDefault::LocateExecutableSymbolFile(
FileSystem::Instance().Exists(symbol_file_spec))
return symbol_file_spec;
- Progress progress("Locating external symbol file", module_spec.GetFileSpec().GetFilename().AsCString("<Unknown>"));
+ Progress progress(
+ "Locating external symbol file",
+ module_spec.GetFileSpec().GetFilename().AsCString("<Unknown>"));
FileSpecList debug_file_search_paths = default_search_paths;
More information about the lldb-commits
mailing list