[Lldb-commits] [lldb] [lldb] Implement coalescing of disjoint progress events (PR #84854)
Jonas Devlieghere via lldb-commits
lldb-commits at lists.llvm.org
Tue Mar 12 15:15:22 PDT 2024
https://github.com/JDevlieghere updated https://github.com/llvm/llvm-project/pull/84854
>From aae699eb956d1e235682b34e6407f6a9990028b3 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere <jonas at devlieghere.com>
Date: Tue, 5 Mar 2024 22:57:43 -0800
Subject: [PATCH 1/2] [lldb] Add an Alarm class
Add an Alarm class which allows scheduling callbacks after a specific
timeout has elapsed.
---
lldb/include/lldb/Host/Alarm.h | 112 +++++++++++++++++
lldb/source/Host/CMakeLists.txt | 1 +
lldb/source/Host/common/Alarm.cpp | 193 +++++++++++++++++++++++++++++
lldb/unittests/Host/AlarmTest.cpp | 164 ++++++++++++++++++++++++
lldb/unittests/Host/CMakeLists.txt | 1 +
5 files changed, 471 insertions(+)
create mode 100644 lldb/include/lldb/Host/Alarm.h
create mode 100644 lldb/source/Host/common/Alarm.cpp
create mode 100644 lldb/unittests/Host/AlarmTest.cpp
diff --git a/lldb/include/lldb/Host/Alarm.h b/lldb/include/lldb/Host/Alarm.h
new file mode 100644
index 00000000000000..7bdbc8f2b0ed74
--- /dev/null
+++ b/lldb/include/lldb/Host/Alarm.h
@@ -0,0 +1,112 @@
+//===-- Alarm.h -------------------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLDB_HOST_ALARM_H
+#define LLDB_HOST_ALARM_H
+
+#include "lldb/Host/HostThread.h"
+#include "lldb/lldb-types.h"
+#include "llvm/Support/Chrono.h"
+
+namespace lldb_private {
+
+/// \class Alarm abstraction that enables scheduling a callback function after a
+/// specified timeout. Creating an alarm for a callback returns a Handle that
+/// can be used to restart or cancel the alarm.
+class Alarm {
+public:
+ using Handle = uint64_t;
+ using Callback = std::function<void()>;
+ using TimePoint = llvm::sys::TimePoint<>;
+ using Duration = std::chrono::milliseconds;
+
+ Alarm(Duration timeout, bool run_callback_on_exit = false);
+ ~Alarm();
+
+ /// Create an alarm for the given callback. The alarm will expire and the
+ /// callback will be called after the timeout.
+ ///
+ /// \returns
+ /// Handle which can be used to restart or cancel the alarm.
+ Handle Create(Callback callback);
+
+ /// Restart the alarm for the given Handle. The alarm will expire and the
+ /// callback will be called after the timeout.
+ ///
+ /// \returns
+ /// True if the alarm was successfully restarted. False if there is no alarm
+ /// for the given Handle or the alarm already expired.
+ bool Restart(Handle handle);
+
+ /// Cancel the alarm for the given Handle. The alarm and its handle will be
+ /// removed.
+ ///
+ /// \returns
+ /// True if the alarm was successfully canceled and the Handle removed.
+ /// False if there is no alarm for the given Handle or the alarm already
+ /// expired.
+ bool Cancel(Handle handle);
+
+ static constexpr Handle INVALID_HANDLE = 0;
+
+private:
+ /// Helper functions to start, stop and check the status of the alarm thread.
+ /// @{
+ void StartAlarmThread();
+ void StopAlarmThread();
+ bool AlarmThreadRunning();
+ /// @}
+
+ /// Return an unique, monotonically increasing handle.
+ static Handle GetNextUniqueHandle();
+
+ /// Helper to compute the next time the alarm thread needs to wake up.
+ TimePoint GetNextExpiration() const;
+
+ /// Alarm entry.
+ struct Entry {
+ Handle handle;
+ Callback callback;
+ TimePoint expiration;
+
+ Entry(Callback callback, TimePoint expiration);
+ bool operator==(const Entry &rhs) { return handle == rhs.handle; }
+ };
+
+ /// List of alarm entries.
+ std::vector<Entry> m_entries;
+
+ /// Timeout between when an alarm is created and when it fires.
+ Duration m_timeout;
+
+ /// The alarm thread.
+ /// @{
+ HostThread m_alarm_thread;
+ lldb::thread_result_t AlarmThread();
+ /// @}
+
+ /// Synchronize access between the alarm thread and the main thread.
+ std::mutex m_alarm_mutex;
+
+ /// Condition variable used to wake up the alarm thread.
+ std::condition_variable m_alarm_cv;
+
+ /// Flag to signal the alarm thread that something changed and we need to
+ /// recompute the next alarm.
+ bool m_recompute_next_alarm = false;
+
+ /// Flag to signal the alarm thread to exit.
+ bool m_exit = false;
+
+ /// Flag to signal we should run all callbacks on exit.
+ bool m_run_callbacks_on_exit = false;
+};
+
+} // namespace lldb_private
+
+#endif // LLDB_HOST_ALARM_H
diff --git a/lldb/source/Host/CMakeLists.txt b/lldb/source/Host/CMakeLists.txt
index fe6e539f758fda..c2e091ee8555b7 100644
--- a/lldb/source/Host/CMakeLists.txt
+++ b/lldb/source/Host/CMakeLists.txt
@@ -13,6 +13,7 @@ macro(add_host_subdirectory group)
endmacro()
add_host_subdirectory(common
+ common/Alarm.cpp
common/FileAction.cpp
common/FileCache.cpp
common/File.cpp
diff --git a/lldb/source/Host/common/Alarm.cpp b/lldb/source/Host/common/Alarm.cpp
new file mode 100644
index 00000000000000..b9366522210361
--- /dev/null
+++ b/lldb/source/Host/common/Alarm.cpp
@@ -0,0 +1,193 @@
+//===-- Alarm.cpp ---------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "lldb/Host/Alarm.h"
+#include "lldb/Host/ThreadLauncher.h"
+#include "lldb/Utility/LLDBLog.h"
+#include "lldb/Utility/Log.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+Alarm::Alarm(Duration timeout, bool run_callback_on_exit)
+ : m_timeout(timeout), m_run_callbacks_on_exit(run_callback_on_exit) {
+ StartAlarmThread();
+}
+
+Alarm::~Alarm() { StopAlarmThread(); }
+
+Alarm::Handle Alarm::Create(std::function<void()> callback) {
+ // Gracefully deal with the unlikely event that the alarm thread failed to
+ // launch.
+ if (!AlarmThreadRunning())
+ return INVALID_HANDLE;
+
+ // Compute the next expiration before we take the lock. This ensures that
+ // waiting on the lock doesn't eat into the timeout.
+ const TimePoint expiration = GetNextExpiration();
+
+ Handle handle = INVALID_HANDLE;
+
+ {
+ std::lock_guard alarm_guard(m_alarm_mutex);
+
+ // Create a new unique entry and remember its handle.
+ m_entries.emplace_back(callback, expiration);
+ handle = m_entries.back().handle;
+
+ // Tell the alarm thread we need to recompute the next alarm.
+ m_recompute_next_alarm = true;
+ }
+
+ m_alarm_cv.notify_one();
+ return handle;
+}
+
+bool Alarm::Restart(Handle handle) {
+ // Gracefully deal with the unlikely event that the alarm thread failed to
+ // launch.
+ if (!AlarmThreadRunning())
+ return false;
+
+ // Compute the next expiration before we take the lock. This ensures that
+ // waiting on the lock doesn't eat into the timeout.
+ const TimePoint expiration = GetNextExpiration();
+
+ {
+ std::lock_guard alarm_guard(m_alarm_mutex);
+
+ // Find the entry corresponding to the given handle.
+ const auto it =
+ std::find_if(m_entries.begin(), m_entries.end(),
+ [handle](Entry &entry) { return entry.handle == handle; });
+ if (it == m_entries.end())
+ return false;
+
+ // Update the expiration.
+ it->expiration = expiration;
+
+ // Tell the alarm thread we need to recompute the next alarm.
+ m_recompute_next_alarm = true;
+ }
+
+ m_alarm_cv.notify_one();
+ return true;
+}
+
+bool Alarm::Cancel(Handle handle) {
+ // Gracefully deal with the unlikely event that the alarm thread failed to
+ // launch.
+ if (!AlarmThreadRunning())
+ return false;
+
+ {
+ std::lock_guard alarm_guard(m_alarm_mutex);
+
+ const auto it =
+ std::find_if(m_entries.begin(), m_entries.end(),
+ [handle](Entry &entry) { return entry.handle == handle; });
+
+ if (it == m_entries.end())
+ return false;
+
+ m_entries.erase(it);
+ }
+
+ // No need to notify the alarm thread. This only affects the alarm thread if
+ // we removed the entry that corresponds to the next alarm. If that's the
+ // case, the thread will wake up as scheduled, find no expired events, and
+ // recompute the next alarm time.
+ return true;
+}
+
+Alarm::Entry::Entry(Alarm::Callback callback, Alarm::TimePoint expiration)
+ : handle(Alarm::GetNextUniqueHandle()), callback(std::move(callback)),
+ expiration(std::move(expiration)) {}
+
+void Alarm::StartAlarmThread() {
+ if (!m_alarm_thread.IsJoinable()) {
+ llvm::Expected<HostThread> alarm_thread = ThreadLauncher::LaunchThread(
+ "lldb.debugger.alarm-thread", [this] { return AlarmThread(); },
+ 8 * 1024 * 1024); // Use larger 8MB stack for this thread
+ if (alarm_thread) {
+ m_alarm_thread = *alarm_thread;
+ } else {
+ LLDB_LOG_ERROR(GetLog(LLDBLog::Host), alarm_thread.takeError(),
+ "failed to launch host thread: {0}");
+ }
+ }
+}
+
+void Alarm::StopAlarmThread() {
+ if (m_alarm_thread.IsJoinable()) {
+ {
+ std::lock_guard alarm_guard(m_alarm_mutex);
+ m_exit = true;
+ }
+ m_alarm_cv.notify_one();
+ m_alarm_thread.Join(nullptr);
+ }
+}
+
+bool Alarm::AlarmThreadRunning() { return m_alarm_thread.IsJoinable(); }
+
+lldb::thread_result_t Alarm::AlarmThread() {
+ bool exit = false;
+ std::optional<TimePoint> next_alarm;
+
+ const auto predicate = [this] { return m_exit || m_recompute_next_alarm; };
+
+ while (!exit) {
+ std::unique_lock alarm_lock(m_alarm_mutex);
+
+ if (next_alarm) {
+ if (!m_alarm_cv.wait_until(alarm_lock, *next_alarm, predicate)) {
+ next_alarm.reset();
+ const TimePoint now = std::chrono::system_clock::now();
+ auto it = m_entries.begin();
+ while (it != m_entries.end()) {
+ if (it->expiration <= now) {
+ it->callback();
+ it = m_entries.erase(it);
+ } else {
+ it++;
+ }
+ }
+ }
+ } else {
+ m_alarm_cv.wait(alarm_lock, predicate);
+ }
+
+ if (m_exit) {
+ exit = true;
+ if (m_run_callbacks_on_exit) {
+ for (Entry &entry : m_entries)
+ entry.callback();
+ }
+ continue;
+ }
+
+ if (m_recompute_next_alarm || !next_alarm) {
+ for (Entry &entry : m_entries) {
+ if (!next_alarm || entry.expiration < *next_alarm)
+ next_alarm = entry.expiration;
+ }
+ m_recompute_next_alarm = false;
+ }
+ }
+ return {};
+}
+
+Alarm::TimePoint Alarm::GetNextExpiration() const {
+ return std::chrono::system_clock::now() + m_timeout;
+}
+
+Alarm::Handle Alarm::GetNextUniqueHandle() {
+ static std::atomic<Handle> g_next_handle = 1;
+ return g_next_handle++;
+}
diff --git a/lldb/unittests/Host/AlarmTest.cpp b/lldb/unittests/Host/AlarmTest.cpp
new file mode 100644
index 00000000000000..233eb3828a31b7
--- /dev/null
+++ b/lldb/unittests/Host/AlarmTest.cpp
@@ -0,0 +1,164 @@
+//===-- AlarmTest.cpp -----------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "lldb/Host/Alarm.h"
+#include "gtest/gtest.h"
+
+#include <chrono>
+#include <thread>
+
+using namespace lldb_private;
+using namespace std::chrono_literals;
+
+static constexpr auto ALARM_TIMEOUT = 500ms;
+static constexpr auto TEST_TIMEOUT = 1000ms;
+static constexpr bool RUN_CALLBACKS_ON_EXIT = true;
+
+// Enable strict checking of the ALARM_TIMEOUTs. This should only be enabled for
+// development and never during automated testing where scheduling and
+// ALARM_TIMEOUTs are unpredictable.
+#define STRICT_ALARM_TIMEOUT 1
+
+#if STRICT_ALARM_TIMEOUT
+static constexpr auto EPSILON = 10ms;
+#endif
+
+TEST(AlarmTest, Create) {
+ std::mutex m;
+
+ std::vector<Alarm::TimePoint> callbacks_actual;
+ std::vector<Alarm::TimePoint> callbacks_expected;
+
+ Alarm alarm(ALARM_TIMEOUT, RUN_CALLBACKS_ON_EXIT);
+
+ // Create 5 alarms some time apart.
+ for (size_t i = 0; i < 5; ++i) {
+ callbacks_actual.emplace_back();
+ callbacks_expected.emplace_back(std::chrono::system_clock::now() +
+ ALARM_TIMEOUT);
+
+ alarm.Create([&callbacks_actual, &m, i]() {
+ std::lock_guard guard(m);
+ callbacks_actual[i] = std::chrono::system_clock::now();
+ });
+
+ std::this_thread::sleep_for(ALARM_TIMEOUT / 5);
+ }
+
+ // Leave plenty of time for all the alarms to fire.
+ std::this_thread::sleep_for(TEST_TIMEOUT);
+
+ // Make sure all the alarms fired around the expected time.
+ for (size_t i = 0; i < 5; ++i) {
+ EXPECT_GE(callbacks_actual[i], callbacks_expected[i]);
+#if STRICT_ALARM_TIMEOUT
+ EXPECT_LE(callbacks_actual[i], callbacks_expected[i] + EPSILON);
+#endif
+ }
+}
+
+TEST(AlarmTest, Exit) {
+ std::mutex m;
+
+ std::vector<Alarm::Handle> handles;
+ std::vector<bool> callbacks;
+
+ {
+ Alarm alarm(ALARM_TIMEOUT, RUN_CALLBACKS_ON_EXIT);
+
+ // Create 5 alarms.
+ for (size_t i = 0; i < 5; ++i) {
+ callbacks.emplace_back(false);
+
+ handles.push_back(alarm.Create([&callbacks, &m, i]() {
+ std::lock_guard guard(m);
+ callbacks[i] = true;
+ }));
+ }
+
+ // Let the alarm go out of scope before any alarm had a chance to fire.
+ }
+
+ // Make sure none of the alarms fired.
+ for (bool callback : callbacks)
+ EXPECT_TRUE(callback);
+}
+
+TEST(AlarmTest, Cancel) {
+ std::mutex m;
+
+ std::vector<Alarm::Handle> handles;
+ std::vector<bool> callbacks;
+
+ Alarm alarm(ALARM_TIMEOUT, RUN_CALLBACKS_ON_EXIT);
+
+ // Create 5 alarms.
+ for (size_t i = 0; i < 5; ++i) {
+ callbacks.emplace_back(false);
+
+ handles.push_back(alarm.Create([&callbacks, &m, i]() {
+ std::lock_guard guard(m);
+ callbacks[i] = true;
+ }));
+ }
+
+ // Make sure we can cancel the first 4 alarms.
+ for (size_t i = 0; i < 4; ++i)
+ EXPECT_TRUE(alarm.Cancel(handles[i]));
+
+ // Leave plenty of time for all the alarms to fire.
+ std::this_thread::sleep_for(TEST_TIMEOUT);
+
+ // Make sure none of the first 4 alarms fired.
+ for (size_t i = 0; i < 4; ++i)
+ EXPECT_FALSE(callbacks[i]);
+
+ // Make sure the fifth alarm still fired.
+ EXPECT_TRUE(callbacks[4]);
+}
+
+TEST(AlarmTest, Restart) {
+ std::mutex m;
+
+ std::vector<Alarm::Handle> handles;
+ std::vector<Alarm::TimePoint> callbacks_actual;
+ std::vector<Alarm::TimePoint> callbacks_expected;
+
+ Alarm alarm(ALARM_TIMEOUT, RUN_CALLBACKS_ON_EXIT);
+
+ // Create 5 alarms some time apart.
+ for (size_t i = 0; i < 5; ++i) {
+ callbacks_actual.emplace_back();
+ callbacks_expected.emplace_back(std::chrono::system_clock::now() +
+ ALARM_TIMEOUT);
+
+ handles.push_back(alarm.Create([&callbacks_actual, &m, i]() {
+ std::lock_guard guard(m);
+ callbacks_actual[i] = std::chrono::system_clock::now();
+ }));
+
+ std::this_thread::sleep_for(ALARM_TIMEOUT / 5);
+ }
+
+ // Update the last 2 alarms.
+ for (size_t i = 3; i < 5; ++i) {
+ callbacks_expected[i] = std::chrono::system_clock::now() + ALARM_TIMEOUT;
+ EXPECT_TRUE(alarm.Restart(handles[i]));
+ }
+
+ // Leave plenty of time for all the alarms to fire.
+ std::this_thread::sleep_for(TEST_TIMEOUT);
+
+ // Make sure all the alarms around the expected time.
+ for (size_t i = 0; i < 5; ++i) {
+ EXPECT_GE(callbacks_actual[i], callbacks_expected[i]);
+#if STRICT_ALARM_TIMEOUT
+ EXPECT_LE(callbacks_actual[i], callbacks_expected[i] + EPSILON);
+#endif
+ }
+}
diff --git a/lldb/unittests/Host/CMakeLists.txt b/lldb/unittests/Host/CMakeLists.txt
index c959478970d184..7c09932b39c2ad 100644
--- a/lldb/unittests/Host/CMakeLists.txt
+++ b/lldb/unittests/Host/CMakeLists.txt
@@ -1,4 +1,5 @@
set (FILES
+ AlarmTest.cpp
ConnectionFileDescriptorTest.cpp
FileActionTest.cpp
FileSystemTest.cpp
>From 94faa51e888d4a83c8499608385b844fa17af78d Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere <jonas at devlieghere.com>
Date: Wed, 6 Mar 2024 17:22:43 -0800
Subject: [PATCH 2/2] [lldb] Implement coalescing of disjoint progress events
This implements coalescing of progress events using a timeout, as
discussed in the RFC on Discourse [1].
[1] https://discourse.llvm.org/t/rfc-improve-lldb-progress-reporting/75717/
---
lldb/include/lldb/Core/Progress.h | 35 +++++-
lldb/source/Core/Progress.cpp | 114 ++++++++++++++----
.../SystemInitializerCommon.cpp | 3 +
lldb/unittests/Core/ProgressReportTest.cpp | 39 +++++-
4 files changed, 159 insertions(+), 32 deletions(-)
diff --git a/lldb/include/lldb/Core/Progress.h b/lldb/include/lldb/Core/Progress.h
index c38f6dd0a140ed..e168b19e31436b 100644
--- a/lldb/include/lldb/Core/Progress.h
+++ b/lldb/include/lldb/Core/Progress.h
@@ -9,6 +9,7 @@
#ifndef LLDB_CORE_PROGRESS_H
#define LLDB_CORE_PROGRESS_H
+#include "lldb/Host/Alarm.h"
#include "lldb/lldb-forward.h"
#include "lldb/lldb-types.h"
#include "llvm/ADT/StringMap.h"
@@ -146,9 +147,12 @@ class ProgressManager {
void Increment(const Progress::ProgressData &);
void Decrement(const Progress::ProgressData &);
+ static void Initialize();
+ static void Terminate();
+ static bool Enabled();
static ProgressManager &Instance();
-private:
+protected:
enum class EventType {
Begin,
End,
@@ -156,9 +160,32 @@ class ProgressManager {
static void ReportProgress(const Progress::ProgressData &progress_data,
EventType type);
- llvm::StringMap<std::pair<uint64_t, Progress::ProgressData>>
- m_progress_category_map;
- std::mutex m_progress_map_mutex;
+ static std::optional<ProgressManager> &InstanceImpl();
+
+ /// Helper function for reporting progress when the alarm in the corresponding
+ /// entry in the map expires.
+ void Expire(llvm::StringRef key);
+
+ /// Entry used for bookkeeping.
+ struct Entry {
+ /// Reference count used for overlapping events.
+ uint64_t refcount = 0;
+
+ /// Data used to emit progress events.
+ Progress::ProgressData data;
+
+ /// Alarm handle used when the refcount reaches zero.
+ Alarm::Handle handle = Alarm::INVALID_HANDLE;
+ };
+
+ /// Map used for bookkeeping.
+ llvm::StringMap<Entry> m_entries;
+
+ /// Mutex to provide the map.
+ std::mutex m_entries_mutex;
+
+ /// Alarm instance to coalesce progress events.
+ Alarm m_alarm;
};
} // namespace lldb_private
diff --git a/lldb/source/Core/Progress.cpp b/lldb/source/Core/Progress.cpp
index b4b5e98b7ba493..0a60e033c17bfb 100644
--- a/lldb/source/Core/Progress.cpp
+++ b/lldb/source/Core/Progress.cpp
@@ -35,7 +35,10 @@ Progress::Progress(std::string title, std::string details,
std::lock_guard<std::mutex> guard(m_mutex);
ReportProgress();
- ProgressManager::Instance().Increment(m_progress_data);
+
+ // Report to the ProgressManager if that subsystem is enabled.
+ if (ProgressManager::Enabled())
+ ProgressManager::Instance().Increment(m_progress_data);
}
Progress::~Progress() {
@@ -45,7 +48,10 @@ Progress::~Progress() {
if (!m_completed)
m_completed = m_total;
ReportProgress();
- ProgressManager::Instance().Decrement(m_progress_data);
+
+ // Report to the ProgressManager if that subsystem is enabled.
+ if (ProgressManager::Enabled())
+ ProgressManager::Instance().Decrement(m_progress_data);
}
void Progress::Increment(uint64_t amount,
@@ -75,45 +81,84 @@ void Progress::ReportProgress() {
}
}
-ProgressManager::ProgressManager() : m_progress_category_map() {}
+ProgressManager::ProgressManager()
+ : m_alarm(std::chrono::milliseconds(100)), m_entries() {}
ProgressManager::~ProgressManager() {}
+void ProgressManager::Initialize() {
+ assert(!InstanceImpl() && "Already initialized.");
+ InstanceImpl().emplace();
+}
+
+void ProgressManager::Terminate() {
+ assert(InstanceImpl() && "Already terminated.");
+ InstanceImpl().reset();
+}
+
+bool ProgressManager::Enabled() { return InstanceImpl().operator bool(); }
+
ProgressManager &ProgressManager::Instance() {
- static std::once_flag g_once_flag;
- static ProgressManager *g_progress_manager = nullptr;
- std::call_once(g_once_flag, []() {
- // NOTE: known leak to avoid global destructor chain issues.
- g_progress_manager = new ProgressManager();
- });
- return *g_progress_manager;
+ assert(InstanceImpl() && "ProgressManager must be initialized");
+ return *InstanceImpl();
+}
+
+std::optional<ProgressManager> &ProgressManager::InstanceImpl() {
+ static std::optional<ProgressManager> g_progress_manager;
+ return g_progress_manager;
}
void ProgressManager::Increment(const Progress::ProgressData &progress_data) {
- std::lock_guard<std::mutex> lock(m_progress_map_mutex);
- // If the current category exists in the map then it is not an initial report,
- // therefore don't broadcast to the category bit. Also, store the current
- // progress data in the map so that we have a note of the ID used for the
- // initial progress report.
- if (!m_progress_category_map.contains(progress_data.title)) {
- m_progress_category_map[progress_data.title].second = progress_data;
+ std::lock_guard<std::mutex> lock(m_entries_mutex);
+
+ llvm::StringRef key = progress_data.title;
+ bool new_entry = !m_entries.contains(key);
+ Entry &entry = m_entries[progress_data.title];
+
+ if (new_entry) {
+ // This is a new progress event. Report progress and store the progress
+ // data.
ReportProgress(progress_data, EventType::Begin);
+ entry.data = progress_data;
+ } else if (entry.refcount == 0) {
+ // This is an existing entry that was scheduled to be deleted but a new one
+ // came in before the timer expired.
+ assert(entry.handle != Alarm::INVALID_HANDLE);
+
+ if (!m_alarm.Cancel(entry.handle)) {
+ // The timer expired before we had a chance to cancel it. We have to treat
+ // this as an entirely new progress event.
+ ReportProgress(progress_data, EventType::Begin);
+ }
+ // Clear the alarm handle.
+ entry.handle = Alarm::INVALID_HANDLE;
}
- m_progress_category_map[progress_data.title].first++;
+
+ // Regardless of how we got here, we need to bump the reference count.
+ entry.refcount++;
}
void ProgressManager::Decrement(const Progress::ProgressData &progress_data) {
- std::lock_guard<std::mutex> lock(m_progress_map_mutex);
- auto pos = m_progress_category_map.find(progress_data.title);
+ std::lock_guard<std::mutex> lock(m_entries_mutex);
+ llvm::StringRef key = progress_data.title;
- if (pos == m_progress_category_map.end())
+ if (!m_entries.contains(key))
return;
- if (pos->second.first <= 1) {
- ReportProgress(pos->second.second, EventType::End);
- m_progress_category_map.erase(progress_data.title);
- } else {
- --pos->second.first;
+ Entry &entry = m_entries[key];
+ entry.refcount--;
+
+ if (entry.refcount == 0) {
+ assert(entry.handle == Alarm::INVALID_HANDLE);
+
+ // Copy the key to a std::string so we can pass it by value to the lambda.
+ // The underlying StringRef will not exist by the time the callback is
+ // called.
+ std::string key_str = std::string(key);
+
+ // Start a timer. If it expires before we see another progress event, it
+ // will be reported.
+ entry.handle = m_alarm.Create([=]() { Expire(key_str); });
}
}
@@ -129,3 +174,20 @@ void ProgressManager::ReportProgress(
progress_data.debugger_id,
Debugger::eBroadcastBitProgressCategory);
}
+
+void ProgressManager::Expire(llvm::StringRef key) {
+ std::lock_guard<std::mutex> lock(m_entries_mutex);
+
+ // This shouldn't happen but be resilient anyway.
+ if (!m_entries.contains(key))
+ return;
+
+ // A new event came in and the alarm fired before we had a chance to restart
+ // it.
+ if (m_entries[key].refcount != 0)
+ return;
+
+ // We're done with this entry.
+ ReportProgress(m_entries[key].data, EventType::End);
+ m_entries.erase(key);
+}
diff --git a/lldb/source/Initialization/SystemInitializerCommon.cpp b/lldb/source/Initialization/SystemInitializerCommon.cpp
index 1a172a95aa1471..5d35854211c202 100644
--- a/lldb/source/Initialization/SystemInitializerCommon.cpp
+++ b/lldb/source/Initialization/SystemInitializerCommon.cpp
@@ -9,6 +9,7 @@
#include "lldb/Initialization/SystemInitializerCommon.h"
#include "Plugins/Process/gdb-remote/ProcessGDBRemoteLog.h"
+#include "lldb/Core/Progress.h"
#include "lldb/Host/FileSystem.h"
#include "lldb/Host/Host.h"
#include "lldb/Host/Socket.h"
@@ -69,6 +70,7 @@ llvm::Error SystemInitializerCommon::Initialize() {
Diagnostics::Initialize();
FileSystem::Initialize();
HostInfo::Initialize(m_shlib_dir_helper);
+ ProgressManager::Initialize();
llvm::Error error = Socket::Initialize();
if (error)
@@ -97,6 +99,7 @@ void SystemInitializerCommon::Terminate() {
#endif
Socket::Terminate();
+ ProgressManager::Terminate();
HostInfo::Terminate();
Log::DisableAllLogChannels();
FileSystem::Terminate();
diff --git a/lldb/unittests/Core/ProgressReportTest.cpp b/lldb/unittests/Core/ProgressReportTest.cpp
index 1f993180fd8392..f0d253be9bf621 100644
--- a/lldb/unittests/Core/ProgressReportTest.cpp
+++ b/lldb/unittests/Core/ProgressReportTest.cpp
@@ -22,7 +22,7 @@
using namespace lldb;
using namespace lldb_private;
-static std::chrono::milliseconds TIMEOUT(100);
+static std::chrono::milliseconds TIMEOUT(500);
class ProgressReportTest : public ::testing::Test {
public:
@@ -56,7 +56,8 @@ class ProgressReportTest : public ::testing::Test {
DebuggerSP m_debugger_sp;
ListenerSP m_listener_sp;
- SubsystemRAII<FileSystem, HostInfo, PlatformMacOSX> subsystems;
+ SubsystemRAII<FileSystem, HostInfo, PlatformMacOSX, ProgressManager>
+ subsystems;
};
TEST_F(ProgressReportTest, TestReportCreation) {
@@ -210,3 +211,37 @@ TEST_F(ProgressReportTest, TestOverlappingEvents) {
// initial report.
EXPECT_EQ(data->GetID(), expected_progress_id);
}
+
+TEST_F(ProgressReportTest, TestProgressManagerDisjointReports) {
+ ListenerSP listener_sp =
+ CreateListenerFor(Debugger::eBroadcastBitProgressCategory);
+ EventSP event_sp;
+ const ProgressEventData *data;
+ uint64_t expected_progress_id;
+
+ { Progress progress("Coalesced report 1", "Starting report 1"); }
+ { Progress progress("Coalesced report 1", "Starting report 2"); }
+ { Progress progress("Coalesced report 1", "Starting report 3"); }
+
+ ASSERT_TRUE(listener_sp->GetEvent(event_sp, TIMEOUT));
+ data = ProgressEventData::GetEventDataFromEvent(event_sp.get());
+ expected_progress_id = data->GetID();
+
+ EXPECT_EQ(data->GetDetails(), "");
+ EXPECT_FALSE(data->IsFinite());
+ EXPECT_FALSE(data->GetCompleted());
+ EXPECT_EQ(data->GetTotal(), Progress::kNonDeterministicTotal);
+ EXPECT_EQ(data->GetMessage(), "Coalesced report 1");
+
+ ASSERT_TRUE(listener_sp->GetEvent(event_sp, TIMEOUT));
+ data = ProgressEventData::GetEventDataFromEvent(event_sp.get());
+
+ EXPECT_EQ(data->GetID(), expected_progress_id);
+ EXPECT_EQ(data->GetDetails(), "");
+ EXPECT_FALSE(data->IsFinite());
+ EXPECT_TRUE(data->GetCompleted());
+ EXPECT_EQ(data->GetTotal(), Progress::kNonDeterministicTotal);
+ EXPECT_EQ(data->GetMessage(), "Coalesced report 1");
+
+ ASSERT_FALSE(listener_sp->GetEvent(event_sp, TIMEOUT));
+}
More information about the lldb-commits
mailing list