wallace updated this revision to Diff 342386.
wallace marked 12 inline comments as done.
wallace added a comment.
Address all comments
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D101128/new/
https://reviews.llvm.org/D101128
Files:
lldb/tools/lldb-vscode/ProgressEvent.cpp
lldb/tools/lldb-vscode/ProgressEvent.h
lldb/tools/lldb-vscode/VSCode.h
Index: lldb/tools/lldb-vscode/VSCode.h
===================================================================
--- lldb/tools/lldb-vscode/VSCode.h
+++ lldb/tools/lldb-vscode/VSCode.h
@@ -125,10 +125,6 @@
int64_t GetLineForPC(int64_t sourceReference, lldb::addr_t pc) const;
ExceptionBreakpoint *GetExceptionBreakpoint(const std::string &filter);
ExceptionBreakpoint *GetExceptionBreakpoint(const lldb::break_id_t bp_id);
- // Send the JSON in "json_str" to the "out" stream. Correctly send the
- // "Content-Length:" field followed by the length, followed by the raw
- // JSON bytes.
- void SendJSON(const std::string &json_str);
// Serialize the JSON value into a string and send the JSON packet to
// the "out" stream.
@@ -208,6 +204,12 @@
/// The callback to execute when the given request is triggered by the
/// IDE.
void RegisterRequestCallback(std::string request, RequestCallback callback);
+
+private:
+ // Send the JSON in "json_str" to the "out" stream. Correctly send the
+ // "Content-Length:" field followed by the length, followed by the raw
+ // JSON bytes.
+ void SendJSON(const std::string &json_str);
};
extern VSCode g_vsc;
Index: lldb/tools/lldb-vscode/ProgressEvent.h
===================================================================
--- lldb/tools/lldb-vscode/ProgressEvent.h
+++ lldb/tools/lldb-vscode/ProgressEvent.h
@@ -6,6 +6,10 @@
//
//===----------------------------------------------------------------------===//
+#include <mutex>
+#include <queue>
+#include <thread>
+
#include "VSCodeForward.h"
#include "llvm/Support/JSON.h"
@@ -28,35 +32,108 @@
llvm::json::Value ToJSON() const;
- /// This operator returns \b true if two event messages
- /// would result in the same event for the IDE, e.g.
- /// same rounded percentage.
- bool operator==(const ProgressEvent &other) const;
+ /// \return
+ /// \b true if two event messages would result in the same event for the
+ /// IDE, e.g. same rounded percentage.
+ bool EqualsForIDE(const ProgressEvent &other) const;
const char *GetEventName() const;
+ ProgressEventType GetEventType() const;
+
bool IsValid() const;
uint64_t GetID() const;
+ void MarkAsReported();
+
+ bool Reported() const;
+
+ std::chrono::duration<double> GetTimestamp() const;
+
private:
uint64_t m_progress_id;
- const char *m_message;
+ std::string m_message;
ProgressEventType m_event_type;
llvm::Optional<uint32_t> m_percentage;
+ std::chrono::duration<double> m_timestamp;
+ bool m_reported;
+};
+
+using ProgressEventReportCallback = std::function<void(ProgressEvent &)>;
+
+/// Class that keeps the start event and its most recent update.
+/// It controls when the event should start being reported to the IDE.
+class ProgressEventManager {
+public:
+ ProgressEventManager(const ProgressEvent &start_event,
+ ProgressEventReportCallback report_callback);
+
+ /// Report the start event and the most recent update if the event has lasted
+ /// for long enough.
+ ///
+ /// \return
+ /// \b false if the event hasn't finished and hasn't reported anything
+ /// yet.
+ bool TryReport();
+
+ /// Receive a new progress event for the start event and try to report it if
+ /// appropriate.
+ void Update(const ProgressEvent &event);
+
+ /// \return
+ /// \b true if a \a progressEnd event has been notified. There's no
+ /// need to try to report manually an event that has finished.
+ bool Finished() const;
+
+ /// \return
+ /// The ID of the underlying start event.
+ uint64_t GetID() const;
+
+private:
+ ProgressEvent m_start_event;
+ llvm::Optional<ProgressEvent> m_last_update_event;
+ std::chrono::duration<double> m_start_event_timestamp;
+ bool m_finished;
+ ProgressEventReportCallback m_report_callback;
};
+using ProgressEventManagerSP = std::shared_ptr<ProgressEventManager>;
+
/// Class that filters out progress event messages that shouldn't be reported
-/// to the IDE, either because they are invalid or because they are too chatty.
+/// to the IDE, because they are invalid, they carry no new information, or they
+/// don't last long enough.
+///
+/// We need to limit the amount of events that are sent to the IDE, as they slow
+/// the render thread of the UI user, and they end up spamming the DAP
+/// connection, which also takes some processing time out of the IDE.
class ProgressEventFilterQueue {
public:
- ProgressEventFilterQueue(std::function<void(ProgressEvent)> callback);
+ /// \param[in] report_callback
+ /// Function to invoke to report the event to the IDE.
+ ProgressEventFilterQueue(ProgressEventReportCallback report_callback);
+
+ ~ProgressEventFilterQueue();
+ /// Add a new event to the internal queue and reports the event if
+ /// appropriate.
void Push(const ProgressEvent &event);
private:
- std::function<void(ProgressEvent)> m_callback;
- std::map<uint64_t, ProgressEvent> m_last_events;
+ /// Report to the IDE events that haven't been reported to the IDE and have
+ /// lasted long enough.
+ void ReportWaitingEvents();
+
+ ProgressEventReportCallback m_report_callback;
+ std::map<uint64_t, ProgressEventManagerSP> m_event_managers;
+ /// Queue of event managers in chronological order
+ std::queue<ProgressEventManagerSP> m_sorted_event_managers;
+ /// Thread used to invoke \a ReportWaitingEvents periodically.
+ std::thread m_aux_reporter_thread;
+ bool m_aux_reporter_thread_should_exit;
+ /// Mutex that prevents running \a Push and \a ReportWaitingEvents
+ /// simultaneously, as both read and modify the same underlying objects.
+ std::mutex m_reporter_mutex;
};
} // namespace lldb_vscode
Index: lldb/tools/lldb-vscode/ProgressEvent.cpp
===================================================================
--- lldb/tools/lldb-vscode/ProgressEvent.cpp
+++ lldb/tools/lldb-vscode/ProgressEvent.cpp
@@ -13,28 +13,40 @@
using namespace lldb_vscode;
using namespace llvm;
-ProgressEvent::ProgressEvent(uint64_t progress_id, const char *message,
- uint64_t completed, uint64_t total)
- : m_progress_id(progress_id), m_message(message) {
+static ProgressEventType DetermineProgressEventType(uint64_t completed,
+ uint64_t total) {
if (completed == total)
- m_event_type = progressEnd;
+ return progressEnd;
else if (completed == 0)
- m_event_type = progressStart;
+ return progressStart;
else if (completed < total)
- m_event_type = progressUpdate;
+ return progressUpdate;
else
- m_event_type = progressInvalid;
+ return progressInvalid;
+}
+
+ProgressEvent::ProgressEvent(uint64_t progress_id, const char *message,
+ uint64_t completed, uint64_t total)
+ : m_progress_id(progress_id),
+ m_event_type(DetermineProgressEventType(completed, total)),
+ m_timestamp(std::chrono::system_clock::now().time_since_epoch()),
+ m_reported(false) {
+
+ if (m_event_type == progressStart)
+ m_message = message;
if (0 < total && total < UINT64_MAX)
m_percentage = (uint32_t)(((float)completed / (float)total) * 100.0);
}
-bool ProgressEvent::operator==(const ProgressEvent &other) const {
+bool ProgressEvent::EqualsForIDE(const ProgressEvent &other) const {
return m_progress_id == other.m_progress_id &&
m_event_type == other.m_event_type &&
m_percentage == other.m_percentage;
}
+ProgressEventType ProgressEvent::GetEventType() const { return m_event_type; }
+
const char *ProgressEvent::GetEventName() const {
if (m_event_type == progressStart)
return "progressStart";
@@ -65,9 +77,7 @@
body.try_emplace("cancellable", false);
}
- auto now = std::chrono::duration<double>(
- std::chrono::system_clock::now().time_since_epoch());
- std::string timestamp(llvm::formatv("{0:f9}", now.count()));
+ std::string timestamp(llvm::formatv("{0:f9}", m_timestamp.count()));
EmplaceSafeString(body, "timestamp", timestamp);
if (m_percentage)
@@ -77,17 +87,106 @@
return json::Value(std::move(event));
}
+void ProgressEvent::MarkAsReported() { m_reported = true; }
+
+bool ProgressEvent::Reported() const { return m_reported; }
+
+std::chrono::duration<double> ProgressEvent::GetTimestamp() const {
+ return m_timestamp;
+}
+
+ProgressEventManager::ProgressEventManager(
+ const ProgressEvent &start_event,
+ ProgressEventReportCallback report_callback)
+ : m_start_event(start_event),
+ m_start_event_timestamp(
+ std::chrono::system_clock::now().time_since_epoch()),
+ m_finished(false), m_report_callback(report_callback) {}
+
+bool ProgressEventManager::TryReport() {
+ // We only report if we have already started reporting or if enough time has
+ // passed since the start event.
+ if (!m_start_event.Reported() &&
+ std::chrono::system_clock::now().time_since_epoch() -
+ m_start_event.GetTimestamp() <
+ std::chrono::seconds(1))
+ return false;
+
+ if (!m_start_event.Reported() && !Finished()) {
+ m_report_callback(m_start_event);
+ m_start_event.MarkAsReported();
+ }
+ if (m_last_update_event && !m_last_update_event->Reported()) {
+ m_report_callback(*m_last_update_event);
+ m_last_update_event->MarkAsReported();
+ }
+ return true;
+}
+
+void ProgressEventManager::Update(const ProgressEvent &event) {
+ if (event.GetEventType() == progressEnd)
+ m_finished = true;
+
+ if (!m_last_update_event || !event.EqualsForIDE(*m_last_update_event)) {
+ m_last_update_event = event;
+ TryReport();
+ }
+}
+
+uint64_t ProgressEventManager::GetID() const { return m_start_event.GetID(); }
+
+bool ProgressEventManager::Finished() const { return m_finished; }
+
ProgressEventFilterQueue::ProgressEventFilterQueue(
- std::function<void(ProgressEvent)> callback)
- : m_callback(callback) {}
+ ProgressEventReportCallback report_callback)
+ : m_report_callback(report_callback) {
+ m_aux_reporter_thread_should_exit = false;
+ m_aux_reporter_thread = std::thread([&] {
+ while (!m_aux_reporter_thread_should_exit) {
+ std::this_thread::sleep_for(std::chrono::seconds(2));
+ ReportWaitingEvents();
+ }
+ });
+}
+
+ProgressEventFilterQueue::~ProgressEventFilterQueue() {
+ m_aux_reporter_thread_should_exit = true;
+ m_aux_reporter_thread.join();
+}
+
+void ProgressEventFilterQueue::ReportWaitingEvents() {
+ std::lock_guard<std::mutex> locker(m_reporter_mutex);
+
+ while (!m_sorted_event_managers.empty()) {
+ ProgressEventManagerSP event_manager = m_sorted_event_managers.front();
+ if (event_manager->Finished())
+ m_sorted_event_managers.pop();
+ else if (event_manager->TryReport())
+ m_sorted_event_managers
+ .pop(); // we remove it from the queue as it started reporting
+ // already, the Push method will be able to continue its
+ // reports.
+ else
+ break; // If we couldn't report it, then the next event in the queue won't
+ // be able as well, as it came later.
+ }
+}
void ProgressEventFilterQueue::Push(const ProgressEvent &event) {
if (!event.IsValid())
return;
- auto it = m_last_events.find(event.GetID());
- if (it == m_last_events.end() || !(it->second == event)) {
- m_last_events[event.GetID()] = event;
- m_callback(event);
+ std::lock_guard<std::mutex> locker(m_reporter_mutex);
+
+ auto it = m_event_managers.find(event.GetID());
+ if (it == m_event_managers.end()) {
+ ProgressEventManagerSP event_manager =
+ std::make_shared<ProgressEventManager>(event, m_report_callback);
+ m_event_managers.insert({event.GetID(), event_manager});
+ m_sorted_event_managers.push(event_manager);
+ } else {
+ it->second->Update(event);
+ if (it->second->Finished())
+ m_event_managers.erase(it);
}
}
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
