[Lldb-commits] [lldb] [lldb] Add an Alarm class for coalescing progress reports (PR #85329)
Jonas Devlieghere via lldb-commits
lldb-commits at lists.llvm.org
Thu Mar 14 16:11:15 PDT 2024
https://github.com/JDevlieghere created https://github.com/llvm/llvm-project/pull/85329
The commit introduces a new, generic, Alarm class. The class lets you to schedule functions (callbacks) that will execute after a predefined timeout. Once scheduled, you can cancel and reset a callback, given the timeout hasn't expired yet.
The alarm class worker thread that sleeps until the next timeout expires. When the thread wakes up, it checks for all the callbacks that have expired and calls them in order. Because the callback is called from the worker thread, the only guarantee is that a callback is called no sooner than the timeout. A long running callback could potentially block the worker threads and delay other callbacks from getting called.
I intentionally kept the implementation as simple as possible while addressing the needs for the use case of coalescing progress events as discussed in [1]. If we want to rely on this somewhere else, we can reassess whether we need to address this class' limitations.
[1] https://discourse.llvm.org/t/rfc-improve-lldb-progress-reporting/75717/
>From 39972360d762f6185d59d54217870945482ecff7 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] [lldb] Add an Alarm class for coalescing progress reports
The commit introduces a new, generic, Alarm class. The class lets you to
schedule functions (callbacks) that will execute after a predefined
timeout. Once scheduled, you can cancel and reset a callback, given the
timeout hasn't expired yet.
The alarm class worker thread that sleeps until the next timeout
expires. When the thread wakes up, it checks for all the callbacks that
have expired and calls them in order. Because the callback is called
from the worker thread, the only guarantee is that a callback is called
no sooner than the timeout. A long running callback could potentially
block the worker threads and delay other callbacks from getting called.
I intentionally kept the implementation as simple as possible while
addressing the needs for the use case of coalescing progress events as
discussed in [1]. If we want to rely on this somewhere else, we can
reassess whether we need to address this class' limitations.
[1] https://discourse.llvm.org/t/rfc-improve-lldb-progress-reporting/75717/
---
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
More information about the lldb-commits
mailing list