JDevlieghere created this revision. JDevlieghere added reviewers: clayborg, mib, kastiglione, bulbazord. Herald added a project: All. JDevlieghere requested review of this revision.
This patch adds the ability to add a message to a progress event update. Consider the following example as motivation. Say you have to load symbols for 3 dynamic libraries: `libFoo`, `libBar`, `libBaz`. Currently, there are two ways to report this: - As 3 separate progress instances. In this case you create a `progress` instance with the message "Loading symbols: libFoo", ""Loading symbols: libBar", and "Loading symbols: libBaz" respectively. If the operations are completely separate, i.e. they belong to different modules, this approach makes sense. This is what we do for parsing the symbol table, loading the dwarf index, etc. - As 1 progress instance with 3 units of work. This is the preferred approach is you know the number of units of work in advance, because you can report determinate progress. The title would be "Loading symbols" and you call `Progress::Increment` for each of the libraries. Note that with the current design, there's no way to include the name of the libraries, which is what this patch addresses. https://reviews.llvm.org/D143690 Files: lldb/include/lldb/Core/Debugger.h lldb/include/lldb/Core/DebuggerEvents.h lldb/include/lldb/Core/Progress.h lldb/source/Core/Debugger.cpp lldb/source/Core/DebuggerEvents.cpp lldb/source/Core/Progress.cpp
Index: lldb/source/Core/Progress.cpp =================================================================== --- lldb/source/Core/Progress.cpp +++ lldb/source/Core/Progress.cpp @@ -36,7 +36,7 @@ } } -void Progress::Increment(uint64_t amount) { +void Progress::Increment(uint64_t amount, std::string update) { if (amount > 0) { std::lock_guard<std::mutex> guard(m_mutex); // Watch out for unsigned overflow and make sure we don't increment too @@ -45,16 +45,16 @@ m_completed = m_total; else m_completed += amount; - ReportProgress(); + ReportProgress(update); } } -void Progress::ReportProgress() { +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; - Debugger::ReportProgress(m_id, m_title, m_completed, m_total, - m_debugger_id); + Debugger::ReportProgress(m_id, m_title, std::move(update), m_completed, + m_total, m_debugger_id); } } Index: lldb/source/Core/DebuggerEvents.cpp =================================================================== --- lldb/source/Core/DebuggerEvents.cpp +++ lldb/source/Core/DebuggerEvents.cpp @@ -33,7 +33,9 @@ } void ProgressEventData::Dump(Stream *s) const { - s->Printf(" id = %" PRIu64 ", message = \"%s\"", m_id, m_message.c_str()); + s->Printf(" id = %" PRIu64 ", title = \"%s\"", m_id, m_title.c_str()); + if (!m_update.empty()) + s->Printf(", update = \"%s\"", m_update.c_str()); if (m_completed == 0 || m_completed == m_total) s->Printf(", type = %s", m_completed == 0 ? "start" : "end"); else Index: lldb/source/Core/Debugger.cpp =================================================================== --- lldb/source/Core/Debugger.cpp +++ lldb/source/Core/Debugger.cpp @@ -1286,7 +1286,7 @@ } static void PrivateReportProgress(Debugger &debugger, uint64_t progress_id, - const std::string &message, + std::string title, std::string update, uint64_t completed, uint64_t total, bool is_debugger_specific) { // Only deliver progress events if we have any progress listeners. @@ -1294,13 +1294,15 @@ if (!debugger.GetBroadcaster().EventTypeHasListeners(event_type)) return; EventSP event_sp(new Event( - event_type, new ProgressEventData(progress_id, message, completed, total, - is_debugger_specific))); + event_type, + new ProgressEventData(progress_id, std::move(title), std::move(update), + completed, total, is_debugger_specific))); debugger.GetBroadcaster().BroadcastEvent(event_sp); } -void Debugger::ReportProgress(uint64_t progress_id, const std::string &message, - uint64_t completed, uint64_t total, +void Debugger::ReportProgress(uint64_t progress_id, std::string title, + std::string update, uint64_t completed, + uint64_t total, std::optional<lldb::user_id_t> debugger_id) { // Check if this progress is for a specific debugger. if (debugger_id) { @@ -1308,8 +1310,9 @@ // still exists. DebuggerSP debugger_sp = FindDebuggerWithID(*debugger_id); if (debugger_sp) - PrivateReportProgress(*debugger_sp, progress_id, message, completed, - total, /*is_debugger_specific*/ true); + PrivateReportProgress(*debugger_sp, progress_id, std::move(title), + std::move(update), completed, total, + /*is_debugger_specific*/ true); return; } // The progress event is not debugger specific, iterate over all debuggers @@ -1318,7 +1321,8 @@ std::lock_guard<std::recursive_mutex> guard(*g_debugger_list_mutex_ptr); DebuggerList::iterator pos, end = g_debugger_list_ptr->end(); for (pos = g_debugger_list_ptr->begin(); pos != end; ++pos) - PrivateReportProgress(*(*pos), progress_id, message, completed, total, + PrivateReportProgress(*(*pos), progress_id, title, update, completed, + total, /*is_debugger_specific*/ false); } } Index: lldb/include/lldb/Core/Progress.h =================================================================== --- lldb/include/lldb/Core/Progress.h +++ lldb/include/lldb/Core/Progress.h @@ -87,10 +87,12 @@ /// anything nor send any progress updates. /// /// @param [in] amount The amount to increment m_completed by. - void Increment(uint64_t amount = 1); + /// + /// @param [in] an optional message associated with this update. + void Increment(uint64_t amount = 1, std::string update = {}); private: - void ReportProgress(); + void ReportProgress(std::string update = {}); static std::atomic<uint64_t> g_id; /// The title of the progress activity. std::string m_title; Index: lldb/include/lldb/Core/DebuggerEvents.h =================================================================== --- lldb/include/lldb/Core/DebuggerEvents.h +++ lldb/include/lldb/Core/DebuggerEvents.h @@ -20,10 +20,11 @@ class ProgressEventData : public EventData { public: - ProgressEventData(uint64_t progress_id, const std::string &message, + ProgressEventData(uint64_t progress_id, std::string title, std::string update, uint64_t completed, uint64_t total, bool debugger_specific) - : m_message(message), m_id(progress_id), m_completed(completed), - m_total(total), m_debugger_specific(debugger_specific) {} + : m_title(std::move(title)), m_update(std::move(update)), + m_id(progress_id), m_completed(completed), m_total(total), + m_debugger_specific(debugger_specific) {} static ConstString GetFlavorString(); @@ -36,11 +37,21 @@ bool IsFinite() const { return m_total != UINT64_MAX; } uint64_t GetCompleted() const { return m_completed; } uint64_t GetTotal() const { return m_total; } - const std::string &GetMessage() const { return m_message; } + std::string GetMessage() const { + std::string message = m_title; + if (!m_update.empty()) { + message.append(": "); + message.append(m_update); + } + return message; + } + const std::string &GetTitle() const { return m_title; } + const std::string &GetUpdate() const { return m_update; } bool IsDebuggerSpecific() const { return m_debugger_specific; } private: - std::string m_message; + std::string m_title; + std::string m_update; const uint64_t m_id; uint64_t m_completed; const uint64_t m_total; Index: lldb/include/lldb/Core/Debugger.h =================================================================== --- lldb/include/lldb/Core/Debugger.h +++ lldb/include/lldb/Core/Debugger.h @@ -484,8 +484,9 @@ /// 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. - static void ReportProgress(uint64_t progress_id, const std::string &message, - uint64_t completed, uint64_t total, + static void ReportProgress(uint64_t progress_id, std::string message, + std::string update, uint64_t completed, + uint64_t total, std::optional<lldb::user_id_t> debugger_id); static void ReportDiagnosticImpl(DiagnosticEventData::Type type,
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits