wallace updated this revision to Diff 342650.
wallace marked 12 inline comments as done.
wallace added a comment.
Address all comments
- now the reporting timing conditions are in ProgressEvent, which simplifies
the other classes
- removed the progressInvalid enum by making sure only valid ProgressEvent
instances are created
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.cpp
lldb/tools/lldb-vscode/VSCode.h
lldb/tools/lldb-vscode/lldb-vscode.cpp
Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===================================================================
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -376,8 +376,7 @@
const char *message = lldb::SBDebugger::GetProgressFromEvent(
event, progress_id, completed, total, is_debugger_specific);
if (message)
- g_vsc.SendProgressEvent(
- ProgressEvent(progress_id, message, completed, total));
+ g_vsc.SendProgressEvent(progress_id, message, completed, total);
}
}
}
Index: lldb/tools/lldb-vscode/VSCode.h
===================================================================
--- lldb/tools/lldb-vscode/VSCode.h
+++ lldb/tools/lldb-vscode/VSCode.h
@@ -114,7 +114,7 @@
uint32_t reverse_request_seq;
std::map<std::string, RequestCallback> request_handlers;
bool waiting_for_run_in_terminal;
- ProgressEventFilterQueue progress_event_queue;
+ ProgressEventReporter progress_event_reporter;
// Keep track of the last stop thread index IDs as threads won't go away
// unless we send a "thread" event to indicate the thread exited.
llvm::DenseSet<lldb::tid_t> thread_ids;
@@ -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.
@@ -138,7 +134,8 @@
void SendOutput(OutputType o, const llvm::StringRef output);
- void SendProgressEvent(const ProgressEvent &event);
+ void SendProgressEvent(uint64_t progress_id, const char *message,
+ uint64_t completed, uint64_t total);
void __attribute__((format(printf, 3, 4)))
SendFormattedOutput(OutputType o, const char *format, ...);
@@ -208,6 +205,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/VSCode.cpp
===================================================================
--- lldb/tools/lldb-vscode/VSCode.cpp
+++ lldb/tools/lldb-vscode/VSCode.cpp
@@ -42,7 +42,7 @@
focus_tid(LLDB_INVALID_THREAD_ID), sent_terminated_event(false),
stop_at_entry(false), is_attach(false), reverse_request_seq(0),
waiting_for_run_in_terminal(false),
- progress_event_queue(
+ progress_event_reporter(
[&](const ProgressEvent &event) { SendJSON(event.ToJSON()); }) {
const char *log_file_path = getenv("LLDBVSCODE_LOG");
#if defined(_WIN32)
@@ -322,8 +322,9 @@
// };
// }
-void VSCode::SendProgressEvent(const ProgressEvent &event) {
- progress_event_queue.Push(event);
+void VSCode::SendProgressEvent(uint64_t progress_id, const char *message,
+ uint64_t completed, uint64_t total) {
+ progress_event_reporter.Push(progress_id, message, completed, total);
}
void __attribute__((format(printf, 3, 4)))
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"
@@ -13,50 +17,127 @@
namespace lldb_vscode {
enum ProgressEventType {
- progressInvalid,
progressStart,
progressUpdate,
progressEnd
};
+class ProgressEvent;
+using ProgressEventReportCallback = std::function<void(ProgressEvent &)>;
+
class ProgressEvent {
public:
- ProgressEvent() {}
+ /// Constructor for an start event
+ ProgressEvent(uint64_t progress_id, llvm::StringRef message,
+ llvm::Optional<uint32_t> percentage);
- ProgressEvent(uint64_t progress_id, const char *message, uint64_t completed,
- uint64_t total);
+ /// Constructor for an update or end event
+ ProgressEvent(uint64_t progress_id, const ProgressEvent &prev_event);
+
+ /// Constructor for an update or end event, which happens when percentage is
+ /// 100.
+ ProgressEvent(uint64_t progress_id, uint32_t percentage,
+ const ProgressEvent &prev_event);
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;
- bool IsValid() const;
+ ProgressEventType GetEventType() const;
+
+ /// Report this progress event to the provided callback only if enough time
+ /// has passed since the creation of the event and since the previous reported
+ /// update.
+ bool Report(ProgressEventReportCallback callback);
- uint64_t GetID() const;
+ bool Reported() 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_creation_time =
+ std::chrono::system_clock::now().time_since_epoch();
+ std::chrono::duration<double> m_minimum_allowed_report_time;
+ bool m_reported = false;
};
+/// 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 ReportIfNeeded();
+
+ /// Receive a new progress event for the start event and try to report it if
+ /// appropriate.
+ void Update(uint64_t progress_id, uint64_t completed, uint64_t total);
+
+ /// \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;
+
+ const ProgressEvent &GetMostRecentEvent() const;
+
+private:
+ ProgressEvent m_start_event;
+ llvm::Optional<ProgressEvent> m_last_update_event;
+ 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.
-class ProgressEventFilterQueue {
+/// 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 ProgressEventReporter {
public:
- ProgressEventFilterQueue(std::function<void(ProgressEvent)> callback);
+ /// \param[in] report_callback
+ /// Function to invoke to report the event to the IDE.
+ ProgressEventReporter(ProgressEventReportCallback report_callback);
+
+ ~ProgressEventReporter();
- void Push(const ProgressEvent &event);
+ /// Add a new event to the internal queue and report the event if
+ /// appropriate.
+ void Push(uint64_t progress_id, const char *message, uint64_t completed,
+ uint64_t total);
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 ReportStartEvents();
+
+ ProgressEventReportCallback m_report_callback;
+ std::map<uint64_t, ProgressEventManagerSP> m_event_managers;
+ /// Queue of start events in chronological order
+ std::queue<ProgressEventManagerSP> m_unreported_start_events;
+ /// Thread used to invoke \a ReportStartEvents periodically.
+ std::thread m_thread;
+ bool m_thread_should_exit;
+ /// Mutex that prevents running \a Push and \a ReportStartEvents
+ /// simultaneously, as both read and modify the same underlying objects.
+ std::mutex m_mutex;
};
} // namespace lldb_vscode
Index: lldb/tools/lldb-vscode/ProgressEvent.cpp
===================================================================
--- lldb/tools/lldb-vscode/ProgressEvent.cpp
+++ lldb/tools/lldb-vscode/ProgressEvent.cpp
@@ -13,43 +13,87 @@
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) {
- if (completed == total)
- m_event_type = progressEnd;
- else if (completed == 0)
- m_event_type = progressStart;
- else if (completed < total)
- m_event_type = progressUpdate;
+// The minimum duration of an event for it to be reported
+const std::chrono::duration<double> kStartProgressEventReportDelay =
+ std::chrono::seconds(1);
+// The minimum time interval between update events for reporting. If multiple
+// updates fall within the same time interval, only the latest is reported.
+const std::chrono::duration<double> kUpdateProgressEventReportDelay =
+ kStartProgressEventReportDelay / 4;
+
+ProgressEvent::ProgressEvent(uint64_t progress_id, StringRef message,
+ Optional<uint32_t> percentage)
+ : m_progress_id(progress_id), m_message(message.str()),
+ m_event_type(progressStart), m_percentage(percentage),
+ m_minimum_allowed_report_time(m_creation_time +
+ kStartProgressEventReportDelay) {}
+
+ProgressEvent::ProgressEvent(uint64_t progress_id, uint32_t percentage,
+ const ProgressEvent &prev_event)
+ : m_progress_id(progress_id),
+ m_event_type(percentage == 100 ? progressEnd : progressUpdate),
+ m_percentage(percentage) {
+ if (m_event_type == progressEnd)
+ m_minimum_allowed_report_time = std::chrono::seconds::zero();
+ else if (prev_event.Reported())
+ m_minimum_allowed_report_time = prev_event.m_minimum_allowed_report_time +
+ kUpdateProgressEventReportDelay;
else
- m_event_type = progressInvalid;
-
- if (0 < total && total < UINT64_MAX)
- m_percentage = (uint32_t)(((float)completed / (float)total) * 100.0);
+ m_minimum_allowed_report_time =
+ prev_event
+ .m_minimum_allowed_report_time; // we should use the previous
+ // timestamp, as it's still pending
}
-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;
}
+static Optional<ProgressEvent> CreateStartProgressEvent(uint64_t progress_id,
+ const char *message,
+ uint64_t completed,
+ uint64_t total) {
+ if (!message || completed > 0 || completed >= total)
+ return None;
+
+ Optional<uint32_t> percentage;
+ if (total < UINT64_MAX)
+ percentage = 0;
+ return ProgressEvent(progress_id, message, percentage);
+}
+
+static Optional<ProgressEvent>
+CreateUpdateOrEndProgressEvent(uint64_t progress_id, uint64_t completed,
+ uint64_t total,
+ const ProgressEvent &prev_event) {
+ if (completed == 0 || completed > total)
+ return None;
+
+ uint32_t percentage =
+ completed == total
+ ? 100
+ : std::min((uint32_t)((float)completed / (float)total * 100.0),
+ (uint32_t)99);
+ ProgressEvent event(progress_id, percentage, prev_event);
+
+ if (prev_event.EqualsForIDE(event))
+ return None;
+ return event;
+}
+
+ProgressEventType ProgressEvent::GetEventType() const { return m_event_type; }
+
const char *ProgressEvent::GetEventName() const {
if (m_event_type == progressStart)
return "progressStart";
else if (m_event_type == progressEnd)
return "progressEnd";
- else if (m_event_type == progressUpdate)
- return "progressUpdate";
else
- return "progressInvalid";
+ return "progressUpdate";
}
-bool ProgressEvent::IsValid() const { return m_event_type != progressInvalid; }
-
-uint64_t ProgressEvent::GetID() const { return m_progress_id; }
-
json::Value ProgressEvent::ToJSON() const {
llvm::json::Object event(CreateEventObject(GetEventName()));
llvm::json::Object body;
@@ -65,9 +109,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_creation_time.count()));
EmplaceSafeString(body, "timestamp", timestamp);
if (m_percentage)
@@ -77,17 +119,108 @@
return json::Value(std::move(event));
}
-ProgressEventFilterQueue::ProgressEventFilterQueue(
- std::function<void(ProgressEvent)> callback)
- : m_callback(callback) {}
+bool ProgressEvent::Report(ProgressEventReportCallback callback) {
+ if (Reported())
+ return true;
+ if (std::chrono::system_clock::now().time_since_epoch() >=
+ m_minimum_allowed_report_time) {
+ m_reported = true;
+ callback(*this);
+ return true;
+ }
+ return false;
+}
+
+bool ProgressEvent::Reported() const { return m_reported; }
+
+ProgressEventManager::ProgressEventManager(
+ const ProgressEvent &start_event,
+ ProgressEventReportCallback report_callback)
+ : m_start_event(start_event), m_finished(false),
+ m_report_callback(report_callback) {}
+
+bool ProgressEventManager::ReportIfNeeded() {
+ // The event finished before we were able to report it.
+ if (!m_start_event.Reported() && Finished())
+ return true;
+
+ if (!m_start_event.Report(m_report_callback))
+ return false;
+
+ if (m_last_update_event)
+ m_last_update_event->Report(m_report_callback);
+ return true;
+}
+
+const ProgressEvent &ProgressEventManager::GetMostRecentEvent() const {
+ return m_last_update_event ? *m_last_update_event : m_start_event;
+}
+
+void ProgressEventManager::Update(uint64_t progress_id, uint64_t completed,
+ uint64_t total) {
+ if (Optional<ProgressEvent> event = CreateUpdateOrEndProgressEvent(
+ progress_id, completed, total, GetMostRecentEvent())) {
+ if (event->GetEventType() == progressEnd)
+ m_finished = true;
-void ProgressEventFilterQueue::Push(const ProgressEvent &event) {
- if (!event.IsValid())
- return;
+ m_last_update_event = *event;
+ ReportIfNeeded();
+ }
+}
+
+bool ProgressEventManager::Finished() const { return m_finished; }
+
+ProgressEventReporter::ProgressEventReporter(
+ ProgressEventReportCallback report_callback)
+ : m_report_callback(report_callback) {
+ m_thread_should_exit = false;
+ m_thread = std::thread([&] {
+ while (!m_thread_should_exit) {
+ std::this_thread::sleep_for(kUpdateProgressEventReportDelay);
+ ReportStartEvents();
+ }
+ });
+}
+
+ProgressEventReporter::~ProgressEventReporter() {
+ m_thread_should_exit = true;
+ m_thread.join();
+}
+
+void ProgressEventReporter::ReportStartEvents() {
+ std::lock_guard<std::mutex> locker(m_mutex);
+
+ while (!m_unreported_start_events.empty()) {
+ ProgressEventManagerSP event_manager = m_unreported_start_events.front();
+ if (event_manager->Finished())
+ m_unreported_start_events.pop();
+ else if (event_manager->ReportIfNeeded())
+ m_unreported_start_events
+ .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.
+ }
+}
- 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);
+void ProgressEventReporter::Push(uint64_t progress_id, const char *message,
+ uint64_t completed, uint64_t total) {
+ std::lock_guard<std::mutex> locker(m_mutex);
+
+ auto it = m_event_managers.find(progress_id);
+ if (it == m_event_managers.end()) {
+ if (Optional<ProgressEvent> event =
+ CreateStartProgressEvent(progress_id, message, completed, total)) {
+ ProgressEventManagerSP event_manager =
+ std::make_shared<ProgressEventManager>(*event, m_report_callback);
+ m_event_managers.insert({progress_id, event_manager});
+ m_unreported_start_events.push(event_manager);
+ }
+ } else {
+ it->second->Update(progress_id, completed, total);
+ if (it->second->Finished())
+ m_event_managers.erase(it);
}
}
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
