wallace created this revision.
wallace requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

When the number of shared libs is massive, there could be hundreds of
thousands of short lived progress events sent to the IDE, which makes it
irresponsive while it's processing all this data. As these small jobs
take less than a second to process, the user doesn't even see them,
because the IDE only display the progress of long operations. So it's
better not to send these events.

I'm fixing that by sending only the events that are taking longer than 5
seconds to process.
In a specific run, I got the number of events from ~500k to 100, because
there was only 1 big lib to parse.

I've tried this on several small and massive targets, and it seems to
work fine.


Repository:
  rG LLVM Github Monorepo

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,99 @@
 
   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;
 
 private:
   uint64_t m_progress_id;
-  const char *m_message;
+  std::string m_message;
   ProgressEventType m_event_type;
   llvm::Optional<uint32_t> m_percentage;
 };
 
+/// Class that keeps the start event and its most recent update.
+/// It controls when the event should start being reported to the IDE.
+class TaskProgressEventQueue {
+public:
+  TaskProgressEventQueue(const ProgressEvent &start_event);
+
+  /// \return
+  ///     \b true if enough seconds have passed since the start event,
+  ///     which means the events should start being reported.
+  bool ShouldReport() const;
+
+  /// Report the start event, if not reported yet, and the most recent
+  /// update if \a ShouldReport() is \b true.
+  bool TryReport(std::function<void(ProgressEvent)> report_callback);
+
+  /// \return
+  ///     \b true if the new event is effectively different to previous
+  ///     updates.
+  bool SetUpdate(const ProgressEvent &event);
+
+  /// \return
+  ///     \b true if a \a progressEnd event has been notified.
+  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_waiting;
+  bool m_finished;
+};
+
 /// 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.
+///
+/// We don't short lived events to be sent to the IDE, as they are rendered
+/// in the UI and end up spamming the DAP connection. The only events
+/// worth of being reported are the ones that take at least a few seconds to run
+/// and that carry new information.
 class ProgressEventFilterQueue {
 public:
-  ProgressEventFilterQueue(std::function<void(ProgressEvent)> callback);
+  ProgressEventFilterQueue(std::function<void(ProgressEvent)> report_callback);
+
+  ~ProgressEventFilterQueue();
 
+  /// Add a new event to the internal queue.
   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 are still in progress and haven't received
+  /// any updates since then.
+  void ReportWaitingEvents();
+
+  /// Report to the IDE the progress events of the provided task.
+  ///
+  /// \return
+  ///     \b true if some events were in fact reported to the IDE.
+  bool TryReport(TaskProgressEventQueue &task);
+
+  std::function<void(ProgressEvent)> m_report_callback;
+  std::map<uint64_t, TaskProgressEventQueue> m_tasks;
+  /// Queue of event ids in chronological order
+  std::queue<uint64_t> m_event_ids;
+  /// Thread used to invoke \a ReportWaitingEvents periodically.
+  std::thread m_aux_reporter_thread;
+  bool m_aux_reporter_thread_finished;
+  /// Mutex that prevents running simulatenously \a Push and \a
+  /// ReportWaitingEvents, 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
@@ -15,7 +15,7 @@
 
 ProgressEvent::ProgressEvent(uint64_t progress_id, const char *message,
                              uint64_t completed, uint64_t total)
-    : m_progress_id(progress_id), m_message(message) {
+    : m_progress_id(progress_id) {
   if (completed == total)
     m_event_type = progressEnd;
   else if (completed == 0)
@@ -25,16 +25,21 @@
   else
     m_event_type = progressInvalid;
 
+  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,11 +70,6 @@
     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()));
-  EmplaceSafeString(body, "timestamp", timestamp);
-
   if (m_percentage)
     body.try_emplace("percentage", *m_percentage);
 
@@ -77,17 +77,103 @@
   return json::Value(std::move(event));
 }
 
+TaskProgressEventQueue::TaskProgressEventQueue(const ProgressEvent &start_event)
+    : m_start_event(start_event),
+      m_start_event_timestamp(
+          std::chrono::system_clock::now().time_since_epoch()),
+      m_waiting(true), m_finished(false) {}
+
+bool TaskProgressEventQueue::ShouldReport() const {
+  return !m_waiting || std::chrono::system_clock::now().time_since_epoch() -
+                               m_start_event_timestamp >=
+                           std::chrono::seconds(3);
+}
+
+bool TaskProgressEventQueue::SetUpdate(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;
+    return true;
+  }
+  return false;
+}
+
+uint64_t TaskProgressEventQueue::GetID() const { return m_start_event.GetID(); }
+
+bool TaskProgressEventQueue::Finished() const { return m_finished; }
+
+bool TaskProgressEventQueue::TryReport(
+    std::function<void(ProgressEvent)> report_callback) {
+  if (!ShouldReport())
+    return false;
+
+  if (m_waiting) {
+    report_callback(m_start_event);
+    m_waiting = false;
+  }
+  if (m_last_update_event)
+    report_callback(*m_last_update_event);
+  return true;
+}
+
 ProgressEventFilterQueue::ProgressEventFilterQueue(
-    std::function<void(ProgressEvent)> callback)
-    : m_callback(callback) {}
+    std::function<void(ProgressEvent)> report_callback)
+    : m_report_callback(report_callback) {
+  m_aux_reporter_thread_finished = false;
+  m_aux_reporter_thread = std::thread([&] {
+    while (!m_aux_reporter_thread_finished) {
+      std::this_thread::sleep_for(std::chrono::milliseconds(100));
+      ReportWaitingEvents();
+    }
+  });
+}
+
+ProgressEventFilterQueue::~ProgressEventFilterQueue() {
+  m_aux_reporter_thread_finished = true;
+  m_aux_reporter_thread.join();
+}
+
+bool ProgressEventFilterQueue::TryReport(TaskProgressEventQueue &task) {
+  bool did_report = task.TryReport(m_report_callback);
+
+  // Regardless of whether we reported the events or not, if they have finished,
+  // we just remove the event.
+  if (task.Finished())
+    m_tasks.erase(task.GetID());
+  return did_report;
+}
+
+void ProgressEventFilterQueue::ReportWaitingEvents() {
+  std::lock_guard<std::mutex> locker(m_reporter_mutex);
+
+  while (!m_event_ids.empty()) {
+    uint64_t progress_id = m_event_ids.front();
+    auto it = m_tasks.find(progress_id);
+    if (it == m_tasks.end())
+      m_event_ids.pop(); // The event was finalized already.
+    else if (TryReport(it->second))
+      m_event_ids.pop();
+    else
+      break; // If we couldn't report it, then the next event in the queue won't
+             // be able as well.
+  }
+}
 
 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_tasks.find(event.GetID());
+  if (it == m_tasks.end()) {
+    m_tasks.insert({event.GetID(), TaskProgressEventQueue(event)});
+    m_event_ids.push(event.GetID());
+  } else {
+    TaskProgressEventQueue &task = it->second;
+    if (task.SetUpdate(event))
+      TryReport(task);
   }
 }
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to