[Lldb-commits] [lldb] [lldb][progress] Hook up new broadcast bit and progress manager (PR #83069)
https://github.com/chelcassanova created
https://github.com/llvm/llvm-project/pull/83069
This commit adds the functionality to broadcast events using the
`Debugger::eBroadcastProgressCategory`
bit (https://github.com/llvm/llvm-project/pull/81169) by keeping track of these
reports with the `ProgressManager`
class (https://github.com/llvm/llvm-project/pull/81319). The new bit is used in
such a way that it will only broadcast the initial and final progress reports
for specific progress categories that are managed by the progress manager.
This commit also adds a new test to the progress report unit test that checks
that only the initial/final reports are broadcasted when using the new bit.
>From c27cab4a6fe067aeac6864e7262b2c57a53f2d0c Mon Sep 17 00:00:00 2001
From: Chelsea Cassanova
Date: Tue, 20 Feb 2024 13:56:53 -0800
Subject: [PATCH] [lldb][progress] Hook up new broadcast bit and progress
manager
This commit adds the functionality to broadcast events using the
`Debugger::eBroadcastProgressCategory`
bit (https://github.com/llvm/llvm-project/pull/81169) by keeping track of these
reports with the `ProgressManager`
class (https://github.com/llvm/llvm-project/pull/81319). The new bit is
used in such a way that it will only broadcast the initial and final
progress reports for specific progress categories that are managed by
the progress manager.
This commit also adds a new test to the progress report unit test that
checks that only the initial/final reports are broadcasted when using
the new bit.
---
lldb/include/lldb/Core/Debugger.h | 4 +-
lldb/include/lldb/Core/Progress.h | 41 ++--
lldb/source/Core/Debugger.cpp | 38 ---
lldb/source/Core/Progress.cpp | 47 +-
lldb/unittests/Core/ProgressReportTest.cpp | 57 ++
5 files changed, 164 insertions(+), 23 deletions(-)
diff --git a/lldb/include/lldb/Core/Debugger.h
b/lldb/include/lldb/Core/Debugger.h
index 6ba90eb6ed8fdf..714ca83b890d87 100644
--- a/lldb/include/lldb/Core/Debugger.h
+++ b/lldb/include/lldb/Core/Debugger.h
@@ -593,6 +593,7 @@ class Debugger : public
std::enable_shared_from_this,
friend class CommandInterpreter;
friend class REPL;
friend class Progress;
+ friend class ProgressManager;
/// Report progress events.
///
@@ -626,7 +627,8 @@ class Debugger : public
std::enable_shared_from_this,
static void ReportProgress(uint64_t progress_id, std::string title,
std::string details, uint64_t completed,
uint64_t total,
- std::optional debugger_id);
+ std::optional debugger_id,
+ uint32_t progress_category_bit);
static void ReportDiagnosticImpl(DiagnosticEventData::Type type,
std::string message,
diff --git a/lldb/include/lldb/Core/Progress.h
b/lldb/include/lldb/Core/Progress.h
index eb4d9f9d7af08e..434a155ca7590e 100644
--- a/lldb/include/lldb/Core/Progress.h
+++ b/lldb/include/lldb/Core/Progress.h
@@ -9,10 +9,11 @@
#ifndef LLDB_CORE_PROGRESS_H
#define LLDB_CORE_PROGRESS_H
-#include "lldb/Utility/ConstString.h"
+#include "lldb/lldb-forward.h"
#include "lldb/lldb-types.h"
#include "llvm/ADT/StringMap.h"
#include
+#include
#include
#include
@@ -97,12 +98,32 @@ class Progress {
/// Used to indicate a non-deterministic progress report
static constexpr uint64_t kNonDeterministicTotal = UINT64_MAX;
+ /// Use a struct to send data from a Progress object to
+ /// ProgressManager::ReportProgress. In addition to the Progress member
fields
+ /// this also passes the debugger progress broadcast bit. Using the progress
+ /// category bit indicates that the given progress report is the initial or
+ /// final report for its category.
+ struct ProgressData {
+std::string title;
+std::string details;
+uint64_t progress_id;
+uint64_t completed;
+uint64_t total;
+std::optional debugger_id;
+uint32_t progress_broadcast_bit;
+ };
+
private:
+ friend class Debugger;
void ReportProgress();
static std::atomic g_id;
- /// The title of the progress activity.
+ /// The title of the progress activity, also used as a category to group
+ /// reports.
std::string m_title;
+ /// More specific information about the current file being displayed in the
+ /// report.
std::string m_details;
+ /// Mutex for synchronization.
std::mutex m_mutex;
/// A unique integer identifier for progress reporting.
const uint64_t m_id;
@@ -118,6 +139,8 @@ class Progress {
/// to ensure that we don't send progress updates after progress has
/// completed.
bool m_complete = false;
+ /// Data needed by the debugger to broadcast a progress event.
+ ProgressData m_progress_data;
};
/// A class used to group progress reports by category. This is done by using a
@@ -130,13 +153,21 @@ class Prog
[Lldb-commits] [lldb] [lldb][progress] Hook up new broadcast bit and progress manager (PR #83069)
llvmbot wrote:
@llvm/pr-subscribers-lldb
Author: Chelsea Cassanova (chelcassanova)
Changes
This commit adds the functionality to broadcast events using the
`Debugger::eBroadcastProgressCategory`
bit (https://github.com/llvm/llvm-project/pull/81169) by keeping track of these
reports with the `ProgressManager`
class (https://github.com/llvm/llvm-project/pull/81319). The new bit is used in
such a way that it will only broadcast the initial and final progress reports
for specific progress categories that are managed by the progress manager.
This commit also adds a new test to the progress report unit test that checks
that only the initial/final reports are broadcasted when using the new bit.
---
Full diff: https://github.com/llvm/llvm-project/pull/83069.diff
5 Files Affected:
- (modified) lldb/include/lldb/Core/Debugger.h (+3-1)
- (modified) lldb/include/lldb/Core/Progress.h (+36-5)
- (modified) lldb/source/Core/Debugger.cpp (+32-6)
- (modified) lldb/source/Core/Progress.cpp (+36-11)
- (modified) lldb/unittests/Core/ProgressReportTest.cpp (+57)
``diff
diff --git a/lldb/include/lldb/Core/Debugger.h
b/lldb/include/lldb/Core/Debugger.h
index 6ba90eb6ed8fdf..714ca83b890d87 100644
--- a/lldb/include/lldb/Core/Debugger.h
+++ b/lldb/include/lldb/Core/Debugger.h
@@ -593,6 +593,7 @@ class Debugger : public
std::enable_shared_from_this,
friend class CommandInterpreter;
friend class REPL;
friend class Progress;
+ friend class ProgressManager;
/// Report progress events.
///
@@ -626,7 +627,8 @@ class Debugger : public
std::enable_shared_from_this,
static void ReportProgress(uint64_t progress_id, std::string title,
std::string details, uint64_t completed,
uint64_t total,
- std::optional debugger_id);
+ std::optional debugger_id,
+ uint32_t progress_category_bit);
static void ReportDiagnosticImpl(DiagnosticEventData::Type type,
std::string message,
diff --git a/lldb/include/lldb/Core/Progress.h
b/lldb/include/lldb/Core/Progress.h
index eb4d9f9d7af08e..434a155ca7590e 100644
--- a/lldb/include/lldb/Core/Progress.h
+++ b/lldb/include/lldb/Core/Progress.h
@@ -9,10 +9,11 @@
#ifndef LLDB_CORE_PROGRESS_H
#define LLDB_CORE_PROGRESS_H
-#include "lldb/Utility/ConstString.h"
+#include "lldb/lldb-forward.h"
#include "lldb/lldb-types.h"
#include "llvm/ADT/StringMap.h"
#include
+#include
#include
#include
@@ -97,12 +98,32 @@ class Progress {
/// Used to indicate a non-deterministic progress report
static constexpr uint64_t kNonDeterministicTotal = UINT64_MAX;
+ /// Use a struct to send data from a Progress object to
+ /// ProgressManager::ReportProgress. In addition to the Progress member
fields
+ /// this also passes the debugger progress broadcast bit. Using the progress
+ /// category bit indicates that the given progress report is the initial or
+ /// final report for its category.
+ struct ProgressData {
+std::string title;
+std::string details;
+uint64_t progress_id;
+uint64_t completed;
+uint64_t total;
+std::optional debugger_id;
+uint32_t progress_broadcast_bit;
+ };
+
private:
+ friend class Debugger;
void ReportProgress();
static std::atomic g_id;
- /// The title of the progress activity.
+ /// The title of the progress activity, also used as a category to group
+ /// reports.
std::string m_title;
+ /// More specific information about the current file being displayed in the
+ /// report.
std::string m_details;
+ /// Mutex for synchronization.
std::mutex m_mutex;
/// A unique integer identifier for progress reporting.
const uint64_t m_id;
@@ -118,6 +139,8 @@ class Progress {
/// to ensure that we don't send progress updates after progress has
/// completed.
bool m_complete = false;
+ /// Data needed by the debugger to broadcast a progress event.
+ ProgressData m_progress_data;
};
/// A class used to group progress reports by category. This is done by using a
@@ -130,13 +153,21 @@ class ProgressManager {
~ProgressManager();
/// Control the refcount of the progress report category as needed.
- void Increment(std::string category);
- void Decrement(std::string category);
+ void Increment(Progress::ProgressData);
+ void Decrement(Progress::ProgressData);
static ProgressManager &Instance();
+ llvm::StringMap>
+ GetProgressCategoryMap() {
+return m_progress_category_map;
+ }
+
+ void ReportProgress(Progress::ProgressData);
+
private:
- llvm::StringMap m_progress_category_map;
+ llvm::StringMap>
+ m_progress_category_map;
std::mutex m_progress_map_mutex;
};
diff --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp
index 97311b4716ac2f..cd0ce3a9558ce6 100644
--- a/lldb/source/Core/Debugger.cpp
+++ b/lldb/source/Core/Debugger.cpp
@@ -15,6
[Lldb-commits] [lldb] [lldb][progress] Hook up new broadcast bit and progress manager (PR #83069)
https://github.com/JDevlieghere edited https://github.com/llvm/llvm-project/pull/83069 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][progress] Hook up new broadcast bit and progress manager (PR #83069)
@@ -593,6 +593,7 @@ class Debugger : public std::enable_shared_from_this, friend class CommandInterpreter; friend class REPL; friend class Progress; + friend class ProgressManager; JDevlieghere wrote: I don't think you need this (anymore)? https://github.com/llvm/llvm-project/pull/83069 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][progress] Hook up new broadcast bit and progress manager (PR #83069)
@@ -626,7 +627,8 @@ class Debugger : public std::enable_shared_from_this, static void ReportProgress(uint64_t progress_id, std::string title, std::string details, uint64_t completed, uint64_t total, - std::optional debugger_id); + std::optional debugger_id, + uint32_t progress_category_bit); JDevlieghere wrote: should this default to `eBroadcastBitProgress`? https://github.com/llvm/llvm-project/pull/83069 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][progress] Hook up new broadcast bit and progress manager (PR #83069)
@@ -97,12 +98,32 @@ class Progress {
/// Used to indicate a non-deterministic progress report
static constexpr uint64_t kNonDeterministicTotal = UINT64_MAX;
+ /// Use a struct to send data from a Progress object to
+ /// ProgressManager::ReportProgress. In addition to the Progress member
fields
+ /// this also passes the debugger progress broadcast bit. Using the progress
+ /// category bit indicates that the given progress report is the initial or
+ /// final report for its category.
+ struct ProgressData {
+std::string title;
+std::string details;
+uint64_t progress_id;
+uint64_t completed;
+uint64_t total;
+std::optional debugger_id;
+uint32_t progress_broadcast_bit;
JDevlieghere wrote:
I don't think the `progress_broadcast_bit` should be stored in the struct. The
bit is a property of the progress manager, which will set the bit for the first
and last progress event.
https://github.com/llvm/llvm-project/pull/83069
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][progress] Hook up new broadcast bit and progress manager (PR #83069)
https://github.com/JDevlieghere requested changes to this pull request. I think this is going in the right direction but I think the interaction between the ProgressManager and the Debugger needs to change: At a high level I'm expecting: 1. Progress object is created 2. Progress is added to the ProgressManager 3. Debugger::ReportProgress is called with (`eBroadcastBitProgress | eBroadcastBitProgressCategory`) 4. Incremental updates call directly into Debugger::ReportProgress without going through the manager, with just `Debugger::ReportProgress`. 5. When progress is complete, we go through the progress manager again and report the final progress to both broadcast bits: `eBroadcastBitProgress | eBroadcastBitProgressCategory` https://github.com/llvm/llvm-project/pull/83069 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][progress] Hook up new broadcast bit and progress manager (PR #83069)
@@ -1433,11 +1434,30 @@ void Debugger::SetDestroyCallback(
static void PrivateReportProgress(Debugger &debugger, uint64_t progress_id,
std::string title, std::string details,
uint64_t completed, uint64_t total,
- bool is_debugger_specific) {
+ bool is_debugger_specific,
+ uint32_t progress_broadcast_bit) {
// Only deliver progress events if we have any progress listeners.
const uint32_t event_type = Debugger::eBroadcastBitProgress;
- if (!debugger.GetBroadcaster().EventTypeHasListeners(event_type))
+ const uint32_t category_event_type = Debugger::eBroadcastBitProgressCategory;
+ if (!debugger.GetBroadcaster().EventTypeHasListeners(event_type |
+ category_event_type))
return;
+
+ if (debugger.GetBroadcaster().EventTypeHasListeners(category_event_type)) {
+ProgressManager &progress_manager = ProgressManager::Instance();
+auto map_refcount =
progress_manager.GetProgressCategoryMap().lookup(title);
+
+// Only broadcast the event to the progress category bit if it's an initial
+// or final report for that category. Since we're broadcasting for the
+// category specifically, clear the details.
+if (progress_broadcast_bit == Debugger::eBroadcastBitProgressCategory) {
+ EventSP event_sp(new Event(
+ category_event_type,
+ new ProgressEventData(progress_id, std::move(title), "", completed,
+total, is_debugger_specific)));
+ debugger.GetBroadcaster().BroadcastEvent(event_sp);
+}
+ }
JDevlieghere wrote:
I think it would be better from a layering perspective to keep
`PrivateReportProgress` "dumb" and pass it the right arguments from the
ProgressManager. The map held by the progress manager should be a private
implementation detail. By letting the debugger mess with it you lose the
ability to synchronize access to it. When we do the timeouts, this isn't going
to work.
I expect this function to simply broadcast to whichever broadcast bit is set.
https://github.com/llvm/llvm-project/pull/83069
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][progress] Hook up new broadcast bit and progress manager (PR #83069)
@@ -97,12 +98,32 @@ class Progress {
/// Used to indicate a non-deterministic progress report
static constexpr uint64_t kNonDeterministicTotal = UINT64_MAX;
+ /// Use a struct to send data from a Progress object to
+ /// ProgressManager::ReportProgress. In addition to the Progress member
fields
JDevlieghere wrote:
I think the first sentence isn't entirely accurate. The struct is used to store
information about the progress event in the progress manager?
https://github.com/llvm/llvm-project/pull/83069
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][progress] Hook up new broadcast bit and progress manager (PR #83069)
@@ -97,12 +98,32 @@ class Progress {
/// Used to indicate a non-deterministic progress report
static constexpr uint64_t kNonDeterministicTotal = UINT64_MAX;
+ /// Use a struct to send data from a Progress object to
+ /// ProgressManager::ReportProgress. In addition to the Progress member
fields
+ /// this also passes the debugger progress broadcast bit. Using the progress
+ /// category bit indicates that the given progress report is the initial or
+ /// final report for its category.
+ struct ProgressData {
+std::string title;
+std::string details;
+uint64_t progress_id;
+uint64_t completed;
+uint64_t total;
+std::optional debugger_id;
+uint32_t progress_broadcast_bit;
+ };
+
private:
+ friend class Debugger;
void ReportProgress();
static std::atomic g_id;
- /// The title of the progress activity.
+ /// The title of the progress activity, also used as a category to group
+ /// reports.
std::string m_title;
+ /// More specific information about the current file being displayed in the
+ /// report.
std::string m_details;
+ /// Mutex for synchronization.
std::mutex m_mutex;
/// A unique integer identifier for progress reporting.
JDevlieghere wrote:
Given that you store the ProgressData, did you mean to document the fields in
the new Struct and remove these?
https://github.com/llvm/llvm-project/pull/83069
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][progress] Hook up new broadcast bit and progress manager (PR #83069)
@@ -97,12 +98,32 @@ class Progress {
/// Used to indicate a non-deterministic progress report
static constexpr uint64_t kNonDeterministicTotal = UINT64_MAX;
+ /// Use a struct to send data from a Progress object to
+ /// ProgressManager::ReportProgress. In addition to the Progress member
fields
chelcassanova wrote:
Yes, this can be generalized more since we use the struct for more than just
the call to `ReportProgress`.
https://github.com/llvm/llvm-project/pull/83069
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][progress] Hook up new broadcast bit and progress manager (PR #83069)
@@ -1433,11 +1434,30 @@ void Debugger::SetDestroyCallback(
static void PrivateReportProgress(Debugger &debugger, uint64_t progress_id,
std::string title, std::string details,
uint64_t completed, uint64_t total,
- bool is_debugger_specific) {
+ bool is_debugger_specific,
+ uint32_t progress_broadcast_bit) {
// Only deliver progress events if we have any progress listeners.
const uint32_t event_type = Debugger::eBroadcastBitProgress;
- if (!debugger.GetBroadcaster().EventTypeHasListeners(event_type))
+ const uint32_t category_event_type = Debugger::eBroadcastBitProgressCategory;
+ if (!debugger.GetBroadcaster().EventTypeHasListeners(event_type |
+ category_event_type))
return;
+
+ if (debugger.GetBroadcaster().EventTypeHasListeners(category_event_type)) {
+ProgressManager &progress_manager = ProgressManager::Instance();
+auto map_refcount =
progress_manager.GetProgressCategoryMap().lookup(title);
+
+// Only broadcast the event to the progress category bit if it's an initial
+// or final report for that category. Since we're broadcasting for the
+// category specifically, clear the details.
+if (progress_broadcast_bit == Debugger::eBroadcastBitProgressCategory) {
+ EventSP event_sp(new Event(
+ category_event_type,
+ new ProgressEventData(progress_id, std::move(title), "", completed,
+total, is_debugger_specific)));
+ debugger.GetBroadcaster().BroadcastEvent(event_sp);
+}
+ }
chelcassanova wrote:
So IIUC, instead of passing the broadcast bit as an argument we just set it in
the progress manager and access it from there in the Debugger?
https://github.com/llvm/llvm-project/pull/83069
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][progress] Hook up new broadcast bit and progress manager (PR #83069)
@@ -1433,11 +1434,30 @@ void Debugger::SetDestroyCallback(
static void PrivateReportProgress(Debugger &debugger, uint64_t progress_id,
std::string title, std::string details,
uint64_t completed, uint64_t total,
- bool is_debugger_specific) {
+ bool is_debugger_specific,
+ uint32_t progress_broadcast_bit) {
// Only deliver progress events if we have any progress listeners.
const uint32_t event_type = Debugger::eBroadcastBitProgress;
- if (!debugger.GetBroadcaster().EventTypeHasListeners(event_type))
+ const uint32_t category_event_type = Debugger::eBroadcastBitProgressCategory;
+ if (!debugger.GetBroadcaster().EventTypeHasListeners(event_type |
+ category_event_type))
return;
+
+ if (debugger.GetBroadcaster().EventTypeHasListeners(category_event_type)) {
+ProgressManager &progress_manager = ProgressManager::Instance();
+auto map_refcount =
progress_manager.GetProgressCategoryMap().lookup(title);
+
+// Only broadcast the event to the progress category bit if it's an initial
+// or final report for that category. Since we're broadcasting for the
+// category specifically, clear the details.
+if (progress_broadcast_bit == Debugger::eBroadcastBitProgressCategory) {
+ EventSP event_sp(new Event(
+ category_event_type,
+ new ProgressEventData(progress_id, std::move(title), "", completed,
+total, is_debugger_specific)));
+ debugger.GetBroadcaster().BroadcastEvent(event_sp);
+}
+ }
JDevlieghere wrote:
Not really. I'm saying that I think we don't need to store it at all, because
its implied in when we call `DebuggerReportProgress`.
Can ProgressManager call `ReportProgress(bit = eBroadcastBitProgress |
eBroadcastBitProgressCategory)` for the first and last update, and every other
time call `ReportProgress(bit = eBroadcastBitProgress) every other time?
https://github.com/llvm/llvm-project/pull/83069
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][progress] Hook up new broadcast bit and progress manager (PR #83069)
@@ -593,6 +593,7 @@ class Debugger : public std::enable_shared_from_this, friend class CommandInterpreter; friend class REPL; friend class Progress; + friend class ProgressManager; chelcassanova wrote: `Debugger::ReportProgress` is protected, so to my understanding if we call that function from `ProgressManager::ReportProgress` without having it as a friend it leads to a compile error. https://github.com/llvm/llvm-project/pull/83069 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][progress] Hook up new broadcast bit and progress manager (PR #83069)
@@ -1433,11 +1434,30 @@ void Debugger::SetDestroyCallback(
static void PrivateReportProgress(Debugger &debugger, uint64_t progress_id,
std::string title, std::string details,
uint64_t completed, uint64_t total,
- bool is_debugger_specific) {
+ bool is_debugger_specific,
+ uint32_t progress_broadcast_bit) {
// Only deliver progress events if we have any progress listeners.
const uint32_t event_type = Debugger::eBroadcastBitProgress;
- if (!debugger.GetBroadcaster().EventTypeHasListeners(event_type))
+ const uint32_t category_event_type = Debugger::eBroadcastBitProgressCategory;
+ if (!debugger.GetBroadcaster().EventTypeHasListeners(event_type |
+ category_event_type))
return;
+
+ if (debugger.GetBroadcaster().EventTypeHasListeners(category_event_type)) {
+ProgressManager &progress_manager = ProgressManager::Instance();
+auto map_refcount =
progress_manager.GetProgressCategoryMap().lookup(title);
+
+// Only broadcast the event to the progress category bit if it's an initial
+// or final report for that category. Since we're broadcasting for the
+// category specifically, clear the details.
+if (progress_broadcast_bit == Debugger::eBroadcastBitProgressCategory) {
+ EventSP event_sp(new Event(
+ category_event_type,
+ new ProgressEventData(progress_id, std::move(title), "", completed,
+total, is_debugger_specific)));
+ debugger.GetBroadcaster().BroadcastEvent(event_sp);
+}
+ }
chelcassanova wrote:
So that instead of having it be a member of the struct or the class we just
call `Debugger::ReportProgress(, eBroadcastBitProgress |
eBroadcastBitProgressCategory)` and set the bit as needed from the progress
manager itself?
https://github.com/llvm/llvm-project/pull/83069
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][progress] Hook up new broadcast bit and progress manager (PR #83069)
@@ -1433,11 +1434,30 @@ void Debugger::SetDestroyCallback(
static void PrivateReportProgress(Debugger &debugger, uint64_t progress_id,
std::string title, std::string details,
uint64_t completed, uint64_t total,
- bool is_debugger_specific) {
+ bool is_debugger_specific,
+ uint32_t progress_broadcast_bit) {
// Only deliver progress events if we have any progress listeners.
const uint32_t event_type = Debugger::eBroadcastBitProgress;
- if (!debugger.GetBroadcaster().EventTypeHasListeners(event_type))
+ const uint32_t category_event_type = Debugger::eBroadcastBitProgressCategory;
+ if (!debugger.GetBroadcaster().EventTypeHasListeners(event_type |
+ category_event_type))
return;
+
+ if (debugger.GetBroadcaster().EventTypeHasListeners(category_event_type)) {
+ProgressManager &progress_manager = ProgressManager::Instance();
+auto map_refcount =
progress_manager.GetProgressCategoryMap().lookup(title);
+
+// Only broadcast the event to the progress category bit if it's an initial
+// or final report for that category. Since we're broadcasting for the
+// category specifically, clear the details.
+if (progress_broadcast_bit == Debugger::eBroadcastBitProgressCategory) {
+ EventSP event_sp(new Event(
+ category_event_type,
+ new ProgressEventData(progress_id, std::move(title), "", completed,
+total, is_debugger_specific)));
+ debugger.GetBroadcaster().BroadcastEvent(event_sp);
+}
+ }
chelcassanova wrote:
Such that we set `progress_category_bit` from the progress manager and
broadcast to that as such:
``` EventSP event_sp(new Event(
category_event_type,
new ProgressEventData(progress_id, std::move(title), "", completed,
total, is_debugger_specific)));
debugger.GetBroadcaster().BroadcastEvent(event_sp);
```
https://github.com/llvm/llvm-project/pull/83069
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][progress] Hook up new broadcast bit and progress manager (PR #83069)
https://github.com/chelcassanova edited https://github.com/llvm/llvm-project/pull/83069 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][progress] Hook up new broadcast bit and progress manager (PR #83069)
@@ -1433,11 +1434,30 @@ void Debugger::SetDestroyCallback(
static void PrivateReportProgress(Debugger &debugger, uint64_t progress_id,
std::string title, std::string details,
uint64_t completed, uint64_t total,
- bool is_debugger_specific) {
+ bool is_debugger_specific,
+ uint32_t progress_broadcast_bit) {
// Only deliver progress events if we have any progress listeners.
const uint32_t event_type = Debugger::eBroadcastBitProgress;
- if (!debugger.GetBroadcaster().EventTypeHasListeners(event_type))
+ const uint32_t category_event_type = Debugger::eBroadcastBitProgressCategory;
+ if (!debugger.GetBroadcaster().EventTypeHasListeners(event_type |
+ category_event_type))
return;
+
+ if (debugger.GetBroadcaster().EventTypeHasListeners(category_event_type)) {
+ProgressManager &progress_manager = ProgressManager::Instance();
+auto map_refcount =
progress_manager.GetProgressCategoryMap().lookup(title);
+
+// Only broadcast the event to the progress category bit if it's an initial
+// or final report for that category. Since we're broadcasting for the
+// category specifically, clear the details.
+if (progress_broadcast_bit == Debugger::eBroadcastBitProgressCategory) {
+ EventSP event_sp(new Event(
+ category_event_type,
+ new ProgressEventData(progress_id, std::move(title), "", completed,
+total, is_debugger_specific)));
+ debugger.GetBroadcaster().BroadcastEvent(event_sp);
+}
+ }
chelcassanova wrote:
Also looking back at `PrivateReportProgress`, the variables for the instance
and refcount are leftover from a previous implementation that I had, they
shouldn't be here anymore.
https://github.com/llvm/llvm-project/pull/83069
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][progress] Hook up new broadcast bit and progress manager (PR #83069)
https://github.com/chelcassanova updated
https://github.com/llvm/llvm-project/pull/83069
>From d2854ecb51d5996cd5cabf4e1c1ac9dc6e01240b Mon Sep 17 00:00:00 2001
From: Chelsea Cassanova
Date: Tue, 20 Feb 2024 13:56:53 -0800
Subject: [PATCH] [lldb][progress] Hook up new broadcast bit and progress
manager
This commit adds the functionality to broadcast events using the
`Debugger::eBroadcastProgressCategory`
bit (https://github.com/llvm/llvm-project/pull/81169) by keeping track of these
reports with the `ProgressManager`
class (https://github.com/llvm/llvm-project/pull/81319). The new bit is
used in such a way that it will only broadcast the initial and final
progress reports for specific progress categories that are managed by
the progress manager.
This commit also adds a new test to the progress report unit test that
checks that only the initial/final reports are broadcasted when using
the new bit.
---
lldb/include/lldb/Core/Debugger.h | 10 ++--
lldb/include/lldb/Core/Progress.h | 43 +++-
lldb/source/Core/Debugger.cpp | 25 +++---
lldb/source/Core/Progress.cpp | 42
lldb/unittests/Core/ProgressReportTest.cpp | 57 ++
5 files changed, 146 insertions(+), 31 deletions(-)
diff --git a/lldb/include/lldb/Core/Debugger.h
b/lldb/include/lldb/Core/Debugger.h
index 6ba90eb6ed8fdf..b65ec1029ab24b 100644
--- a/lldb/include/lldb/Core/Debugger.h
+++ b/lldb/include/lldb/Core/Debugger.h
@@ -593,6 +593,7 @@ class Debugger : public
std::enable_shared_from_this,
friend class CommandInterpreter;
friend class REPL;
friend class Progress;
+ friend class ProgressManager;
/// Report progress events.
///
@@ -623,10 +624,11 @@ class Debugger : public
std::enable_shared_from_this,
/// debugger identifier that this progress should be delivered to. If this
/// optional parameter does not have a value, the progress will be
/// delivered to all debuggers.
- static void ReportProgress(uint64_t progress_id, std::string title,
- std::string details, uint64_t completed,
- uint64_t total,
- std::optional debugger_id);
+ static void
+ ReportProgress(uint64_t progress_id, std::string title, std::string details,
+ uint64_t completed, uint64_t total,
+ std::optional debugger_id,
+ uint32_t progress_category_bit = eBroadcastBitProgress);
static void ReportDiagnosticImpl(DiagnosticEventData::Type type,
std::string message,
diff --git a/lldb/include/lldb/Core/Progress.h
b/lldb/include/lldb/Core/Progress.h
index eb4d9f9d7af08e..7554d4fe2b77e9 100644
--- a/lldb/include/lldb/Core/Progress.h
+++ b/lldb/include/lldb/Core/Progress.h
@@ -9,10 +9,11 @@
#ifndef LLDB_CORE_PROGRESS_H
#define LLDB_CORE_PROGRESS_H
-#include "lldb/Utility/ConstString.h"
+#include "lldb/lldb-forward.h"
#include "lldb/lldb-types.h"
#include "llvm/ADT/StringMap.h"
#include
+#include
#include
#include
@@ -97,27 +98,44 @@ class Progress {
/// Used to indicate a non-deterministic progress report
static constexpr uint64_t kNonDeterministicTotal = UINT64_MAX;
+ /// Use a struct to send data from a Progress object to
+ /// the progress manager.
+ struct ProgressData {
+/// The title of the progress activity, also used as a category to group
+/// reports.
+std::string title;
+/// More specific information about the current file being displayed in the
+/// report.
+std::string details;
+/// A unique integer identifier for progress reporting.
+uint64_t progress_id;
+/// How much work ([0...m_total]) that has been completed.
+uint64_t completed;
+/// Total amount of work, use a std::nullopt in the constructor for non
+/// deterministic progress.
+uint64_t total;
+/// The optional debugger ID to report progress to. If this has no value
+/// then all debuggers will receive this event.
+std::optional debugger_id;
+ };
+
private:
+ friend class Debugger;
void ReportProgress();
static std::atomic g_id;
- /// The title of the progress activity.
std::string m_title;
std::string m_details;
std::mutex m_mutex;
- /// A unique integer identifier for progress reporting.
const uint64_t m_id;
- /// How much work ([0...m_total]) that has been completed.
uint64_t m_completed;
- /// Total amount of work, use a std::nullopt in the constructor for non
- /// deterministic progress.
uint64_t m_total;
- /// The optional debugger ID to report progress to. If this has no value then
- /// all debuggers will receive this event.
std::optional m_debugger_id;
/// Set to true when progress has been reported where m_completed == m_total
/// to ensure that we don't send progress updates after progress has
/// completed.
bool m_complete = false;
+ /// Data needed by the deb
[Lldb-commits] [lldb] [lldb][progress] Hook up new broadcast bit and progress manager (PR #83069)
https://github.com/chelcassanova updated
https://github.com/llvm/llvm-project/pull/83069
>From 2cef4a29f0105847fa11b7f6a6ee063184fb838a Mon Sep 17 00:00:00 2001
From: Chelsea Cassanova
Date: Tue, 20 Feb 2024 13:56:53 -0800
Subject: [PATCH] [lldb][progress] Hook up new broadcast bit and progress
manager
This commit adds the functionality to broadcast events using the
`Debugger::eBroadcastProgressCategory`
bit (https://github.com/llvm/llvm-project/pull/81169) by keeping track of these
reports with the `ProgressManager`
class (https://github.com/llvm/llvm-project/pull/81319). The new bit is
used in such a way that it will only broadcast the initial and final
progress reports for specific progress categories that are managed by
the progress manager.
This commit also adds a new test to the progress report unit test that
checks that only the initial/final reports are broadcasted when using
the new bit.
---
lldb/include/lldb/Core/Debugger.h | 10 ++--
lldb/include/lldb/Core/Progress.h | 43 +++-
lldb/source/Core/Debugger.cpp | 25 +++---
lldb/source/Core/Progress.cpp | 40 ---
lldb/unittests/Core/ProgressReportTest.cpp | 57 ++
5 files changed, 145 insertions(+), 30 deletions(-)
diff --git a/lldb/include/lldb/Core/Debugger.h
b/lldb/include/lldb/Core/Debugger.h
index 6ba90eb6ed8fdf..b65ec1029ab24b 100644
--- a/lldb/include/lldb/Core/Debugger.h
+++ b/lldb/include/lldb/Core/Debugger.h
@@ -593,6 +593,7 @@ class Debugger : public
std::enable_shared_from_this,
friend class CommandInterpreter;
friend class REPL;
friend class Progress;
+ friend class ProgressManager;
/// Report progress events.
///
@@ -623,10 +624,11 @@ class Debugger : public
std::enable_shared_from_this,
/// debugger identifier that this progress should be delivered to. If this
/// optional parameter does not have a value, the progress will be
/// delivered to all debuggers.
- static void ReportProgress(uint64_t progress_id, std::string title,
- std::string details, uint64_t completed,
- uint64_t total,
- std::optional debugger_id);
+ static void
+ ReportProgress(uint64_t progress_id, std::string title, std::string details,
+ uint64_t completed, uint64_t total,
+ std::optional debugger_id,
+ uint32_t progress_category_bit = eBroadcastBitProgress);
static void ReportDiagnosticImpl(DiagnosticEventData::Type type,
std::string message,
diff --git a/lldb/include/lldb/Core/Progress.h
b/lldb/include/lldb/Core/Progress.h
index eb4d9f9d7af08e..7554d4fe2b77e9 100644
--- a/lldb/include/lldb/Core/Progress.h
+++ b/lldb/include/lldb/Core/Progress.h
@@ -9,10 +9,11 @@
#ifndef LLDB_CORE_PROGRESS_H
#define LLDB_CORE_PROGRESS_H
-#include "lldb/Utility/ConstString.h"
+#include "lldb/lldb-forward.h"
#include "lldb/lldb-types.h"
#include "llvm/ADT/StringMap.h"
#include
+#include
#include
#include
@@ -97,27 +98,44 @@ class Progress {
/// Used to indicate a non-deterministic progress report
static constexpr uint64_t kNonDeterministicTotal = UINT64_MAX;
+ /// Use a struct to send data from a Progress object to
+ /// the progress manager.
+ struct ProgressData {
+/// The title of the progress activity, also used as a category to group
+/// reports.
+std::string title;
+/// More specific information about the current file being displayed in the
+/// report.
+std::string details;
+/// A unique integer identifier for progress reporting.
+uint64_t progress_id;
+/// How much work ([0...m_total]) that has been completed.
+uint64_t completed;
+/// Total amount of work, use a std::nullopt in the constructor for non
+/// deterministic progress.
+uint64_t total;
+/// The optional debugger ID to report progress to. If this has no value
+/// then all debuggers will receive this event.
+std::optional debugger_id;
+ };
+
private:
+ friend class Debugger;
void ReportProgress();
static std::atomic g_id;
- /// The title of the progress activity.
std::string m_title;
std::string m_details;
std::mutex m_mutex;
- /// A unique integer identifier for progress reporting.
const uint64_t m_id;
- /// How much work ([0...m_total]) that has been completed.
uint64_t m_completed;
- /// Total amount of work, use a std::nullopt in the constructor for non
- /// deterministic progress.
uint64_t m_total;
- /// The optional debugger ID to report progress to. If this has no value then
- /// all debuggers will receive this event.
std::optional m_debugger_id;
/// Set to true when progress has been reported where m_completed == m_total
/// to ensure that we don't send progress updates after progress has
/// completed.
bool m_complete = false;
+ /// Data needed by the debu
[Lldb-commits] [lldb] [lldb][progress] Hook up new broadcast bit and progress manager (PR #83069)
@@ -1433,13 +1434,17 @@ void Debugger::SetDestroyCallback(
static void PrivateReportProgress(Debugger &debugger, uint64_t progress_id,
std::string title, std::string details,
uint64_t completed, uint64_t total,
- bool is_debugger_specific) {
+ bool is_debugger_specific,
+ uint32_t progress_broadcast_bit) {
// Only deliver progress events if we have any progress listeners.
const uint32_t event_type = Debugger::eBroadcastBitProgress;
- if (!debugger.GetBroadcaster().EventTypeHasListeners(event_type))
+ const uint32_t category_event_type = Debugger::eBroadcastBitProgressCategory;
+ if (!debugger.GetBroadcaster().EventTypeHasListeners(event_type |
+ category_event_type))
JDevlieghere wrote:
You only have to check the `progress_broadcast_bit`.
https://github.com/llvm/llvm-project/pull/83069
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][progress] Hook up new broadcast bit and progress manager (PR #83069)
@@ -1929,6 +1938,8 @@ lldb::thread_result_t Debugger::DefaultEventHandler() {
} else if (broadcaster == &m_broadcaster) {
if (event_type & Debugger::eBroadcastBitProgress)
HandleProgressEvent(event_sp);
+else if (event_type & Debugger::eBroadcastBitProgressCategory)
+ HandleProgressEvent(event_sp);
JDevlieghere wrote:
Now we're going to handle the same event twice, once when broadcast as
`eBroadcastBitProgress` and once when broadcast as
`eBroadcastBitProgressCategory`. In the default event handler we want to see
all the individual updates and as that's a superset, we should not handle
`eBroadcastBitProgressCategory`.
https://github.com/llvm/llvm-project/pull/83069
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][progress] Hook up new broadcast bit and progress manager (PR #83069)
@@ -29,8 +30,12 @@ Progress::Progress(std::string title, std::string details,
if (debugger)
m_debugger_id = debugger->GetID();
+
+ m_progress_data = {m_title, m_details, m_id,
+ m_completed, m_total, m_debugger_id};
JDevlieghere wrote:
Did `clang-format` format this like this? Interesting...
Anyway, you should initialize this in the constructors initializer list (and
get rid of `m_title` etc) as per my previous comment.
https://github.com/llvm/llvm-project/pull/83069
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][progress] Hook up new broadcast bit and progress manager (PR #83069)
@@ -1875,7 +1883,8 @@ lldb::thread_result_t Debugger::DefaultEventHandler() {
listener_sp->StartListeningForEvents(
&m_broadcaster, eBroadcastBitProgress | eBroadcastBitWarning |
- eBroadcastBitError | eBroadcastSymbolChange);
+ eBroadcastBitError | eBroadcastSymbolChange |
JDevlieghere wrote:
See my comment below why we shouldn't listen for
`eBroadcastBitProgressCategory` in the default event handler.
https://github.com/llvm/llvm-project/pull/83069
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][progress] Hook up new broadcast bit and progress manager (PR #83069)
@@ -97,27 +98,44 @@ class Progress {
/// Used to indicate a non-deterministic progress report
static constexpr uint64_t kNonDeterministicTotal = UINT64_MAX;
+ /// Use a struct to send data from a Progress object to
+ /// the progress manager.
+ struct ProgressData {
+/// The title of the progress activity, also used as a category to group
+/// reports.
+std::string title;
+/// More specific information about the current file being displayed in the
+/// report.
+std::string details;
+/// A unique integer identifier for progress reporting.
+uint64_t progress_id;
+/// How much work ([0...m_total]) that has been completed.
+uint64_t completed;
+/// Total amount of work, use a std::nullopt in the constructor for non
+/// deterministic progress.
+uint64_t total;
+/// The optional debugger ID to report progress to. If this has no value
+/// then all debuggers will receive this event.
+std::optional debugger_id;
+ };
+
private:
+ friend class Debugger;
void ReportProgress();
static std::atomic g_id;
- /// The title of the progress activity.
std::string m_title;
std::string m_details;
JDevlieghere wrote:
You're still storing the same fields as in `ProgressData`. You should only have
the latter and initialize the progress data in the constructor.
https://github.com/llvm/llvm-project/pull/83069
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][progress] Hook up new broadcast bit and progress manager (PR #83069)
@@ -29,8 +30,12 @@ Progress::Progress(std::string title, std::string details,
if (debugger)
m_debugger_id = debugger->GetID();
+
+ m_progress_data = {m_title, m_details, m_id,
+ m_completed, m_total, m_debugger_id};
chelcassanova wrote:
I'm removing this anyways but yeah not sure what `clang-format` was thinking
here.
https://github.com/llvm/llvm-project/pull/83069
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][progress] Hook up new broadcast bit and progress manager (PR #83069)
https://github.com/chelcassanova updated
https://github.com/llvm/llvm-project/pull/83069
>From 63dded34a47dd161fa918e45aaeecf966ba08476 Mon Sep 17 00:00:00 2001
From: Chelsea Cassanova
Date: Tue, 20 Feb 2024 13:56:53 -0800
Subject: [PATCH] [lldb][progress] Hook up new broadcast bit and progress
manager
This commit adds the functionality to broadcast events using the
`Debugger::eBroadcastProgressCategory`
bit (https://github.com/llvm/llvm-project/pull/81169) by keeping track of these
reports with the `ProgressManager`
class (https://github.com/llvm/llvm-project/pull/81319). The new bit is
used in such a way that it will only broadcast the initial and final
progress reports for specific progress categories that are managed by
the progress manager.
This commit also adds a new test to the progress report unit test that
checks that only the initial/final reports are broadcasted when using
the new bit.
---
lldb/include/lldb/Core/Debugger.h | 10 +--
lldb/include/lldb/Core/Progress.h | 48 +-
lldb/source/Core/Debugger.cpp | 19 +++---
lldb/source/Core/Progress.cpp | 73 +++---
lldb/unittests/Core/ProgressReportTest.cpp | 57 +
5 files changed, 157 insertions(+), 50 deletions(-)
diff --git a/lldb/include/lldb/Core/Debugger.h
b/lldb/include/lldb/Core/Debugger.h
index 6ba90eb6ed8fdf..b65ec1029ab24b 100644
--- a/lldb/include/lldb/Core/Debugger.h
+++ b/lldb/include/lldb/Core/Debugger.h
@@ -593,6 +593,7 @@ class Debugger : public
std::enable_shared_from_this,
friend class CommandInterpreter;
friend class REPL;
friend class Progress;
+ friend class ProgressManager;
/// Report progress events.
///
@@ -623,10 +624,11 @@ class Debugger : public
std::enable_shared_from_this,
/// debugger identifier that this progress should be delivered to. If this
/// optional parameter does not have a value, the progress will be
/// delivered to all debuggers.
- static void ReportProgress(uint64_t progress_id, std::string title,
- std::string details, uint64_t completed,
- uint64_t total,
- std::optional debugger_id);
+ static void
+ ReportProgress(uint64_t progress_id, std::string title, std::string details,
+ uint64_t completed, uint64_t total,
+ std::optional debugger_id,
+ uint32_t progress_category_bit = eBroadcastBitProgress);
static void ReportDiagnosticImpl(DiagnosticEventData::Type type,
std::string message,
diff --git a/lldb/include/lldb/Core/Progress.h
b/lldb/include/lldb/Core/Progress.h
index eb4d9f9d7af08e..1dad11a73429ba 100644
--- a/lldb/include/lldb/Core/Progress.h
+++ b/lldb/include/lldb/Core/Progress.h
@@ -9,10 +9,11 @@
#ifndef LLDB_CORE_PROGRESS_H
#define LLDB_CORE_PROGRESS_H
-#include "lldb/Utility/ConstString.h"
+#include "lldb/lldb-forward.h"
#include "lldb/lldb-types.h"
#include "llvm/ADT/StringMap.h"
#include
+#include
#include
#include
@@ -97,27 +98,37 @@ class Progress {
/// Used to indicate a non-deterministic progress report
static constexpr uint64_t kNonDeterministicTotal = UINT64_MAX;
+ /// Use a struct to send data from a Progress object to
+ /// the progress manager.
+ struct ProgressData {
+/// The title of the progress activity, also used as a category to group
+/// reports.
+std::string title;
+/// More specific information about the current file being displayed in the
+/// report.
+std::string details;
+/// A unique integer identifier for progress reporting.
+uint64_t progress_id;
+/// How much work ([0...m_total]) that has been completed.
+uint64_t completed;
+/// Total amount of work, use a std::nullopt in the constructor for non
+/// deterministic progress.
+uint64_t total;
+/// The optional debugger ID to report progress to. If this has no value
+/// then all debuggers will receive this event.
+std::optional debugger_id;
+ };
+
private:
void ReportProgress();
static std::atomic g_id;
- /// The title of the progress activity.
- std::string m_title;
- std::string m_details;
std::mutex m_mutex;
- /// A unique integer identifier for progress reporting.
- const uint64_t m_id;
- /// How much work ([0...m_total]) that has been completed.
- uint64_t m_completed;
- /// Total amount of work, use a std::nullopt in the constructor for non
- /// deterministic progress.
- uint64_t m_total;
- /// The optional debugger ID to report progress to. If this has no value then
- /// all debuggers will receive this event.
- std::optional m_debugger_id;
/// Set to true when progress has been reported where m_completed == m_total
/// to ensure that we don't send progress updates after progress has
/// completed.
bool m_complete = false;
+ /// Data needed by the debugger to broadcast a progress ev
[Lldb-commits] [lldb] [lldb][progress] Hook up new broadcast bit and progress manager (PR #83069)
@@ -82,20 +94,37 @@ ProgressManager &ProgressManager::Instance() {
return *g_progress_manager;
}
-void ProgressManager::Increment(std::string title) {
+void ProgressManager::Increment(Progress::ProgressData progress_data) {
JDevlieghere wrote:
This should take a const ref to avoid the copy `const Progress::ProgressData&
progress_data`. If we'd unconditionally store it in the map I'd say the copy is
fine as you'd be able to move it into the map, but I'm assuming most of the
time we're only using the title to do the lookup.
https://github.com/llvm/llvm-project/pull/83069
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][progress] Hook up new broadcast bit and progress manager (PR #83069)
@@ -97,27 +98,37 @@ class Progress {
/// Used to indicate a non-deterministic progress report
static constexpr uint64_t kNonDeterministicTotal = UINT64_MAX;
+ /// Use a struct to send data from a Progress object to
+ /// the progress manager.
+ struct ProgressData {
+/// The title of the progress activity, also used as a category to group
+/// reports.
+std::string title;
+/// More specific information about the current file being displayed in the
+/// report.
+std::string details;
+/// A unique integer identifier for progress reporting.
+uint64_t progress_id;
+/// How much work ([0...m_total]) that has been completed.
+uint64_t completed;
+/// Total amount of work, use a std::nullopt in the constructor for non
+/// deterministic progress.
+uint64_t total;
+/// The optional debugger ID to report progress to. If this has no value
+/// then all debuggers will receive this event.
+std::optional debugger_id;
+ };
+
private:
void ReportProgress();
static std::atomic g_id;
- /// The title of the progress activity.
- std::string m_title;
- std::string m_details;
std::mutex m_mutex;
- /// A unique integer identifier for progress reporting.
- const uint64_t m_id;
- /// How much work ([0...m_total]) that has been completed.
- uint64_t m_completed;
- /// Total amount of work, use a std::nullopt in the constructor for non
- /// deterministic progress.
- uint64_t m_total;
- /// The optional debugger ID to report progress to. If this has no value then
- /// all debuggers will receive this event.
- std::optional m_debugger_id;
/// Set to true when progress has been reported where m_completed == m_total
/// to ensure that we don't send progress updates after progress has
/// completed.
bool m_complete = false;
+ /// Data needed by the debugger to broadcast a progress event.
JDevlieghere wrote:
Suggestion:
```
/// Data data belonging to this Progress event. This data is used by the
Debugger to broadcast the event and by the ProgressManager for bookkeeping.
```
https://github.com/llvm/llvm-project/pull/83069
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][progress] Hook up new broadcast bit and progress manager (PR #83069)
https://github.com/JDevlieghere approved this pull request. LGTM with the nit addressed. https://github.com/llvm/llvm-project/pull/83069 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][progress] Hook up new broadcast bit and progress manager (PR #83069)
https://github.com/JDevlieghere edited https://github.com/llvm/llvm-project/pull/83069 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][progress] Hook up new broadcast bit and progress manager (PR #83069)
@@ -82,20 +94,37 @@ ProgressManager &ProgressManager::Instance() {
return *g_progress_manager;
}
-void ProgressManager::Increment(std::string title) {
+void ProgressManager::Increment(Progress::ProgressData progress_data) {
chelcassanova wrote:
Yeah, the title is used for map queries but the data itself isn't stored in the
map, it's passed on to `ProgressManager::ReportProgress`. If we change it to a
const ref (`const Progress::ProgressData& progress_data`) here then we probably
also want to do this for `Decrement` since it uses the map in a similar way.
https://github.com/llvm/llvm-project/pull/83069
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][progress] Hook up new broadcast bit and progress manager (PR #83069)
https://github.com/chelcassanova updated
https://github.com/llvm/llvm-project/pull/83069
>From 03268312834c61a16c4bc28efc442fcd027ec0a9 Mon Sep 17 00:00:00 2001
From: Chelsea Cassanova
Date: Tue, 20 Feb 2024 13:56:53 -0800
Subject: [PATCH] [lldb][progress] Hook up new broadcast bit and progress
manager
This commit adds the functionality to broadcast events using the
`Debugger::eBroadcastProgressCategory`
bit (https://github.com/llvm/llvm-project/pull/81169) by keeping track of these
reports with the `ProgressManager`
class (https://github.com/llvm/llvm-project/pull/81319). The new bit is
used in such a way that it will only broadcast the initial and final
progress reports for specific progress categories that are managed by
the progress manager.
This commit also adds a new test to the progress report unit test that
checks that only the initial/final reports are broadcasted when using
the new bit.
---
lldb/include/lldb/Core/Debugger.h | 10 +--
lldb/include/lldb/Core/Progress.h | 48 +-
lldb/source/Core/Debugger.cpp | 19 +++---
lldb/source/Core/Progress.cpp | 73 +++---
lldb/unittests/Core/ProgressReportTest.cpp | 57 +
5 files changed, 157 insertions(+), 50 deletions(-)
diff --git a/lldb/include/lldb/Core/Debugger.h
b/lldb/include/lldb/Core/Debugger.h
index 6ba90eb6ed8fdf..b65ec1029ab24b 100644
--- a/lldb/include/lldb/Core/Debugger.h
+++ b/lldb/include/lldb/Core/Debugger.h
@@ -593,6 +593,7 @@ class Debugger : public
std::enable_shared_from_this,
friend class CommandInterpreter;
friend class REPL;
friend class Progress;
+ friend class ProgressManager;
/// Report progress events.
///
@@ -623,10 +624,11 @@ class Debugger : public
std::enable_shared_from_this,
/// debugger identifier that this progress should be delivered to. If this
/// optional parameter does not have a value, the progress will be
/// delivered to all debuggers.
- static void ReportProgress(uint64_t progress_id, std::string title,
- std::string details, uint64_t completed,
- uint64_t total,
- std::optional debugger_id);
+ static void
+ ReportProgress(uint64_t progress_id, std::string title, std::string details,
+ uint64_t completed, uint64_t total,
+ std::optional debugger_id,
+ uint32_t progress_category_bit = eBroadcastBitProgress);
static void ReportDiagnosticImpl(DiagnosticEventData::Type type,
std::string message,
diff --git a/lldb/include/lldb/Core/Progress.h
b/lldb/include/lldb/Core/Progress.h
index eb4d9f9d7af08e..d3077ef4823eac 100644
--- a/lldb/include/lldb/Core/Progress.h
+++ b/lldb/include/lldb/Core/Progress.h
@@ -9,10 +9,11 @@
#ifndef LLDB_CORE_PROGRESS_H
#define LLDB_CORE_PROGRESS_H
-#include "lldb/Utility/ConstString.h"
+#include "lldb/lldb-forward.h"
#include "lldb/lldb-types.h"
#include "llvm/ADT/StringMap.h"
#include
+#include
#include
#include
@@ -97,27 +98,37 @@ class Progress {
/// Used to indicate a non-deterministic progress report
static constexpr uint64_t kNonDeterministicTotal = UINT64_MAX;
+ /// Use a struct to send data from a Progress object to
+ /// the progress manager.
+ struct ProgressData {
+/// Data belonging to this Progress event. This data is used by the
Debugger
+/// to broadcast the event and by the ProgressManager for bookkeeping.
+std::string title;
+/// More specific information about the current file being displayed in the
+/// report.
+std::string details;
+/// A unique integer identifier for progress reporting.
+uint64_t progress_id;
+/// How much work ([0...m_total]) that has been completed.
+uint64_t completed;
+/// Total amount of work, use a std::nullopt in the constructor for non
+/// deterministic progress.
+uint64_t total;
+/// The optional debugger ID to report progress to. If this has no value
+/// then all debuggers will receive this event.
+std::optional debugger_id;
+ };
+
private:
void ReportProgress();
static std::atomic g_id;
- /// The title of the progress activity.
- std::string m_title;
- std::string m_details;
std::mutex m_mutex;
- /// A unique integer identifier for progress reporting.
- const uint64_t m_id;
- /// How much work ([0...m_total]) that has been completed.
- uint64_t m_completed;
- /// Total amount of work, use a std::nullopt in the constructor for non
- /// deterministic progress.
- uint64_t m_total;
- /// The optional debugger ID to report progress to. If this has no value then
- /// all debuggers will receive this event.
- std::optional m_debugger_id;
/// Set to true when progress has been reported where m_completed == m_total
/// to ensure that we don't send progress updates after progress has
/// completed.
bool m_complete = false
[Lldb-commits] [lldb] [lldb][progress] Hook up new broadcast bit and progress manager (PR #83069)
https://github.com/chelcassanova updated
https://github.com/llvm/llvm-project/pull/83069
>From 40caaa80180d4845393fc4b80ee2dc09b7c87a7e Mon Sep 17 00:00:00 2001
From: Chelsea Cassanova
Date: Tue, 20 Feb 2024 13:56:53 -0800
Subject: [PATCH] [lldb][progress] Hook up new broadcast bit and progress
manager
This commit adds the functionality to broadcast events using the
`Debugger::eBroadcastProgressCategory`
bit (https://github.com/llvm/llvm-project/pull/81169) by keeping track of these
reports with the `ProgressManager`
class (https://github.com/llvm/llvm-project/pull/81319). The new bit is
used in such a way that it will only broadcast the initial and final
progress reports for specific progress categories that are managed by
the progress manager.
This commit also adds a new test to the progress report unit test that
checks that only the initial/final reports are broadcasted when using
the new bit.
---
lldb/include/lldb/Core/Debugger.h | 10 +--
lldb/include/lldb/Core/Progress.h | 47 +-
lldb/source/Core/Debugger.cpp | 19 +++---
lldb/source/Core/Progress.cpp | 73 +++---
lldb/unittests/Core/ProgressReportTest.cpp | 57 +
5 files changed, 156 insertions(+), 50 deletions(-)
diff --git a/lldb/include/lldb/Core/Debugger.h
b/lldb/include/lldb/Core/Debugger.h
index 6ba90eb6ed8fdf..b65ec1029ab24b 100644
--- a/lldb/include/lldb/Core/Debugger.h
+++ b/lldb/include/lldb/Core/Debugger.h
@@ -593,6 +593,7 @@ class Debugger : public
std::enable_shared_from_this,
friend class CommandInterpreter;
friend class REPL;
friend class Progress;
+ friend class ProgressManager;
/// Report progress events.
///
@@ -623,10 +624,11 @@ class Debugger : public
std::enable_shared_from_this,
/// debugger identifier that this progress should be delivered to. If this
/// optional parameter does not have a value, the progress will be
/// delivered to all debuggers.
- static void ReportProgress(uint64_t progress_id, std::string title,
- std::string details, uint64_t completed,
- uint64_t total,
- std::optional debugger_id);
+ static void
+ ReportProgress(uint64_t progress_id, std::string title, std::string details,
+ uint64_t completed, uint64_t total,
+ std::optional debugger_id,
+ uint32_t progress_category_bit = eBroadcastBitProgress);
static void ReportDiagnosticImpl(DiagnosticEventData::Type type,
std::string message,
diff --git a/lldb/include/lldb/Core/Progress.h
b/lldb/include/lldb/Core/Progress.h
index eb4d9f9d7af08e..b2038a9ffd9317 100644
--- a/lldb/include/lldb/Core/Progress.h
+++ b/lldb/include/lldb/Core/Progress.h
@@ -9,10 +9,11 @@
#ifndef LLDB_CORE_PROGRESS_H
#define LLDB_CORE_PROGRESS_H
-#include "lldb/Utility/ConstString.h"
+#include "lldb/lldb-forward.h"
#include "lldb/lldb-types.h"
#include "llvm/ADT/StringMap.h"
#include
+#include
#include
#include
@@ -97,27 +98,36 @@ class Progress {
/// Used to indicate a non-deterministic progress report
static constexpr uint64_t kNonDeterministicTotal = UINT64_MAX;
+/// Data belonging to this Progress event. This data is used by the
Debugger
+/// to broadcast the event and by the ProgressManager for bookkeeping.
+ struct ProgressData {
+/// The title of the progress activity, also used as a category.
+std::string title;
+/// More specific information about the current file being displayed in the
+/// report.
+std::string details;
+/// A unique integer identifier for progress reporting.
+uint64_t progress_id;
+/// How much work ([0...m_total]) that has been completed.
+uint64_t completed;
+/// Total amount of work, use a std::nullopt in the constructor for non
+/// deterministic progress.
+uint64_t total;
+/// The optional debugger ID to report progress to. If this has no value
+/// then all debuggers will receive this event.
+std::optional debugger_id;
+ };
+
private:
void ReportProgress();
static std::atomic g_id;
- /// The title of the progress activity.
- std::string m_title;
- std::string m_details;
std::mutex m_mutex;
- /// A unique integer identifier for progress reporting.
- const uint64_t m_id;
- /// How much work ([0...m_total]) that has been completed.
- uint64_t m_completed;
- /// Total amount of work, use a std::nullopt in the constructor for non
- /// deterministic progress.
- uint64_t m_total;
- /// The optional debugger ID to report progress to. If this has no value then
- /// all debuggers will receive this event.
- std::optional m_debugger_id;
/// Set to true when progress has been reported where m_completed == m_total
/// to ensure that we don't send progress updates after progress has
/// completed.
bool m_complete = false;
+ /// Data need
[Lldb-commits] [lldb] [lldb][progress] Hook up new broadcast bit and progress manager (PR #83069)
github-actions[bot] wrote:
:warning: C/C++ code formatter, clang-format found issues in your code.
:warning:
You can test this locally with the following command:
``bash
git-clang-format --diff f01ed3bc8884223bf3edbaad8d3685622444cbf5
40caaa80180d4845393fc4b80ee2dc09b7c87a7e -- lldb/include/lldb/Core/Debugger.h
lldb/include/lldb/Core/Progress.h lldb/source/Core/Debugger.cpp
lldb/source/Core/Progress.cpp lldb/unittests/Core/ProgressReportTest.cpp
``
View the diff from clang-format here.
``diff
diff --git a/lldb/include/lldb/Core/Progress.h
b/lldb/include/lldb/Core/Progress.h
index b2038a9ffd..450c0b4399 100644
--- a/lldb/include/lldb/Core/Progress.h
+++ b/lldb/include/lldb/Core/Progress.h
@@ -98,8 +98,8 @@ public:
/// Used to indicate a non-deterministic progress report
static constexpr uint64_t kNonDeterministicTotal = UINT64_MAX;
-/// Data belonging to this Progress event. This data is used by the
Debugger
-/// to broadcast the event and by the ProgressManager for bookkeeping.
+ /// Data belonging to this Progress event. This data is used by the Debugger
+ /// to broadcast the event and by the ProgressManager for bookkeeping.
struct ProgressData {
/// The title of the progress activity, also used as a category.
std::string title;
``
https://github.com/llvm/llvm-project/pull/83069
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][progress] Hook up new broadcast bit and progress manager (PR #83069)
https://github.com/chelcassanova updated
https://github.com/llvm/llvm-project/pull/83069
>From f38204941062691bf3e3f6e57d8171a5c0258f77 Mon Sep 17 00:00:00 2001
From: Chelsea Cassanova
Date: Tue, 20 Feb 2024 13:56:53 -0800
Subject: [PATCH] [lldb][progress] Hook up new broadcast bit and progress
manager
This commit adds the functionality to broadcast events using the
`Debugger::eBroadcastProgressCategory`
bit (https://github.com/llvm/llvm-project/pull/81169) by keeping track of these
reports with the `ProgressManager`
class (https://github.com/llvm/llvm-project/pull/81319). The new bit is
used in such a way that it will only broadcast the initial and final
progress reports for specific progress categories that are managed by
the progress manager.
This commit also adds a new test to the progress report unit test that
checks that only the initial/final reports are broadcasted when using
the new bit.
---
lldb/include/lldb/Core/Debugger.h | 10 +--
lldb/include/lldb/Core/Progress.h | 47 +-
lldb/source/Core/Debugger.cpp | 19 +++---
lldb/source/Core/Progress.cpp | 73 +++---
lldb/unittests/Core/ProgressReportTest.cpp | 57 +
5 files changed, 156 insertions(+), 50 deletions(-)
diff --git a/lldb/include/lldb/Core/Debugger.h
b/lldb/include/lldb/Core/Debugger.h
index 6ba90eb6ed8fdf..b65ec1029ab24b 100644
--- a/lldb/include/lldb/Core/Debugger.h
+++ b/lldb/include/lldb/Core/Debugger.h
@@ -593,6 +593,7 @@ class Debugger : public
std::enable_shared_from_this,
friend class CommandInterpreter;
friend class REPL;
friend class Progress;
+ friend class ProgressManager;
/// Report progress events.
///
@@ -623,10 +624,11 @@ class Debugger : public
std::enable_shared_from_this,
/// debugger identifier that this progress should be delivered to. If this
/// optional parameter does not have a value, the progress will be
/// delivered to all debuggers.
- static void ReportProgress(uint64_t progress_id, std::string title,
- std::string details, uint64_t completed,
- uint64_t total,
- std::optional debugger_id);
+ static void
+ ReportProgress(uint64_t progress_id, std::string title, std::string details,
+ uint64_t completed, uint64_t total,
+ std::optional debugger_id,
+ uint32_t progress_category_bit = eBroadcastBitProgress);
static void ReportDiagnosticImpl(DiagnosticEventData::Type type,
std::string message,
diff --git a/lldb/include/lldb/Core/Progress.h
b/lldb/include/lldb/Core/Progress.h
index eb4d9f9d7af08e..450c0b439910f2 100644
--- a/lldb/include/lldb/Core/Progress.h
+++ b/lldb/include/lldb/Core/Progress.h
@@ -9,10 +9,11 @@
#ifndef LLDB_CORE_PROGRESS_H
#define LLDB_CORE_PROGRESS_H
-#include "lldb/Utility/ConstString.h"
+#include "lldb/lldb-forward.h"
#include "lldb/lldb-types.h"
#include "llvm/ADT/StringMap.h"
#include
+#include
#include
#include
@@ -97,27 +98,36 @@ class Progress {
/// Used to indicate a non-deterministic progress report
static constexpr uint64_t kNonDeterministicTotal = UINT64_MAX;
+ /// Data belonging to this Progress event. This data is used by the Debugger
+ /// to broadcast the event and by the ProgressManager for bookkeeping.
+ struct ProgressData {
+/// The title of the progress activity, also used as a category.
+std::string title;
+/// More specific information about the current file being displayed in the
+/// report.
+std::string details;
+/// A unique integer identifier for progress reporting.
+uint64_t progress_id;
+/// How much work ([0...m_total]) that has been completed.
+uint64_t completed;
+/// Total amount of work, use a std::nullopt in the constructor for non
+/// deterministic progress.
+uint64_t total;
+/// The optional debugger ID to report progress to. If this has no value
+/// then all debuggers will receive this event.
+std::optional debugger_id;
+ };
+
private:
void ReportProgress();
static std::atomic g_id;
- /// The title of the progress activity.
- std::string m_title;
- std::string m_details;
std::mutex m_mutex;
- /// A unique integer identifier for progress reporting.
- const uint64_t m_id;
- /// How much work ([0...m_total]) that has been completed.
- uint64_t m_completed;
- /// Total amount of work, use a std::nullopt in the constructor for non
- /// deterministic progress.
- uint64_t m_total;
- /// The optional debugger ID to report progress to. If this has no value then
- /// all debuggers will receive this event.
- std::optional m_debugger_id;
/// Set to true when progress has been reported where m_completed == m_total
/// to ensure that we don't send progress updates after progress has
/// completed.
bool m_complete = false;
+ /// Data needed by
[Lldb-commits] [lldb] [lldb][progress] Hook up new broadcast bit and progress manager (PR #83069)
https://github.com/chelcassanova updated
https://github.com/llvm/llvm-project/pull/83069
>From 59e3b1da1335261badeace24d6136d924cda6949 Mon Sep 17 00:00:00 2001
From: Chelsea Cassanova
Date: Tue, 20 Feb 2024 13:56:53 -0800
Subject: [PATCH] [lldb][progress] Hook up new broadcast bit and progress
manager
This commit adds the functionality to broadcast events using the
`Debugger::eBroadcastProgressCategory`
bit (https://github.com/llvm/llvm-project/pull/81169) by keeping track of these
reports with the `ProgressManager`
class (https://github.com/llvm/llvm-project/pull/81319). The new bit is
used in such a way that it will only broadcast the initial and final
progress reports for specific progress categories that are managed by
the progress manager.
This commit also adds a new test to the progress report unit test that
checks that only the initial/final reports are broadcasted when using
the new bit.
---
lldb/include/lldb/Core/Debugger.h | 10 ++-
lldb/include/lldb/Core/Progress.h | 47 +++
lldb/source/Core/Debugger.cpp | 19 +++--
lldb/source/Core/Progress.cpp | 77 +-
lldb/unittests/Core/ProgressReportTest.cpp | 93 +-
5 files changed, 195 insertions(+), 51 deletions(-)
diff --git a/lldb/include/lldb/Core/Debugger.h
b/lldb/include/lldb/Core/Debugger.h
index 6ba90eb6ed8fdf..b65ec1029ab24b 100644
--- a/lldb/include/lldb/Core/Debugger.h
+++ b/lldb/include/lldb/Core/Debugger.h
@@ -593,6 +593,7 @@ class Debugger : public
std::enable_shared_from_this,
friend class CommandInterpreter;
friend class REPL;
friend class Progress;
+ friend class ProgressManager;
/// Report progress events.
///
@@ -623,10 +624,11 @@ class Debugger : public
std::enable_shared_from_this,
/// debugger identifier that this progress should be delivered to. If this
/// optional parameter does not have a value, the progress will be
/// delivered to all debuggers.
- static void ReportProgress(uint64_t progress_id, std::string title,
- std::string details, uint64_t completed,
- uint64_t total,
- std::optional debugger_id);
+ static void
+ ReportProgress(uint64_t progress_id, std::string title, std::string details,
+ uint64_t completed, uint64_t total,
+ std::optional debugger_id,
+ uint32_t progress_category_bit = eBroadcastBitProgress);
static void ReportDiagnosticImpl(DiagnosticEventData::Type type,
std::string message,
diff --git a/lldb/include/lldb/Core/Progress.h
b/lldb/include/lldb/Core/Progress.h
index eb4d9f9d7af08e..450c0b439910f2 100644
--- a/lldb/include/lldb/Core/Progress.h
+++ b/lldb/include/lldb/Core/Progress.h
@@ -9,10 +9,11 @@
#ifndef LLDB_CORE_PROGRESS_H
#define LLDB_CORE_PROGRESS_H
-#include "lldb/Utility/ConstString.h"
+#include "lldb/lldb-forward.h"
#include "lldb/lldb-types.h"
#include "llvm/ADT/StringMap.h"
#include
+#include
#include
#include
@@ -97,27 +98,36 @@ class Progress {
/// Used to indicate a non-deterministic progress report
static constexpr uint64_t kNonDeterministicTotal = UINT64_MAX;
+ /// Data belonging to this Progress event. This data is used by the Debugger
+ /// to broadcast the event and by the ProgressManager for bookkeeping.
+ struct ProgressData {
+/// The title of the progress activity, also used as a category.
+std::string title;
+/// More specific information about the current file being displayed in the
+/// report.
+std::string details;
+/// A unique integer identifier for progress reporting.
+uint64_t progress_id;
+/// How much work ([0...m_total]) that has been completed.
+uint64_t completed;
+/// Total amount of work, use a std::nullopt in the constructor for non
+/// deterministic progress.
+uint64_t total;
+/// The optional debugger ID to report progress to. If this has no value
+/// then all debuggers will receive this event.
+std::optional debugger_id;
+ };
+
private:
void ReportProgress();
static std::atomic g_id;
- /// The title of the progress activity.
- std::string m_title;
- std::string m_details;
std::mutex m_mutex;
- /// A unique integer identifier for progress reporting.
- const uint64_t m_id;
- /// How much work ([0...m_total]) that has been completed.
- uint64_t m_completed;
- /// Total amount of work, use a std::nullopt in the constructor for non
- /// deterministic progress.
- uint64_t m_total;
- /// The optional debugger ID to report progress to. If this has no value then
- /// all debuggers will receive this event.
- std::optional m_debugger_id;
/// Set to true when progress has been reported where m_completed == m_total
/// to ensure that we don't send progress updates after progress has
/// completed.
bool m_complete = false;
+ /// Data needed by th
[Lldb-commits] [lldb] [lldb][progress] Hook up new broadcast bit and progress manager (PR #83069)
https://github.com/chelcassanova updated
https://github.com/llvm/llvm-project/pull/83069
>From 5fcab5d9b7af70861bbc46f096032dce7de41517 Mon Sep 17 00:00:00 2001
From: Chelsea Cassanova
Date: Tue, 20 Feb 2024 13:56:53 -0800
Subject: [PATCH] [lldb][progress] Hook up new broadcast bit and progress
manager
This commit adds the functionality to broadcast events using the
`Debugger::eBroadcastProgressCategory`
bit (https://github.com/llvm/llvm-project/pull/81169) by keeping track of these
reports with the `ProgressManager`
class (https://github.com/llvm/llvm-project/pull/81319). The new bit is
used in such a way that it will only broadcast the initial and final
progress reports for specific progress categories that are managed by
the progress manager.
This commit also adds a new test to the progress report unit test that
checks that only the initial/final reports are broadcasted when using
the new bit.
---
lldb/include/lldb/Core/Debugger.h | 10 ++-
lldb/include/lldb/Core/Progress.h | 37 ++---
lldb/source/Core/Debugger.cpp | 19 +++--
lldb/source/Core/Progress.cpp | 56 +
lldb/unittests/Core/ProgressReportTest.cpp | 93 +-
5 files changed, 177 insertions(+), 38 deletions(-)
diff --git a/lldb/include/lldb/Core/Debugger.h
b/lldb/include/lldb/Core/Debugger.h
index 6ba90eb6ed8fdf..b65ec1029ab24b 100644
--- a/lldb/include/lldb/Core/Debugger.h
+++ b/lldb/include/lldb/Core/Debugger.h
@@ -593,6 +593,7 @@ class Debugger : public
std::enable_shared_from_this,
friend class CommandInterpreter;
friend class REPL;
friend class Progress;
+ friend class ProgressManager;
/// Report progress events.
///
@@ -623,10 +624,11 @@ class Debugger : public
std::enable_shared_from_this,
/// debugger identifier that this progress should be delivered to. If this
/// optional parameter does not have a value, the progress will be
/// delivered to all debuggers.
- static void ReportProgress(uint64_t progress_id, std::string title,
- std::string details, uint64_t completed,
- uint64_t total,
- std::optional debugger_id);
+ static void
+ ReportProgress(uint64_t progress_id, std::string title, std::string details,
+ uint64_t completed, uint64_t total,
+ std::optional debugger_id,
+ uint32_t progress_category_bit = eBroadcastBitProgress);
static void ReportDiagnosticImpl(DiagnosticEventData::Type type,
std::string message,
diff --git a/lldb/include/lldb/Core/Progress.h
b/lldb/include/lldb/Core/Progress.h
index eb4d9f9d7af08e..766bda0cf1c5e5 100644
--- a/lldb/include/lldb/Core/Progress.h
+++ b/lldb/include/lldb/Core/Progress.h
@@ -9,10 +9,11 @@
#ifndef LLDB_CORE_PROGRESS_H
#define LLDB_CORE_PROGRESS_H
-#include "lldb/Utility/ConstString.h"
+#include "lldb/lldb-forward.h"
#include "lldb/lldb-types.h"
#include "llvm/ADT/StringMap.h"
#include
+#include
#include
#include
@@ -97,27 +98,36 @@ class Progress {
/// Used to indicate a non-deterministic progress report
static constexpr uint64_t kNonDeterministicTotal = UINT64_MAX;
+ /// Data belonging to this Progress event that is used for bookkeeping by
+ /// ProgressManager.
+ struct ProgressData {
+/// The title of the progress activity, also used as a category.
+std::string title;
+/// A unique integer identifier for progress reporting.
+uint64_t progress_id;
+/// The optional debugger ID to report progress to. If this has no value
+/// then all debuggers will receive this event.
+std::optional debugger_id;
+ };
+
private:
void ReportProgress();
static std::atomic g_id;
- /// The title of the progress activity.
- std::string m_title;
+ /// More specific information about the current file being displayed in the
+ /// report.
std::string m_details;
- std::mutex m_mutex;
- /// A unique integer identifier for progress reporting.
- const uint64_t m_id;
/// How much work ([0...m_total]) that has been completed.
uint64_t m_completed;
/// Total amount of work, use a std::nullopt in the constructor for non
/// deterministic progress.
uint64_t m_total;
- /// The optional debugger ID to report progress to. If this has no value then
- /// all debuggers will receive this event.
- std::optional m_debugger_id;
+ std::mutex m_mutex;
/// Set to true when progress has been reported where m_completed == m_total
/// to ensure that we don't send progress updates after progress has
/// completed.
bool m_complete = false;
+ /// Data needed by the debugger to broadcast a progress event.
+ ProgressData m_progress_data;
};
/// A class used to group progress reports by category. This is done by using a
@@ -130,13 +140,16 @@ class ProgressManager {
~ProgressManager();
/// Control the refcount of the progress report cat
[Lldb-commits] [lldb] [lldb][progress] Hook up new broadcast bit and progress manager (PR #83069)
https://github.com/JDevlieghere edited https://github.com/llvm/llvm-project/pull/83069 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][progress] Hook up new broadcast bit and progress manager (PR #83069)
https://github.com/JDevlieghere approved this pull request. LGTM! https://github.com/llvm/llvm-project/pull/83069 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][progress] Hook up new broadcast bit and progress manager (PR #83069)
@@ -22,38 +23,47 @@ std::atomic Progress::g_id(0);
Progress::Progress(std::string title, std::string details,
std::optional total,
lldb_private::Debugger *debugger)
-: m_title(title), m_details(details), m_id(++g_id), m_completed(0),
- m_total(Progress::kNonDeterministicTotal) {
+: m_progress_data{title,
+ details,
+ ++g_id,
+ /*m_progress_data.completed*/ 0,
JDevlieghere wrote:
Nit: llvm uses this style: ```/*m_progress_data.completed=*/0,```
https://github.com/llvm/llvm-project/pull/83069
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][progress] Hook up new broadcast bit and progress manager (PR #83069)
https://github.com/JDevlieghere deleted https://github.com/llvm/llvm-project/pull/83069 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][progress] Hook up new broadcast bit and progress manager (PR #83069)
https://github.com/chelcassanova closed https://github.com/llvm/llvm-project/pull/83069 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
