[Lldb-commits] [lldb] [lldb] Don't report all progress event as completed. (PR #84281)

[Jonas Devlieghere via lldb-commits]

https://github.com/JDevlieghere created https://github.com/llvm/llvm-project/pull/84281

Currently, progress events reported by the ProgressManager and broadcast to eBroadcastBitProgressCategory always specify they're complete. The problem is that the ProgressManager reports kNonDeterministicTotal for both the total and the completed number of (sub)events. Because the values are the same, the event reports itself as complete.

This patch fixes the issue by reporting 0 as the completed value for the start event and kNonDeterministicTotal for the end event.

>From 9211c334ab91ba424aa06460b3be9759e85db15c Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere <jonas at devlieghere.com>
Date: Wed, 6 Mar 2024 22:28:14 -0800
Subject: [PATCH] [lldb] Don't report all progress event as completed.

Currently, progress events reported by the ProgressManager and broadcast
to eBroadcastBitProgressCategory always specify they're complete. The
problem is that the ProgressManager reports kNonDeterministicTotal for
both the total and the completed number of (sub)events. Because the
values are the same, the event reports itself as complete.

This patch fixes the issue by reporting 0 as the completed value for the
start event and kNonDeterministicTotal for the end event.
---
 lldb/include/lldb/Core/Progress.h          |  9 +++++++--
 lldb/source/Core/Progress.cpp              | 18 ++++++++++--------
 lldb/unittests/Core/ProgressReportTest.cpp |  4 ++--
 3 files changed, 19 insertions(+), 12 deletions(-)

diff --git a/lldb/include/lldb/Core/Progress.h b/lldb/include/lldb/Core/Progress.h
index c6fc861fb71d86..c38f6dd0a140ed 100644
--- a/lldb/include/lldb/Core/Progress.h
+++ b/lldb/include/lldb/Core/Progress.h
@@ -148,9 +148,14 @@ class ProgressManager {
 
   static ProgressManager &Instance();
 
-  static void ReportProgress(const Progress::ProgressData &);
-
 private:
+  enum class EventType {
+    Begin,
+    End,
+  };
+  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;
diff --git a/lldb/source/Core/Progress.cpp b/lldb/source/Core/Progress.cpp
index 9dcd7cf75ae057..6f2f62c21a0c7e 100644
--- a/lldb/source/Core/Progress.cpp
+++ b/lldb/source/Core/Progress.cpp
@@ -97,7 +97,7 @@ void ProgressManager::Increment(const Progress::ProgressData &progress_data) {
   // initial progress report.
   if (!m_progress_category_map.contains(progress_data.title)) {
     m_progress_category_map[progress_data.title].second = progress_data;
-    ReportProgress(progress_data);
+    ReportProgress(progress_data, EventType::Begin);
   }
   m_progress_category_map[progress_data.title].first++;
 }
@@ -110,7 +110,7 @@ void ProgressManager::Decrement(const Progress::ProgressData &progress_data) {
     return;
 
   if (pos->second.first <= 1) {
-    ReportProgress(pos->second.second);
+    ReportProgress(pos->second.second, EventType::End);
     m_progress_category_map.erase(progress_data.title);
   } else {
     --pos->second.first;
@@ -118,12 +118,14 @@ void ProgressManager::Decrement(const Progress::ProgressData &progress_data) {
 }
 
 void ProgressManager::ReportProgress(
-    const Progress::ProgressData &progress_data) {
-  // The category bit only keeps track of when progress report categories have
+    const Progress::ProgressData &progress_data, EventType type) {
+  // the category bit only keeps track of when progress report categories have
   // started and ended, so clear the details and reset other fields when
   // broadcasting to it since that bit doesn't need that information.
-  Debugger::ReportProgress(
-      progress_data.progress_id, progress_data.title, "",
-      Progress::kNonDeterministicTotal, Progress::kNonDeterministicTotal,
-      progress_data.debugger_id, Debugger::eBroadcastBitProgressCategory);
+  const uint64_t completed =
+      (type == EventType::Begin) ? 0 : Progress::kNonDeterministicTotal;
+  Debugger::ReportProgress(progress_data.progress_id, progress_data.title, "",
+                           completed, Progress::kNonDeterministicTotal,
+                           progress_data.debugger_id,
+                           Debugger::eBroadcastBitProgressCategory);
 }
diff --git a/lldb/unittests/Core/ProgressReportTest.cpp b/lldb/unittests/Core/ProgressReportTest.cpp
index 98cbc475ce2835..e0253cbc4ec59b 100644
--- a/lldb/unittests/Core/ProgressReportTest.cpp
+++ b/lldb/unittests/Core/ProgressReportTest.cpp
@@ -168,7 +168,7 @@ TEST_F(ProgressReportTest, TestProgressManager) {
 
   ASSERT_EQ(data->GetDetails(), "");
   ASSERT_FALSE(data->IsFinite());
-  ASSERT_TRUE(data->GetCompleted());
+  ASSERT_FALSE(data->GetCompleted());
   ASSERT_EQ(data->GetTotal(), Progress::kNonDeterministicTotal);
   ASSERT_EQ(data->GetMessage(), "Progress report 1");
 
@@ -199,7 +199,7 @@ TEST_F(ProgressReportTest, TestProgressManager) {
 
   ASSERT_EQ(data->GetDetails(), "");
   ASSERT_FALSE(data->IsFinite());
-  ASSERT_TRUE(data->GetCompleted());
+  ASSERT_FALSE(data->GetCompleted());
   ASSERT_EQ(data->GetTotal(), Progress::kNonDeterministicTotal);
   ASSERT_EQ(data->GetMessage(), "Overlapping report 1");