diff --git a/lldb/include/lldb/Core/Progress.h b/lldb/include/lldb/Core/Progress.h index 421e435a9e685..f6cea282842e1 100644 --- a/lldb/include/lldb/Core/Progress.h +++ b/lldb/include/lldb/Core/Progress.h @@ -10,6 +10,7 @@ #define LLDB_CORE_PROGRESS_H #include "lldb/Host/Alarm.h" +#include "lldb/Utility/Timeout.h" #include "lldb/lldb-forward.h" #include "lldb/lldb-types.h" #include "llvm/ADT/StringMap.h" @@ -81,7 +82,8 @@ class Progress { /// progress is to be reported only to specific debuggers. Progress(std::string title, std::string details = {}, std::optional total = std::nullopt, - lldb_private::Debugger *debugger = nullptr); + lldb_private::Debugger *debugger = nullptr, + Timeout minimum_report_time = std::nullopt); /// Destroy the progress object. /// @@ -121,21 +123,32 @@ class Progress { private: void ReportProgress(); static std::atomic g_id; - /// More specific information about the current file being displayed in the - /// report. - std::string m_details; - /// How much work ([0...m_total]) that has been completed. - uint64_t m_completed; + /// Total amount of work, use a std::nullopt in the constructor for non /// deterministic progress. - uint64_t m_total; - std::mutex m_mutex; - /// 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; + const uint64_t m_total; + + // Minimum amount of time between two progress reports. + const Timeout m_minimum_report_time; + /// Data needed by the debugger to broadcast a progress event. - ProgressData m_progress_data; + const ProgressData m_progress_data; + + /// How much work ([0...m_total]) that has been completed. + std::atomic m_completed = 0; + + /// Time (in nanoseconds since epoch) of the last progress report. + std::atomic m_last_report_time_ns; + + /// Guards non-const non-atomic members of the class. + std::mutex m_mutex; + + /// More specific information about the current file being displayed in the + /// report. + std::string m_details; + + /// The "completed" value of the last reported event. + std::optional m_prev_completed; }; /// A class used to group progress reports by category. This is done by using a diff --git a/lldb/source/Core/Progress.cpp b/lldb/source/Core/Progress.cpp index c9a556472c06b..ed8dfb85639b7 100644 --- a/lldb/source/Core/Progress.cpp +++ b/lldb/source/Core/Progress.cpp @@ -11,7 +11,8 @@ #include "lldb/Core/Debugger.h" #include "lldb/Utility/StreamString.h" #include "llvm/Support/Signposts.h" - +#include +#include #include #include #include @@ -26,17 +27,18 @@ static llvm::ManagedStatic g_progress_signposts; Progress::Progress(std::string title, std::string details, std::optional total, - lldb_private::Debugger *debugger) - : m_details(details), m_completed(0), - m_total(Progress::kNonDeterministicTotal), + lldb_private::Debugger *debugger, + Timeout minimum_report_time) + : m_total(total.value_or(Progress::kNonDeterministicTotal)), + m_minimum_report_time(minimum_report_time), m_progress_data{title, ++g_id, - /*m_progress_data.debugger_id=*/std::nullopt} { - if (total) - m_total = *total; - - if (debugger) - m_progress_data.debugger_id = debugger->GetID(); - + debugger ? std::optional(debugger->GetID()) + : std::nullopt}, + m_last_report_time_ns( + std::chrono::nanoseconds( + std::chrono::steady_clock::now().time_since_epoch()) + .count()), + m_details(std::move(details)) { std::lock_guard guard(m_mutex); ReportProgress(); @@ -65,29 +67,49 @@ Progress::~Progress() { void Progress::Increment(uint64_t amount, std::optional updated_detail) { - if (amount > 0) { - std::lock_guard 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 the total. - if (m_total && (amount > (m_total - m_completed))) - m_completed = m_total; - else - m_completed += amount; - ReportProgress(); + if (amount == 0) + return; + + m_completed.fetch_add(amount, std::memory_order_relaxed); + + if (m_minimum_report_time) { + using namespace std::chrono; + + nanoseconds now; + uint64_t last_report_time_ns = + m_last_report_time_ns.load(std::memory_order_relaxed); + + do { + now = steady_clock::now().time_since_epoch(); + if (now < nanoseconds(last_report_time_ns) + *m_minimum_report_time) + return; // Too little time has passed since the last report. + + } while (!m_last_report_time_ns.compare_exchange_weak( + last_report_time_ns, now.count(), std::memory_order_relaxed, + std::memory_order_relaxed)); } + + std::lock_guard guard(m_mutex); + if (updated_detail) + m_details = std::move(updated_detail.value()); + ReportProgress(); } void Progress::ReportProgress() { - 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_progress_data.progress_id, m_progress_data.title, - m_details, m_completed, m_total, - m_progress_data.debugger_id); - } + // NB: Comparisons with optional rely on the fact that std::nullopt is + // "smaller" than zero. + if (m_prev_completed >= m_total) + return; // We've reported completion already. + + uint64_t completed = + std::min(m_completed.load(std::memory_order_relaxed), m_total); + if (completed < m_prev_completed) + return; // An overflow in the m_completed counter. Just ignore these events. + + Debugger::ReportProgress(m_progress_data.progress_id, m_progress_data.title, + m_details, completed, m_total, + m_progress_data.debugger_id); + m_prev_completed = completed; } ProgressManager::ProgressManager() diff --git a/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp b/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp index 5b325e30bef43..6f2c45e74132c 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp @@ -80,7 +80,8 @@ void ManualDWARFIndex::Index() { // 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); + total_progress, /*debugger=*/nullptr, + /*minimum_report_time=*/std::chrono::milliseconds(20)); // Share one thread pool across operations to avoid the overhead of // recreating the threads. diff --git a/lldb/unittests/Core/ProgressReportTest.cpp b/lldb/unittests/Core/ProgressReportTest.cpp index 0149b1de77add..d03b3bc39f8e0 100644 --- a/lldb/unittests/Core/ProgressReportTest.cpp +++ b/lldb/unittests/Core/ProgressReportTest.cpp @@ -18,6 +18,7 @@ #include "gtest/gtest.h" #include #include +#include using namespace lldb; using namespace lldb_private; @@ -208,6 +209,110 @@ TEST_F(ProgressReportTest, TestReportDestructionWithPartialProgress) { EXPECT_EQ(data->GetMessage(), "Infinite progress: Report 2"); } +TEST_F(ProgressReportTest, TestFiniteOverflow) { + ListenerSP listener_sp = CreateListenerFor(lldb::eBroadcastBitProgress); + EventSP event_sp; + const ProgressEventData *data; + + // Increment the report beyond its limit and make sure we only get one + // completed event. + { + Progress progress("Finite progress", "Report 1", 10); + progress.Increment(11); + progress.Increment(47); + } + + ASSERT_TRUE(listener_sp->GetEvent(event_sp, TIMEOUT)); + data = ProgressEventData::GetEventDataFromEvent(event_sp.get()); + EXPECT_TRUE(data->IsFinite()); + EXPECT_EQ(data->GetCompleted(), 0); + EXPECT_EQ(data->GetTotal(), 10); + + ASSERT_TRUE(listener_sp->GetEvent(event_sp, TIMEOUT)); + data = ProgressEventData::GetEventDataFromEvent(event_sp.get()); + EXPECT_TRUE(data->IsFinite()); + EXPECT_EQ(data->GetCompleted(), 10); + EXPECT_EQ(data->GetTotal(), 10); + + ASSERT_FALSE(listener_sp->GetEvent(event_sp, TIMEOUT)); +} + +TEST_F(ProgressReportTest, TestNonDeterministicOverflow) { + ListenerSP listener_sp = CreateListenerFor(lldb::eBroadcastBitProgress); + EventSP event_sp; + const ProgressEventData *data; + constexpr uint64_t max_minus_1 = std::numeric_limits::max() - 1; + + // Increment the report beyond its limit and make sure we only get one + // completed event. The event which overflows the counter should be ignored. + { + Progress progress("Non deterministic progress", "Report 1"); + progress.Increment(max_minus_1); + progress.Increment(max_minus_1); + } + + ASSERT_TRUE(listener_sp->GetEvent(event_sp, TIMEOUT)); + data = ProgressEventData::GetEventDataFromEvent(event_sp.get()); + EXPECT_FALSE(data->IsFinite()); + EXPECT_EQ(data->GetCompleted(), 0); + EXPECT_EQ(data->GetTotal(), Progress::kNonDeterministicTotal); + + ASSERT_TRUE(listener_sp->GetEvent(event_sp, TIMEOUT)); + data = ProgressEventData::GetEventDataFromEvent(event_sp.get()); + EXPECT_FALSE(data->IsFinite()); + EXPECT_EQ(data->GetCompleted(), max_minus_1); + EXPECT_EQ(data->GetTotal(), Progress::kNonDeterministicTotal); + + ASSERT_TRUE(listener_sp->GetEvent(event_sp, TIMEOUT)); + data = ProgressEventData::GetEventDataFromEvent(event_sp.get()); + EXPECT_FALSE(data->IsFinite()); + EXPECT_EQ(data->GetCompleted(), Progress::kNonDeterministicTotal); + EXPECT_EQ(data->GetTotal(), Progress::kNonDeterministicTotal); + + ASSERT_FALSE(listener_sp->GetEvent(event_sp, TIMEOUT)); +} + +TEST_F(ProgressReportTest, TestMinimumReportTime) { + ListenerSP listener_sp = CreateListenerFor(lldb::eBroadcastBitProgress); + EventSP event_sp; + const ProgressEventData *data; + + { + Progress progress("Finite progress", "Report 1", /*total=*/20, + m_debugger_sp.get(), + /*minimum_report_time=*/std::chrono::seconds(1)); + // Send 10 events in quick succession. These should not generate any events. + for (int i = 0; i < 10; ++i) + progress.Increment(); + + // Sleep, then send 10 more. This should generate one event for the first + // increment, and then another for completion. + std::this_thread::sleep_for(std::chrono::seconds(1)); + for (int i = 0; i < 10; ++i) + progress.Increment(); + } + + ASSERT_TRUE(listener_sp->GetEvent(event_sp, TIMEOUT)); + data = ProgressEventData::GetEventDataFromEvent(event_sp.get()); + EXPECT_TRUE(data->IsFinite()); + EXPECT_EQ(data->GetCompleted(), 0); + EXPECT_EQ(data->GetTotal(), 20); + + ASSERT_TRUE(listener_sp->GetEvent(event_sp, TIMEOUT)); + data = ProgressEventData::GetEventDataFromEvent(event_sp.get()); + EXPECT_TRUE(data->IsFinite()); + EXPECT_EQ(data->GetCompleted(), 11); + EXPECT_EQ(data->GetTotal(), 20); + + ASSERT_TRUE(listener_sp->GetEvent(event_sp, TIMEOUT)); + data = ProgressEventData::GetEventDataFromEvent(event_sp.get()); + EXPECT_TRUE(data->IsFinite()); + EXPECT_EQ(data->GetCompleted(), 20); + EXPECT_EQ(data->GetTotal(), 20); + + ASSERT_FALSE(listener_sp->GetEvent(event_sp, TIMEOUT)); +} + TEST_F(ProgressReportTest, TestProgressManager) { ListenerSP listener_sp = CreateListenerFor(lldb::eBroadcastBitProgressCategory);