[lldb-vscode] only report long running progress events

[wallace]

Summary

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.

[clayborg]

lldb/tools/lldb-vscode/ProgressEvent.cpp

128

If we are waiting a number of seconds for each progress, not sure we need to sleep for only 100ms. Shouldn't we sleep for a bit longer time?

132

The patch description says 5 seconds, here it is 3 seconds, and I would prefer maybe 1 second.

lldb/tools/lldb-vscode/ProgressEvent.h

123

"We don't report"?

[clayborg]

lldb/tools/lldb-vscode/ProgressEvent.cpp

125

This might be better named "m_aux_reporter_thread_should_exit". This variable sounds like the thread itself will modify the value when it finishes, but this is a "I want to tell the thread when it is no longer needed when I want it to exit.

135

I would just name this "TaskProgressEventQueue::Update(...)"

136–137

If this is an progressEnd event, we could track if this event did report any progressStart or progressUpdate events and just send the update right away here? That would require that the report_callback is stored during construction though. It would be easy if TaskProgressEventQueue just got a reference to the ProgressEventFilterQueue since it owns these TaskProgressEventQueue. Then we could extract the callback from it.

152

If I am reading this loop correctly, we are going to only notify the progress whose id is m_event_ids.front()? So if we have 10 progress start events we will only report the first one? Can VS Code only handle one progress at a time? If not we need to iterate over all m_event_ids and report any possible progress on all events that have new status as many different threads could be progressing on indexing DWARF.

156

Should we check m_last_update_event here and make sure it isn't a progressEnd event? It is is, we will notify both start and end right away? Edge case, but just wanted to check.

158

ProgressEventFilterQueue::TryReport() returns true if the event was reported even if it didn't finish. Do we always want to pop from m_event_ids even if the task didn't finish? Is this loop only to catch reporting the start event?

217–218

Can't "SetUpdate" just report things if needed? This could be just:

it->second.SetUpdate(event);

lldb/tools/lldb-vscode/ProgressEvent.h

44

Yikes this must have caused some serious bugs in the reporting mechanism as the old string would have been freed!!

121

TaskProgressEventQueue might be better named "ProgressEventManager"? It doesn't have a queue, but it does manage event deliveries

125–128

make private?

150

Should be just be storing this in the ProgressEvent? That way we can accurately track the timestamp for any packets we deliver, like when we deliver one from m_last_update_event

151

After reading the code, this might be more clear if it were named "m_reported_start_event".

152

Curious, when we report events, I remember adding a timestamp to the progress event packet. If we report events later after the timeout that filters quick progress packets, do we send the original timestamp from when the progress was reported? We should make sure we do because we want to be able to use this timing information from the packet log to deduce performance issues.

[wallace]

lldb/tools/lldb-vscode/ProgressEvent.cpp

125

+1

128

yes, you are right, at least 1 second should be fine.

132

yes, 1 second is reasonable. I'll test it with a huge binary as well

136–137

good one

152

not quite right, this will report as many as it can until it finds the first one who can't be reported, which means that that event is not old enough, and therefore the other events are also not old enough

156

good catch

158

this is only looping through the start events. I'll make this clearer

lldb/tools/lldb-vscode/ProgressEvent.h

44

well, not really as we were dispatching the event right away, but with async events we need to save the data to a string

150

makes sense

152

i'll put that back!

[wallace]

Address all comments

[clayborg]

lldb/tools/lldb-vscode/ProgressEvent.cpp

45

128

We have a 1 second timeout before events should be reported, so we should probably sleep a shorter amount of time than 2 seconds as if the events come in such that a progress start event is queued right after this loop sleeps for 2 seconds, it can now be three seconds before an event is reported. How about 250 ms? That way the most we will wait before reporting a start event is 1.25 seconds. Also since we expect the thread to exit in ~ProgressEventFilterQueue, we don't want to have to wait 2 seconds during shut down.

135

remove m_start_event_timestamp as it is no longer needed since ProgressEvent has a timestamp now.

139

rename to "ReportIfNeeded()"?

139

Should this be named "ReportStartEvents()"? Is that the only thing this is doing here?

142–151

147

If we are using m_sorted_event_managers only for reporting start events, we should rename to "m_unreported_start_events"

149–150

We have many places in the code that are reporting progress and then having to manually mark the event as reported. We should make a ProgressEvent::Report() function that looks something like:

void ProgressEvent::Report(ProgressEventReportCallback report_callback) {
  if (!m_reported) {
    m_reported = true;
    report_callback(*this);
  }
}

152

Ok so this class is a start event reporter then?

153–154

lldb/tools/lldb-vscode/ProgressEvent.h

151

m_thread should suffice here.

152

m_thread_should_exit should suffice.

155

m_mutex is fine unless we get more than one mutex in this class.

[wallace]

Address all comments

• now the reporting timing conditions are in ProgressEvent, which simplifies the other classes
• now two subsequent updates are only sent if there's a 0.25 delay between them
• removed the progressInvalid enum by making sure only valid ProgressEvent instances are created

[wallace]

nits

[clayborg]

lldb/tools/lldb-vscode/ProgressEvent.cpp

144–149

196

put in header file

201

init before { with "m_thread_should_exit(false)"

215–216

Why is this returning an optional event? We know we need a start event here and it must be made. The Optional seems like it isn't needed?

222–226

lldb/tools/lldb-vscode/ProgressEvent.h

30–39

Seems like all these constructors are a bit much. We should probably have one that just takes all the needed parameters including the ProgressEventType. It is unclear from each constructor what each one does unless you look very carefully at the header documentation. It will be very clear if we just have:

ProgressEvent(uint64_t progress_id, StringRef message, ProgressEventType event_type, llvm::Optional<uint32_t> percentage = llvm::None);

31

Why is there an optional percentage for a start event?

31

Why do we need this constructor? At the very least we don't need the progress_id since the prev_event will have one

35–36

Don't need progress_id if we have a prev_event. We probably don't need the previous constructor if we have this one. Might be better as:

ProgressEvent(const ProgressEvent &prev_event, uint32_t percentage);

If the percentage is 100, then this is an end event? Otherwise it is an update event?

[wallace]

lldb/tools/lldb-vscode/ProgressEvent.h

31

if total is INT_MAX, there's nothing to report

[wallace]

Simplified the constructors

[clayborg]

The only last nit is we are passing the report progress callback around all over the place. I think it would be nicer to just make a static function that we can call and remove the passing of the instance variable for the report callback all over to the event classes.

lldb/tools/lldb-vscode/ProgressEvent.cpp

27

So we are copying the string for every event? I thought we only needed this for the start event?

28–57

I would be nice to simplify this down a bit as we have both "completed" being used along with "prev_event" to try and figure out what the event is. We probably don't need "long double" for calculating the percentage either. It would be clearer to maybe do:

const bool calculate_percentage = total != UINT64_MAX;
if (completed == 0) {
  // Start event
  m_event_type = progressStart;
  // Wait a bit before reporting the start event in case in completes really quickly.
  m_minimum_allowed_report_time = m_creation_time + kStartProgressEventReportDelay;
  if (calculate_percentage)
    m_precentage = 0;
} else if (completed == total) {
  // End event
  m_event_type = progressEnd;
  // We should report the end event right away.
  m_minimum_allowed_report_time = std::chrono::seconds::zero();
  if (calculate_percentage)
    m_precentage = 100;
} else {
  // Update event
  assert(prev_event);
  m_percentage = std::min(
        (uint32_t)((double)completed / (double)total * 100.0),
        (uint32_t)99);
  if (prev_event->Reported()) {
    // Add a small delay between reports
    m_minimum_allowed_report_time =
        prev_event->m_minimum_allowed_report_time +
        kUpdateProgressEventReportDelay; 
  } else {
    // We should use the previous timestamp, as it's still pending
    m_minimum_allowed_report_time =
        prev_event->m_minimum_allowed_report_time; 
  }
}

206

Should we return the result of "m_last_update_event->Report(m_report_callback);" here?

216

Should we add a safeguard and check m_finished first?

[wallace]

Addressed the comments except for the "static function that we can call and remove the passing of the instance variable for the report callback all over to the event classes". That's a little bit hard to achieve because the callback requires an instance of VSCode, so it can't easily be fully static. I think it's fine to the callbacks as they are not used in too many places.

[wallace]

https://reviews.llvm.org/P8265 is a sample DAP output with these messages

[wallace]

ping

[stella.stamenova]

@wallace : Your most recent change to comment out ProgressEventThreadFunction is causing warnings to be generated when building with clang. Please have a look:

/usr/bin/clang++ -DGTEST_HAS_RTTI=0 -DHAVE_ROUND -D_DEBUG -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Itools/lldb/tools/lldb-vscode -I/mnt/vss/_work/1/s/lldb/tools/lldb-vscode -I/mnt/vss/_work/1/s/lldb/include -Itools/lldb/include -Iinclude -I/mnt/vss/_work/1/s/llvm/include -I/mnt/vss/_work/1/s/llvm/../clang/include -Itools/lldb/../clang/include -I/usr/include/libxml2 -fPIC -fvisibility-inlines-hidden -Werror -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wc++98-compat-extra-semi -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion -Wmisleading-indentation -fdiagnostics-color -ffunction-sections -fdata-sections -Wno-deprecated-declarations -Wno-unknown-pragmas -Wno-strict-aliasing -Wno-deprecated-register -Wno-vla-extension -O3 -DNDEBUG  -fno-exceptions -fno-rtti -UNDEBUG -std=c++14 -MD -MT tools/lldb/tools/lldb-vscode/CMakeFiles/lldb-vscode.dir/lldb-vscode.cpp.o -MF tools/lldb/tools/lldb-vscode/CMakeFiles/lldb-vscode.dir/lldb-vscode.cpp.o.d -o tools/lldb/tools/lldb-vscode/CMakeFiles/lldb-vscode.dir/lldb-vscode.cpp.o -c /mnt/vss/_work/1/s/lldb/tools/lldb-vscode/lldb-vscode.cpp
/mnt/vss/_work/1/s/lldb/tools/lldb-vscode/lldb-vscode.cpp:370:6: error: unused function 'ProgressEventThreadFunction' [-Werror,-Wunused-function]
void ProgressEventThreadFunction() {
     ^
1 error generated.

[wallace]

I'm reverting it asap

Il giorno lun 21 giu 2021 alle ore 10:48 Stella Stamenova via Phabricator <
[e-mail]> ha scritto:

stella.stamenova added a comment.

@wallace : Your most recent change to comment out
ProgressEventThreadFunction is causing warnings to be generated when
building with clang. Please have a look:

/usr/bin/clang++ -DGTEST_HAS_RTTI=0 -DHAVE_ROUND -D_DEBUG -D_GNU_SOURCE

-DSTDC_CONSTANT_MACROS -DSTDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS
-Itools/lldb/tools/lldb-vscode -I/mnt/vss/_work/1/s/lldb/tools/lldb-vscode
-I/mnt/vss/_work/1/s/lldb/include -Itools/lldb/include -Iinclude
-I/mnt/vss/_work/1/s/llvm/include
-I/mnt/vss/_work/1/s/llvm/../clang/include -Itools/lldb/../clang/include
-I/usr/include/libxml2 -fPIC -fvisibility-inlines-hidden -Werror
-Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra
-Wno-unused-parameter -Wwrite-strings -Wcast-qual
-Wmissing-field-initializers -pedantic -Wno-long-long
-Wc++98-compat-extra-semi -Wimplicit-fallthrough -Wcovered-switch-default
-Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor
-Wsuggest-override -Wstring-conversion -Wmisleading-indentation
-fdiagnostics-color -ffunction-sections -fdata-sections
-Wno-deprecated-declarations -Wno-unknown-pragmas -Wno-strict-aliasing
-Wno-deprecated-register -Wno-vla-extension -O3 -DNDEBUG -fno-exceptions
-fno-rtti -UNDEBUG -std=c++14 -MD -MT
tools/lldb/tools/lldb-vscode/CMakeFiles/lldb-vscode.dir/lldb-vscode.cpp.o
-MF
tools/lldb/tools/lldb-vscode/CMakeFiles/lldb-vscode.dir/lldb-vscode.cpp.o.d
-o
tools/lldb/tools/lldb-vscode/CMakeFiles/lldb-vscode.dir/lldb-vscode.cpp.o
-c /mnt/vss/_work/1/s/lldb/tools/lldb-vscode/lldb-vscode.cpp

/mnt/vss/_work/1/s/lldb/tools/lldb-vscode/lldb-vscode.cpp:370:6: error:

unused function 'ProgressEventThreadFunction' [-Werror,-Wunused-function]

void ProgressEventThreadFunction() {
     ^
1 error generated.

Repository:

rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION

https://reviews.llvm.org/D101128/new/

https://reviews.llvm.org/D101128

[stella.stamenova]

In D101128#2831098, @wallace wrote:

I'm reverting it asap

Thanks!

lldb/tools/lldb-vscode/ProgressEvent.h

 //===-- ProgressEvent.cpp ---------------------------------------*- 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
 //
 //===----------------------------------------------------------------------===//
 
+#include <mutex>
+#include <queue>
+#include <thread>
+
 #include "VSCodeForward.h"
 
 #include "llvm/Support/JSON.h"
 
 namespace lldb_vscode {
 
 enum ProgressEventType {
-  progressInvalid,
   progressStart,
   progressUpdate,
   progressEnd
 };
 
+class ProgressEvent;
+using ProgressEventReportCallback = std::function<void(ProgressEvent &)>;
+
 class ProgressEvent {
 public:
-  ProgressEvent() {}
-
-  ProgressEvent(uint64_t progress_id, const char *message, uint64_t completed,
-                uint64_t total);
+  /// Actual constructor to use that returns an optional, as the event might be
+  /// not apt for the IDE, e.g. an unnamed start event, or a redundant one.
+  ///
+  /// \param[in] progress_id
+  ///   ID for this event.
+  ///
+  /// \param[in] message
+  ///   Message to display in the UI. Required for start events.
+  ///
+  /// \param[in] completed
+  ///   Number of jobs completed.
+  ///
+  /// \param[in] total
+  ///   Total number of jobs, or \b UINT64_MAX if not determined.
+  ///
+  /// \param[in] prev_event
+  ///   Previous event if this one is an update. If \b nullptr, then a start
+  ///   event will be created.
+  static llvm::Optional<ProgressEvent>
+  Create(uint64_t progress_id, llvm::Optional<llvm::StringRef> message,
+         uint64_t completed, uint64_t total,
+         const ProgressEvent *prev_event = nullptr);
 
   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;
+  llvm::StringRef GetEventName() const;
 
-  bool IsValid() const;
+  ProgressEventType GetEventType() const;
 
-  uint64_t GetID() 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);
+
+  bool Reported() const;
 
 private:
+  ProgressEvent(uint64_t progress_id, llvm::Optional<llvm::StringRef> message,
+                uint64_t completed, uint64_t total,
+                const ProgressEvent *prev_event);
+
   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 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 {
+/// 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:
-  ProgressEventFilterQueue(std::function<void(ProgressEvent)> callback);
+  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;
 
-  void Push(const ProgressEvent &event);
+  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, 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:
+  /// \param[in] report_callback
+  ///     Function to invoke to report the event to the IDE.
+  ProgressEventReporter(ProgressEventReportCallback report_callback);
+
+  ~ProgressEventReporter();
+
+  /// 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

lldb/tools/lldb-vscode/ProgressEvent.cpp

 //===-- ProgressEvent.cpp ---------------------------------------*- 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
 //
 //===----------------------------------------------------------------------===//
 
 #include "ProgressEvent.h"
 
 #include "JSONUtils.h"
 
 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)
+// 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 =
+    std::chrono::milliseconds(250);
+
+ProgressEvent::ProgressEvent(uint64_t progress_id, Optional<StringRef> message,
+                             uint64_t completed, uint64_t total,
+                             const ProgressEvent *prev_event)
+    : m_progress_id(progress_id) {
+  if (message)
+    m_message = message->str();
+
+  const bool calculate_percentage = total != UINT64_MAX;
+  if (completed == 0) {
+    // Start event
     m_event_type = progressStart;
-  else if (completed < total)
-    m_event_type = progressUpdate;
-  else
-    m_event_type = progressInvalid;
+    // Wait a bit before reporting the start event in case in completes really
+    // quickly.
+    m_minimum_allowed_report_time =
+        m_creation_time + kStartProgressEventReportDelay;
+    if (calculate_percentage)
+      m_percentage = 0;
+  } else if (completed == total) {
+    // End event
+    m_event_type = progressEnd;
+    // We should report the end event right away.
+    m_minimum_allowed_report_time = std::chrono::seconds::zero();
+    if (calculate_percentage)
+      m_percentage = 100;
+  } else {
+    // Update event
+    assert(prev_event);
+    m_percentage = std::min(
+        (uint32_t)((double)completed / (double)total * 100.0), (uint32_t)99);
+    if (prev_event->Reported()) {
+      // Add a small delay between reports
+      m_minimum_allowed_report_time =
+          prev_event->m_minimum_allowed_report_time +
+          kUpdateProgressEventReportDelay;
+    } else {
+      // We should use the previous timestamp, as it's still pending
+      m_minimum_allowed_report_time = prev_event->m_minimum_allowed_report_time;
+    }
+  }
+}
+
+Optional<ProgressEvent> ProgressEvent::Create(uint64_t progress_id,
+                                              Optional<StringRef> message,
+                                              uint64_t completed,
+                                              uint64_t total,
+                                              const ProgressEvent *prev_event) {
+  ProgressEvent event(progress_id, message, completed, total, prev_event);
+  // We shouldn't show unnamed start events in the IDE
+  if (event.GetEventType() == progressStart && event.GetEventName().empty())
+    return None;
+
+  if (prev_event && prev_event->EqualsForIDE(event))
+    return None;
 
-  if (0 < total && total < UINT64_MAX)
-    m_percentage = (uint32_t)(((float)completed / (float)total) * 100.0);
+  return event;
 }
 
-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;
 }
 
-const char *ProgressEvent::GetEventName() const {
+ProgressEventType ProgressEvent::GetEventType() const { return m_event_type; }
+
+StringRef 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;
 
   std::string progress_id_str;
   llvm::raw_string_ostream progress_id_strm(progress_id_str);
   progress_id_strm << m_progress_id;
   progress_id_strm.flush();
   body.try_emplace("progressId", progress_id_str);
 
   if (m_event_type == progressStart) {
     EmplaceSafeString(body, "title", m_message);
     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)
     body.try_emplace("percentage", *m_percentage);
 
   event.try_emplace("body", std::move(body));
   return json::Value(std::move(event));
 }
 
-ProgressEventFilterQueue::ProgressEventFilterQueue(
-    std::function<void(ProgressEvent)> callback)
-    : m_callback(callback) {}
-
-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);
+bool ProgressEvent::Report(ProgressEventReportCallback callback) {
+  if (Reported())
+    return true;
+  if (std::chrono::system_clock::now().time_since_epoch() <
+      m_minimum_allowed_report_time)
+    return false;
+
+  m_reported = true;
+  callback(*this);
+  return true;
+}
+
+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 = ProgressEvent::Create(
+          progress_id, None, completed, total, &GetMostRecentEvent())) {
+    if (event->GetEventType() == progressEnd)
+      m_finished = true;
+
+    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.
+  }
+}
+
+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 =
+            ProgressEvent::Create(progress_id, StringRef(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/tools/lldb-vscode/VSCode.h

struct VSCode {
   std::vector<std::string> terminate_commands;
   lldb::tid_t focus_tid;
   bool sent_terminated_event;
   bool stop_at_entry;
   bool is_attach;
   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;
   VSCode();
   ~VSCode();
   VSCode(const VSCode &rhs) = delete;
   void operator=(const VSCode &rhs) = delete;
   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.
   void SendJSON(const llvm::json::Value &json);
 
   std::string ReadJSON();
 
   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, ...);
 
   static int64_t GetNextSourceReference();
 
   ExceptionBreakpoint *GetExceptionBPFromStopReason(lldb::SBThread &thread);
 
struct VSCode {
   /// \param[in] request
   ///     The name of the request following the Debug Adapter Protocol
   ///     specification.
   ///
   /// \param[in] callback
   ///     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;
 
 } // namespace lldb_vscode
 
 #endif

lldb/tools/lldb-vscode/VSCode.cpp

: variables(), broadcaster("lldb-vscode"), num_regs(0), num_locals(0),
            {"cpp_throw", "C++ Throw", lldb::eLanguageTypeC_plus_plus},
            {"objc_catch", "Objective C Catch", lldb::eLanguageTypeObjC},
            {"objc_throw", "Objective C Throw", lldb::eLanguageTypeObjC},
            {"swift_catch", "Swift Catch", lldb::eLanguageTypeSwift},
            {"swift_throw", "Swift Throw", lldb::eLanguageTypeSwift}}),
       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)
   // Windows opens stdout and stdin in text mode which converts \n to 13,10
   // while the value is just 10 on Darwin/Linux. Setting the file mode to binary
   // fixes this.
   int result = _setmode(fileno(stdout), _O_BINARY);
   assert(result);

 //     /**
 //      * Optional, more detailed progress message. If omitted, the previous
 //      * message (if any) is used.
 //      */
 //     message?: string;
 //   };
 // }
 
-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)))
 VSCode::SendFormattedOutput(OutputType o, const char *format, ...) {
   char buffer[1024];
   va_list args;
   va_start(args, format);
   int actual_length = vsnprintf(buffer, sizeof(buffer), format, args);

lldb/tools/lldb-vscode/lldb-vscode.cpp

if (listener.WaitForEvent(1, event)) {
       } else {
         uint64_t progress_id = 0;
         uint64_t completed = 0;
         uint64_t total = 0;
         bool is_debugger_specific = false;
         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);
       }
     }
   }
 }
 
 // All events from the debugger, target, process, thread and frames are
 // received in this function that runs in its own thread. We are using a
 // "FILE *" to output packets back to VS Code and they have mutexes in them