labath created this revision. labath added reviewers: JDevlieghere, saugustine, rupprecht. Herald added a project: All. labath requested review of this revision. Herald added a project: LLDB.
Have the Progress class spawn a thread to periodically send progress reports. The reporting period could be made configurable, but for now I've hardcoded it to 100ms. (This is the main WIP part) It could be argued that creating a thread for progress reporting adds overhead, but I would counter that by saying "If the task is so fast that creating a thread noticably slows it down, then it really doesn't need progress reporting". For me, this speeds up DWARF indexing by about 1.5% (which is only slightly above the error bars), but I expect it will have a much bigger impact in situations where printing a single progress update takes a nontrivial amount of time. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D152364 Files: lldb/include/lldb/Core/Progress.h lldb/source/Core/Progress.cpp
Index: lldb/source/Core/Progress.cpp =================================================================== --- lldb/source/Core/Progress.cpp +++ lldb/source/Core/Progress.cpp @@ -9,6 +9,8 @@ #include "lldb/Core/Progress.h" #include "lldb/Core/Debugger.h" +#include "lldb/Host/ThreadLauncher.h" +#include "lldb/Utility/LLDBLog.h" #include "lldb/Utility/StreamString.h" using namespace lldb; @@ -22,39 +24,67 @@ assert(total > 0); if (debugger) m_debugger_id = debugger->GetID(); - std::lock_guard<std::mutex> guard(m_mutex); - ReportProgress(); + + // Using a shared_future because std::function needs to be copyable. + if (llvm::Expected<HostThread> reporting_thread = + ThreadLauncher::LaunchThread( + "<lldb.progress>", + [this, future = std::shared_future<void>( + m_stop_reporting_thread.get_future())]() { + SendPeriodicReports(future); + return lldb::thread_result_t(); + })) { + m_reporting_thread = std::move(*reporting_thread); + } else { + LLDB_LOG_ERROR(GetLog(LLDBLog::Host), reporting_thread.takeError(), + "failed to launch host thread: {}"); + } } Progress::~Progress() { - // Make sure to always report progress completed when this object is - // 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; - ReportProgress(); + m_stop_reporting_thread.set_value(); + if (m_reporting_thread.IsJoinable()) { + m_reporting_thread.Join(nullptr); } } -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 - // much and exceed m_total. - if (amount > (m_total - m_completed)) - m_completed = m_total; - else - m_completed += amount; - ReportProgress(update); +void Progress::SendPeriodicReports(std::shared_future<void> done) { + uint64_t last_completed = 0; + Debugger::ReportProgress(m_id, m_title, "", last_completed, m_total, + m_debugger_id); + + while (last_completed != m_total && + done.wait_for(std::chrono::milliseconds(100)) == + std::future_status::timeout) { + uint64_t current_completed = m_completed.load(); + if (current_completed == last_completed) + continue; + + if (current_completed == m_total || + current_completed < last_completed /*overflow*/) { + break; + } + + std::string current_update; + { + std::lock_guard<std::mutex> guard(m_update_mutex); + current_update = std::move(m_update); + m_update.clear(); + } + Debugger::ReportProgress(m_id, m_title, std::move(current_update), + current_completed, m_total, m_debugger_id); + last_completed = current_completed; } + + Debugger::ReportProgress(m_id, m_title, "", m_total, m_total, m_debugger_id); } -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, std::move(update), m_completed, - m_total, m_debugger_id); +void Progress::Increment(uint64_t amount, std::string update) { + if (amount == 0) + return; + if (!update.empty()) { + std::lock_guard<std::mutex> guard(m_update_mutex); + m_update = std::move(update); } + m_completed += amount; } Index: lldb/include/lldb/Core/Progress.h =================================================================== --- lldb/include/lldb/Core/Progress.h +++ lldb/include/lldb/Core/Progress.h @@ -9,9 +9,11 @@ #ifndef LLDB_CORE_PROGRESS_H #define LLDB_CORE_PROGRESS_H +#include "lldb/Host/HostThread.h" #include "lldb/Utility/ConstString.h" #include "lldb/lldb-types.h" #include <atomic> +#include <future> #include <mutex> #include <optional> @@ -92,24 +94,26 @@ void Increment(uint64_t amount = 1, std::string update = {}); private: - void ReportProgress(std::string update = {}); + void SendPeriodicReports(std::shared_future<void> done); + void ReportProgress(std::string update); static std::atomic<uint64_t> g_id; /// The title of the progress activity. std::string m_title; - 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; + std::atomic<uint64_t> m_completed; /// Total amount of work, UINT64_MAX for non deterministic progress. const 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; - /// Set to true when progress has been reported where m_completed == m_total - /// to ensure that we don't send progress updates after progress has - /// completed. - bool m_complete = false; + + std::mutex m_update_mutex; + std::string m_update; + + std::promise<void> m_stop_reporting_thread; + HostThread m_reporting_thread; }; } // namespace lldb_private
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits