[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
Fri Mar 15 09:20:44 PDT 2024
https://github.com/JDevlieghere updated https://github.com/llvm/llvm-project/pull/85329
>From 1ee7c2ffb76f13caa2052bef5dbe4f1982c8bade 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 | 216 +++++++++++++++++++++++++++++
lldb/unittests/Host/AlarmTest.cpp | 159 +++++++++++++++++++++
lldb/unittests/Host/CMakeLists.txt | 1 +
5 files changed, 489 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..80c544773d7650
--- /dev/null
+++ b/lldb/source/Host/common/Alarm.cpp
@@ -0,0 +1,216 @@
+//===-- 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) {
+ // Synchronization between the main thread and the alarm thread using a
+ // mutex and condition variable. There are 2 reasons the thread can wake up:
+ //
+ // 1. The timeout for the next alarm expired.
+ //
+ // 2. The condition variable is notified that one of our shared variables
+ // (see predicate) was modified. Either the thread is asked to shut down
+ // or a new alarm came in and we need to recompute the next timeout.
+ //
+ // Below we only deal with the timeout expiring and fall through for dealing
+ // with the rest.
+ std::unique_lock alarm_lock(m_alarm_mutex);
+ if (next_alarm) {
+ if (!m_alarm_cv.wait_until(alarm_lock, *next_alarm, predicate)) {
+ // The timeout for the next alarm expired.
+
+ // Clear the next timeout to signal that we need to recompute the next
+ // timeout.
+ next_alarm.reset();
+
+ // Iterate over all the callbacks. Call the ones that have expired
+ // and remove them from the list.
+ 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);
+ }
+
+ // Fall through after waiting on the condition variable. At this point
+ // either the predicate is true or we woke up because an alarm expired.
+
+ // The alarm thread is shutting down.
+ if (m_exit) {
+ exit = true;
+ if (m_run_callbacks_on_exit) {
+ for (Entry &entry : m_entries)
+ entry.callback();
+ }
+ continue;
+ }
+
+ // A new alarm was added or an alarm expired. Either way we need to
+ // recompute when this thread should wake up for the next alarm.
+ 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..e5895574376e38
--- /dev/null
+++ b/lldb/unittests/Host/AlarmTest.cpp
@@ -0,0 +1,159 @@
+//===-- 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;
+
+// Increase the timeout tenfold when running under ASan as it can have about the
+// same performance overhead.
+#if __has_feature(address_sanitizer)
+static constexpr auto TEST_TIMEOUT = 10000ms;
+#else
+static constexpr auto TEST_TIMEOUT = 1000ms;
+#endif
+
+// The time between scheduling a callback and it getting executed. This should
+// NOT be increased under ASan.
+static constexpr auto ALARM_TIMEOUT = 500ms;
+
+// If there are any pending callbacks, make sure they run before the Alarm
+// object is destroyed.
+static constexpr bool RUN_CALLBACKS_ON_EXIT = true;
+
+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]);
+}
+
+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]);
+}
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