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

[via lldb-commits]

Author: Jonas Devlieghere
Date: 2024-03-07T09:55:00-08:00
New Revision: ea49e04b35bc8e4bed7ee4db4074d201f780a15c

URL: https://github.com/llvm/llvm-project/commit/ea49e04b35bc8e4bed7ee4db4074d201f780a15c
DIFF: https://github.com/llvm/llvm-project/commit/ea49e04b35bc8e4bed7ee4db4074d201f780a15c.diff

LOG: [lldb] Don't report all progress event as completed. (#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.

Added: 
    

Modified: 
    lldb/include/lldb/Core/Progress.h
    lldb/source/Core/Progress.cpp
    lldb/unittests/Core/ProgressReportTest.cpp

Removed: 
    


################################################################################
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..b4b5e98b7ba493 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) {
+    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");