https://github.com/JDevlieghere created https://github.com/llvm/llvm-project/pull/84854
This implements coalescing of progress events using a timeout, as discussed in the RFC on Discourse [1]. This PR consists of two commits which, depending on the feedback, I may split up into two PRs. For now, I think it's easier to review this as a whole. 1. The first commit introduces a new generic `Alarm` class. The class lets you to schedule a function (callback) to be executed after a given timeout expires. You can cancel and reset a callback before its corresponding timeout expires. It achieves this with the help of a worker thread that sleeps until the next timeout expires. The only guarantee it provides is that your function is called no sooner than the requested timeout. Because the callback is called directly from the worker thread, a long running callback could potentially block the worker thread. I intentionally kept the implementation as simple as possible while addressing the needs for the `ProgressManager` use case. If we want to rely on this somewhere else, we can reassess whether we need to address those limitations. 2. The second commit uses the Alarm class to coalesce progress events. To recap the Discourse discussion, when multiple progress events with the same title execute in close succession, they get broadcast as one to `eBroadcastBitProgressCategory`. The `ProgressManager` keeps track of the in-flight progress events and when the refcount hits zero, the Alarm class is used to schedule broadcasting the event. If a new progress event comes in before the alarm fires, the alarm is reset (and the process repeats when the new progress event ends). If no new event comes in before the timeout expires, the progress event is broadcast. [1] https://discourse.llvm.org/t/rfc-improve-lldb-progress-reporting/75717/ >From 3aae84b8caaf904c11b1dab620893620a0fa1c85 Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere <jo...@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 | 88 +++++++++++++ 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, 447 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..8b4180fbb32835 --- /dev/null +++ b/lldb/include/lldb/Host/Alarm.h @@ -0,0 +1,88 @@ +//===-- 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 { +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(); + + Handle Create(Callback callback); + bool Restart(Handle handle); + 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(); + + 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..cbbf96f3645bc9 --- /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 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 first 4 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 d456f257bc2746a0075a11f799e0ccb7c557a639 Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere <jo...@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 | 21 +++- lldb/source/Core/Progress.cpp | 114 ++++++++++++++---- .../SystemInitializerCommon.cpp | 3 + lldb/unittests/Core/ProgressReportTest.cpp | 56 ++++++++- 4 files changed, 163 insertions(+), 31 deletions(-) diff --git a/lldb/include/lldb/Core/Progress.h b/lldb/include/lldb/Core/Progress.h index c38f6dd0a140ed..67d96b01e67c4a 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,18 @@ 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(); + + void Expire(llvm::StringRef key); + struct Entry { + uint64_t refcount = 0; + Progress::ProgressData data; + Alarm::Handle handle = Alarm::INVALID_HANDLE; + }; + + Alarm m_alarm; + llvm::StringMap<Entry> m_entries; + std::mutex m_entries_mutex; }; } // namespace lldb_private diff --git a/lldb/source/Core/Progress.cpp b/lldb/source/Core/Progress.cpp index b4b5e98b7ba493..da6fe8843dab6d 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,86 @@ 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; + + // This is a new progress event. + if (!m_entries.contains(key)) { ReportProgress(progress_data, EventType::Begin); + Entry &entry = m_entries[key]; + entry.data = progress_data; + entry.refcount = 1; + return; + } + + // This is an existing progress event. + Entry &entry = m_entries[key]; + + // The progress event was scheduled to be deleted but a new one came in before + // the timer expired. + if (entry.refcount == 0) { + 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); + } + entry.refcount = 1; + entry.handle = Alarm::INVALID_HANDLE; + } else { + entry.refcount++; } - m_progress_category_map[progress_data.title].first++; } 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); + Entry &entry = m_entries[key]; + if (entry.refcount <= 1) { + assert(entry.handle == Alarm::INVALID_HANDLE); + // Start a timer. If it expires before we see another progress event, it + // will be reported. + entry.refcount = 0; + // 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); + entry.handle = m_alarm.Create([=]() { Expire(key_str); }); } else { - --pos->second.first; + entry.refcount--; } } @@ -129,3 +176,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..e45bc4ac0a3917 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,54 @@ TEST_F(ProgressReportTest, TestOverlappingEvents) { // initial report. EXPECT_EQ(data->GetID(), expected_progress_id); } + +TEST_F(ProgressReportTest, TestProgressManagerDisjointReports) { + // Set up the debugger, make sure that was done properly. + ArchSpec arch("x86_64-apple-macosx-"); + Platform::SetHostPlatform(PlatformRemoteMacOSX::CreateInstance(true, &arch)); + + DebuggerSP debugger_sp = Debugger::CreateInstance(); + ASSERT_TRUE(debugger_sp); + + // Get the debugger's broadcaster. + Broadcaster &broadcaster = debugger_sp->GetBroadcaster(); + + // Create a listener, make sure it can receive events and that it's + // listening to the correct broadcast bit. + ListenerSP listener_sp = Listener::MakeListener("progress-category-listener"); + + listener_sp->StartListeningForEvents(&broadcaster, + Debugger::eBroadcastBitProgressCategory); + ASSERT_TRUE(broadcaster.EventTypeHasListeners( + 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)); +} _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
