[Lldb-commits] [lldb] f1ef910 - [lldb][Progress] Separate title and details (#77547)
via lldb-commits
lldb-commits at lists.llvm.org
Tue Jan 16 07:57:22 PST 2024
Author: Chelsea Cassanova
Date: 2024-01-16T07:57:18-08:00
New Revision: f1ef910b97d6acb80480b79a4144541311369cc9
URL: https://github.com/llvm/llvm-project/commit/f1ef910b97d6acb80480b79a4144541311369cc9
DIFF: https://github.com/llvm/llvm-project/commit/f1ef910b97d6acb80480b79a4144541311369cc9.diff
LOG: [lldb][Progress] Separate title and details (#77547)
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. In addition, changes the total amount of progress completed into a std::optional. Also
updates the test to check for details being correctly reported from the
event structured data dictionary.
Added:
Modified:
lldb/include/lldb/Core/DebuggerEvents.h
lldb/include/lldb/Core/Progress.h
lldb/source/Core/Progress.cpp
lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
lldb/source/Plugins/SymbolLocator/Default/SymbolLocatorDefault.cpp
lldb/test/API/functionalities/progress_reporting/TestProgressReporting.py
Removed:
################################################################################
diff --git a/lldb/include/lldb/Core/DebuggerEvents.h b/lldb/include/lldb/Core/DebuggerEvents.h
index 982b22229701f8..4a27766e94e3aa 100644
--- a/lldb/include/lldb/Core/DebuggerEvents.h
+++ b/lldb/include/lldb/Core/DebuggerEvents.h
@@ -20,9 +20,10 @@ 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)
- : m_title(std::move(title)), m_details(std::move(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(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..65d30ea25cd292 100644
--- a/lldb/include/lldb/Core/Progress.h
+++ b/lldb/include/lldb/Core/Progress.h
@@ -64,12 +64,13 @@ 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
+ /// set to std::nullopt 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, std::string details = {},
+ std::optional<uint64_t> total = std::nullopt,
lldb_private::Debugger *debugger = nullptr);
/// Destroy the progress object.
@@ -89,20 +90,23 @@ class Progress {
/// @param [in] amount The amount to increment m_completed by.
///
/// @param [in] an optional message associated with this update.
- void Increment(uint64_t amount = 1, std::string update = {});
+ void Increment(uint64_t amount = 1,
+ std::optional<std::string> updated_detail = {});
private:
- void ReportProgress(std::string update = {});
+ void ReportProgress();
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;
/// 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 in the constructor for non
+ /// deterministic progress.
+ 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/Progress.cpp b/lldb/source/Core/Progress.cpp
index ea3f874916a999..355d6952e53ca9 100644
--- a/lldb/source/Core/Progress.cpp
+++ b/lldb/source/Core/Progress.cpp
@@ -11,15 +11,22 @@
#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, std::string details,
+ std::optional<uint64_t> total,
lldb_private::Debugger *debugger)
- : m_title(title), m_id(++g_id), m_completed(0), m_total(total) {
- assert(total > 0);
+ : m_title(title), m_details(details), m_id(++g_id), m_completed(0),
+ m_total(1) {
+ assert(total == std::nullopt || total > 0);
+ if (total)
+ m_total = *total;
+
if (debugger)
m_debugger_id = debugger->GetID();
std::lock_guard<std::mutex> guard(m_mutex);
@@ -35,25 +42,28 @@ Progress::~Progress() {
ReportProgress();
}
-void Progress::Increment(uint64_t amount, std::string update) {
+void Progress::Increment(uint64_t amount,
+ std::optional<std::string> updated_detail) {
if (amount > 0) {
std::lock_guard<std::mutex> guard(m_mutex);
+ if (updated_detail)
+ m_details = std::move(updated_detail.value());
// Watch out for unsigned overflow and make sure we don't increment too
// much and exceed m_total.
- if (amount > (m_total - m_completed))
+ if (m_total && (amount > (m_total - m_completed)))
m_completed = m_total;
else
m_completed += amount;
- ReportProgress(update);
+ ReportProgress();
}
}
-void Progress::ReportProgress(std::string update) {
+void Progress::ReportProgress() {
if (!m_complete) {
// Make sure we only send one notification that indicates the progress is
- // complete.
+ // complete
m_complete = m_completed == m_total;
- Debugger::ReportProgress(m_id, m_title, std::move(update), 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/ObjectFile/ELF/ObjectFileELF.cpp b/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
index 43ab87f08e1925..0d95a1c12bde35 100644
--- a/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ b/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -2897,9 +2897,8 @@ void ObjectFileELF::ParseSymtab(Symtab &lldb_symtab) {
if (!module_sp)
return;
- Progress progress(
- llvm::formatv("Parsing symbol table for {0}",
- m_file.GetFilename().AsCString("<Unknown>")));
+ Progress progress("Parsing symbol table",
+ m_file.GetFilename().AsCString("<Unknown>"));
ElapsedTime elapsed(module_sp->GetSymtabParseTime());
// We always want to use the main object file so we (hopefully) only have one
diff --git a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
index d7a2846200fcaf..e40ba8d3addb51 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..92275600f99caa 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
@@ -75,9 +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(
- 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 3371803b4ca04a..bec8111c412161 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -519,8 +519,6 @@ 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()));
m_index = AppleDWARFIndex::Create(
*GetObjectFile()->GetModule(), apple_names, apple_namespaces,
apple_types, apple_objc, m_context.getOrLoadStrData());
@@ -532,8 +530,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..6f0126b16cdc00 100644
--- a/lldb/source/Plugins/SymbolLocator/Default/SymbolLocatorDefault.cpp
+++ b/lldb/source/Plugins/SymbolLocator/Default/SymbolLocatorDefault.cpp
@@ -98,9 +98,9 @@ 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)
More information about the lldb-commits
mailing list