[Lldb-commits] [lldb] [lldb][progress] Add progress manager class (PR #81319)
Chelsea Cassanova via lldb-commits
lldb-commits at lists.llvm.org
Thu Feb 15 12:48:02 PST 2024
https://github.com/chelcassanova updated https://github.com/llvm/llvm-project/pull/81319
>From f5ef07849c61ee9387f92376d5e1bd13bedc43e5 Mon Sep 17 00:00:00 2001
From: Chelsea Cassanova <chelsea_cassanova at apple.com>
Date: Thu, 8 Feb 2024 15:31:33 -0800
Subject: [PATCH 1/4] [lldb][progress] Add progress manager class
Per discussions from https://github.com/llvm/llvm-project/pull/81026,
it was decided that having a class that manages a map of progress
reports would be beneficial in order to categorize them. This class is a
part of the overall `Progress` class and utilizes a map that keeps a
count of how many times a progress report category has been sent. This
would be used with the new debugger broadcast bit added in
https://github.com/llvm/llvm-project/pull/81169 so that a user listening
with that bit will receive grouped progress reports.
---
lldb/include/lldb/Core/Progress.h | 27 +++++++++++++++++++
lldb/source/Core/Progress.cpp | 44 +++++++++++++++++++++++++++++++
2 files changed, 71 insertions(+)
diff --git a/lldb/include/lldb/Core/Progress.h b/lldb/include/lldb/Core/Progress.h
index 5d882910246054..5e07789b0e5337 100644
--- a/lldb/include/lldb/Core/Progress.h
+++ b/lldb/include/lldb/Core/Progress.h
@@ -11,6 +11,7 @@
#include "lldb/Utility/ConstString.h"
#include "lldb/lldb-types.h"
+#include "llvm/ADT/StringMap.h"
#include <atomic>
#include <mutex>
#include <optional>
@@ -119,6 +120,32 @@ class Progress {
bool m_complete = false;
};
+/// A class used to group progress reports by category. This is done by using a
+/// map that maintains a refcount of each category of progress reports that have
+/// come in. Keeping track of progress reports this way will be done if a
+/// debugger is listening to the eBroadcastBitProgressByCategory broadcast bit.
+class ProgressManager {
+public:
+ ProgressManager();
+ ~ProgressManager();
+
+ static void Initialize();
+ static void Terminate();
+ // Control the refcount of the progress report category as needed
+ void Increment(std::string category);
+ void Decrement(std::string category);
+
+ // Public accessor for the class instance
+ static ProgressManager &Instance();
+
+private:
+ // Manage the class instance internally using a std::optional
+ static std::optional<ProgressManager> &InstanceImpl();
+
+ llvm::StringMap<uint64_t> m_progress_category_map;
+ std::mutex m_progress_map_mutex;
+};
+
} // namespace lldb_private
#endif // LLDB_CORE_PROGRESS_H
diff --git a/lldb/source/Core/Progress.cpp b/lldb/source/Core/Progress.cpp
index 732efbc342b450..063e64a0ef499c 100644
--- a/lldb/source/Core/Progress.cpp
+++ b/lldb/source/Core/Progress.cpp
@@ -66,3 +66,47 @@ void Progress::ReportProgress() {
m_debugger_id);
}
}
+
+void ProgressManager::Initialize() {
+ lldbassert(!InstanceImpl() && "A progress report manager already exists.");
+ InstanceImpl().emplace();
+}
+
+void ProgressManager::Terminate() {
+ lldbassert(InstanceImpl() &&
+ "A progress report manager has already been terminated.");
+ InstanceImpl().reset();
+}
+
+std::optional<ProgressManager> &ProgressManager::InstanceImpl() {
+ static std::optional<ProgressManager> g_progress_manager;
+ return g_progress_manager;
+}
+
+ProgressManager::ProgressManager() : m_progress_category_map() {}
+
+ProgressManager::~ProgressManager() {}
+
+ProgressManager &ProgressManager::Instance() { return *InstanceImpl(); }
+
+void ProgressManager::Increment(std::string title) {
+ std::lock_guard<std::mutex> lock(m_progress_map_mutex);
+ auto pair = m_progress_category_map.insert(std::pair(title, 1));
+
+ // If pair.first is not empty after insertion it means that that
+ // category was entered for the first time and should not be incremented
+ if (!pair.second)
+ ++pair.first->second;
+}
+
+void ProgressManager::Decrement(std::string title) {
+ std::lock_guard<std::mutex> lock(m_progress_map_mutex);
+ auto pos = m_progress_category_map.find(title);
+
+ if (pos == m_progress_category_map.end())
+ return;
+
+ // Remove the category from the map if the refcount reaches 0
+ if (--pos->second == 0)
+ m_progress_category_map.erase(title);
+}
>From 5a045966224a809b8a5726411e406f3526c0332c Mon Sep 17 00:00:00 2001
From: Chelsea Cassanova <chelsea_cassanova at apple.com>
Date: Tue, 13 Feb 2024 15:19:41 -0800
Subject: [PATCH 2/4] Use call_once in InstanceImpl(), leak progress manager
object
In order to prevent potential crashes when LLDB is terminated,
this commit instead creates the instance for the progress manager in a
`call_once` within its initializer.
---
lldb/include/lldb/Core/Progress.h | 9 +++------
lldb/source/Core/Progress.cpp | 31 +++++++++++++------------------
2 files changed, 16 insertions(+), 24 deletions(-)
diff --git a/lldb/include/lldb/Core/Progress.h b/lldb/include/lldb/Core/Progress.h
index 5e07789b0e5337..1f2d48c27d63d3 100644
--- a/lldb/include/lldb/Core/Progress.h
+++ b/lldb/include/lldb/Core/Progress.h
@@ -129,18 +129,15 @@ class ProgressManager {
ProgressManager();
~ProgressManager();
- static void Initialize();
- static void Terminate();
- // Control the refcount of the progress report category as needed
+ /// Control the refcount of the progress report category as needed.
void Increment(std::string category);
void Decrement(std::string category);
- // Public accessor for the class instance
static ProgressManager &Instance();
private:
- // Manage the class instance internally using a std::optional
- static std::optional<ProgressManager> &InstanceImpl();
+ /// Manage the class instance internally.
+ static ProgressManager &InstanceImpl();
llvm::StringMap<uint64_t> 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 063e64a0ef499c..f5c745b2d70e2c 100644
--- a/lldb/source/Core/Progress.cpp
+++ b/lldb/source/Core/Progress.cpp
@@ -11,6 +11,7 @@
#include "lldb/Core/Debugger.h"
#include "lldb/Utility/StreamString.h"
+#include <mutex>
#include <optional>
using namespace lldb;
@@ -67,34 +68,28 @@ void Progress::ReportProgress() {
}
}
-void ProgressManager::Initialize() {
- lldbassert(!InstanceImpl() && "A progress report manager already exists.");
- InstanceImpl().emplace();
-}
-
-void ProgressManager::Terminate() {
- lldbassert(InstanceImpl() &&
- "A progress report manager has already been terminated.");
- InstanceImpl().reset();
-}
-
-std::optional<ProgressManager> &ProgressManager::InstanceImpl() {
- static std::optional<ProgressManager> g_progress_manager;
- return g_progress_manager;
+ProgressManager &ProgressManager::InstanceImpl() {
+ static std::once_flag g_once_flag;
+ static ProgressManager *g_progress_manager = nullptr;
+ std::call_once(g_once_flag, []() {
+ // NOTE: known leak to avoid global destructor chain issues.
+ g_progress_manager = new ProgressManager();
+ });
+ return *g_progress_manager;
}
ProgressManager::ProgressManager() : m_progress_category_map() {}
ProgressManager::~ProgressManager() {}
-ProgressManager &ProgressManager::Instance() { return *InstanceImpl(); }
+ProgressManager &ProgressManager::Instance() { return InstanceImpl(); }
void ProgressManager::Increment(std::string title) {
std::lock_guard<std::mutex> lock(m_progress_map_mutex);
auto pair = m_progress_category_map.insert(std::pair(title, 1));
// If pair.first is not empty after insertion it means that that
- // category was entered for the first time and should not be incremented
+ // category was entered for the first time and should not be incremented.
if (!pair.second)
++pair.first->second;
}
@@ -103,10 +98,10 @@ void ProgressManager::Decrement(std::string title) {
std::lock_guard<std::mutex> lock(m_progress_map_mutex);
auto pos = m_progress_category_map.find(title);
- if (pos == m_progress_category_map.end())
+ if (pos == m_progress_category_map.end() || pos->second == 0)
return;
- // Remove the category from the map if the refcount reaches 0
+ // Remove the category from the map if the refcount reaches 0.
if (--pos->second == 0)
m_progress_category_map.erase(title);
}
>From d341ce9496ec8bad657ff34b9ac401143ab40a51 Mon Sep 17 00:00:00 2001
From: Chelsea Cassanova <chelsea_cassanova at apple.com>
Date: Thu, 15 Feb 2024 11:10:35 -0800
Subject: [PATCH 3/4] Simpify Increment() and Decrement()
Increment() now uses the operator[] which removes the need that the
insertion in the map was of a new category. Decrement() removes the
category if its refcount is <= 1, otherwise it will then decrement it.
---
lldb/source/Core/Progress.cpp | 14 +++++---------
1 file changed, 5 insertions(+), 9 deletions(-)
diff --git a/lldb/source/Core/Progress.cpp b/lldb/source/Core/Progress.cpp
index f5c745b2d70e2c..f7ea209e01727b 100644
--- a/lldb/source/Core/Progress.cpp
+++ b/lldb/source/Core/Progress.cpp
@@ -86,22 +86,18 @@ ProgressManager &ProgressManager::Instance() { return InstanceImpl(); }
void ProgressManager::Increment(std::string title) {
std::lock_guard<std::mutex> lock(m_progress_map_mutex);
- auto pair = m_progress_category_map.insert(std::pair(title, 1));
-
- // If pair.first is not empty after insertion it means that that
- // category was entered for the first time and should not be incremented.
- if (!pair.second)
- ++pair.first->second;
+ m_progress_category_map[title]++;
}
void ProgressManager::Decrement(std::string title) {
std::lock_guard<std::mutex> lock(m_progress_map_mutex);
auto pos = m_progress_category_map.find(title);
- if (pos == m_progress_category_map.end() || pos->second == 0)
+ if (pos == m_progress_category_map.end())
return;
- // Remove the category from the map if the refcount reaches 0.
- if (--pos->second == 0)
+ if (pos->second <= 1)
m_progress_category_map.erase(title);
+ else
+ --pos->second;
}
>From da98a7680bb3ae92716270f3e97550733a17c1d4 Mon Sep 17 00:00:00 2001
From: Chelsea Cassanova <chelsea_cassanova at apple.com>
Date: Thu, 15 Feb 2024 12:44:15 -0800
Subject: [PATCH 4/4] Remove InstanceImpl()
Removes `InstanceImpl()` and inlines its code into `Instance()`.
---
lldb/include/lldb/Core/Progress.h | 3 ---
lldb/source/Core/Progress.cpp | 12 +++++-------
2 files changed, 5 insertions(+), 10 deletions(-)
diff --git a/lldb/include/lldb/Core/Progress.h b/lldb/include/lldb/Core/Progress.h
index 1f2d48c27d63d3..eb4d9f9d7af08e 100644
--- a/lldb/include/lldb/Core/Progress.h
+++ b/lldb/include/lldb/Core/Progress.h
@@ -136,9 +136,6 @@ class ProgressManager {
static ProgressManager &Instance();
private:
- /// Manage the class instance internally.
- static ProgressManager &InstanceImpl();
-
llvm::StringMap<uint64_t> 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 f7ea209e01727b..9e8deb1ad4e731 100644
--- a/lldb/source/Core/Progress.cpp
+++ b/lldb/source/Core/Progress.cpp
@@ -68,7 +68,11 @@ void Progress::ReportProgress() {
}
}
-ProgressManager &ProgressManager::InstanceImpl() {
+ProgressManager::ProgressManager() : m_progress_category_map() {}
+
+ProgressManager::~ProgressManager() {}
+
+ProgressManager &ProgressManager::Instance() {
static std::once_flag g_once_flag;
static ProgressManager *g_progress_manager = nullptr;
std::call_once(g_once_flag, []() {
@@ -78,12 +82,6 @@ ProgressManager &ProgressManager::InstanceImpl() {
return *g_progress_manager;
}
-ProgressManager::ProgressManager() : m_progress_category_map() {}
-
-ProgressManager::~ProgressManager() {}
-
-ProgressManager &ProgressManager::Instance() { return InstanceImpl(); }
-
void ProgressManager::Increment(std::string title) {
std::lock_guard<std::mutex> lock(m_progress_map_mutex);
m_progress_category_map[title]++;
More information about the lldb-commits
mailing list