Skip to content

Commit 774c226

Browse files
authored
[LLDB] Add external progress bit category (llvm#120171)
As feedback on llvm#119052, it was recommended I add a new bit to delineate internal and external progress events. This patch adds this new category, and sets up Progress.h to support external events via SBProgress.
1 parent cb1c156 commit 774c226

File tree

4 files changed

+130
-6
lines changed

4 files changed

+130
-6
lines changed

lldb/include/lldb/Core/Progress.h

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,12 @@ namespace lldb_private {
5959

6060
class Progress {
6161
public:
62+
/// Enum to indicate the origin of a progress event, internal or external.
63+
enum class Origin : uint8_t {
64+
eInternal = 0,
65+
eExternal = 1,
66+
};
67+
6268
/// Construct a progress object that will report information.
6369
///
6470
/// The constructor will create a unique progress reporting object and
@@ -83,7 +89,8 @@ class Progress {
8389
Progress(std::string title, std::string details = {},
8490
std::optional<uint64_t> total = std::nullopt,
8591
lldb_private::Debugger *debugger = nullptr,
86-
Timeout<std::nano> minimum_report_time = std::nullopt);
92+
Timeout<std::nano> minimum_report_time = std::nullopt,
93+
Origin origin = Origin::eInternal);
8794

8895
/// Destroy the progress object.
8996
///
@@ -118,6 +125,9 @@ class Progress {
118125
/// The optional debugger ID to report progress to. If this has no value
119126
/// then all debuggers will receive this event.
120127
std::optional<lldb::user_id_t> debugger_id;
128+
129+
/// The origin of the progress event, wheter it is internal or external.
130+
Origin origin;
121131
};
122132

123133
private:

lldb/include/lldb/lldb-enumerations.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1357,6 +1357,8 @@ enum DebuggerBroadcastBit {
13571357
eBroadcastBitError = (1 << 2),
13581358
eBroadcastSymbolChange = (1 << 3),
13591359
eBroadcastBitProgressCategory = (1 << 4),
1360+
eBroadcastBitExternalProgress = (1 << 5),
1361+
eBroadcastBitExternalProgressCategory = (1 << 6),
13601362
};
13611363

13621364
/// Used for expressing severity in logs and diagnostics.

lldb/source/Core/Progress.cpp

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,12 +28,14 @@ static llvm::ManagedStatic<llvm::SignpostEmitter> g_progress_signposts;
2828
Progress::Progress(std::string title, std::string details,
2929
std::optional<uint64_t> total,
3030
lldb_private::Debugger *debugger,
31-
Timeout<std::nano> minimum_report_time)
31+
Timeout<std::nano> minimum_report_time,
32+
Progress::Origin origin)
3233
: m_total(total.value_or(Progress::kNonDeterministicTotal)),
3334
m_minimum_report_time(minimum_report_time),
3435
m_progress_data{title, ++g_id,
3536
debugger ? std::optional<user_id_t>(debugger->GetID())
36-
: std::nullopt},
37+
: std::nullopt,
38+
origin},
3739
m_last_report_time_ns(
3840
std::chrono::nanoseconds(
3941
std::chrono::steady_clock::now().time_since_epoch())
@@ -106,9 +108,15 @@ void Progress::ReportProgress() {
106108
if (completed < m_prev_completed)
107109
return; // An overflow in the m_completed counter. Just ignore these events.
108110

111+
// Change the category bit if we're an internal or external progress.
112+
uint32_t progress_category_bit =
113+
m_progress_data.origin == Progress::Origin::eExternal
114+
? lldb::eBroadcastBitExternalProgress
115+
: lldb::eBroadcastBitProgress;
116+
109117
Debugger::ReportProgress(m_progress_data.progress_id, m_progress_data.title,
110118
m_details, completed, m_total,
111-
m_progress_data.debugger_id);
119+
m_progress_data.debugger_id, progress_category_bit);
112120
m_prev_completed = completed;
113121
}
114122

@@ -201,10 +209,13 @@ void ProgressManager::ReportProgress(
201209
// broadcasting to it since that bit doesn't need that information.
202210
const uint64_t completed =
203211
(type == EventType::Begin) ? 0 : Progress::kNonDeterministicTotal;
212+
const uint32_t progress_category_bit =
213+
progress_data.origin == Progress::Origin::eExternal
214+
? lldb::eBroadcastBitExternalProgressCategory
215+
: lldb::eBroadcastBitProgressCategory;
204216
Debugger::ReportProgress(progress_data.progress_id, progress_data.title, "",
205217
completed, Progress::kNonDeterministicTotal,
206-
progress_data.debugger_id,
207-
lldb::eBroadcastBitProgressCategory);
218+
progress_data.debugger_id, progress_category_bit);
208219
}
209220

210221
void ProgressManager::Expire(llvm::StringRef key) {

lldb/unittests/Core/ProgressReportTest.cpp

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -425,3 +425,104 @@ TEST_F(ProgressReportTest, TestProgressManagerDisjointReports) {
425425

426426
ASSERT_FALSE(listener_sp->GetEvent(event_sp, TIMEOUT));
427427
}
428+
429+
TEST_F(ProgressReportTest, TestExternalReportCreation) {
430+
ListenerSP listener_sp =
431+
CreateListenerFor(lldb::eBroadcastBitExternalProgress);
432+
EventSP event_sp;
433+
const ProgressEventData *data;
434+
435+
// Scope this for RAII on the progress objects.
436+
// Create progress reports and check that their respective events for having
437+
// started and ended are broadcasted.
438+
{
439+
Progress progress1("Progress report 1", "Starting report 1",
440+
/*total=*/std::nullopt, /*debugger=*/nullptr,
441+
/*minimum_report_time=*/std::chrono::seconds(0),
442+
Progress::Origin::eExternal);
443+
Progress progress2("Progress report 2", "Starting report 2",
444+
/*total=*/std::nullopt, /*debugger=*/nullptr,
445+
/*minimum_report_time=*/std::chrono::seconds(0),
446+
Progress::Origin::eExternal);
447+
Progress progress3("Progress report 3", "Starting report 3",
448+
/*total=*/std::nullopt, /*debugger=*/nullptr,
449+
/*minimum_report_time=*/std::chrono::seconds(0),
450+
Progress::Origin::eExternal);
451+
}
452+
453+
// Start popping events from the queue, they should have been recevied
454+
// in this order:
455+
// Starting progress: 1, 2, 3
456+
// Ending progress: 3, 2, 1
457+
ASSERT_TRUE(listener_sp->GetEvent(event_sp, TIMEOUT));
458+
data = ProgressEventData::GetEventDataFromEvent(event_sp.get());
459+
460+
EXPECT_EQ(data->GetDetails(), "Starting report 1");
461+
EXPECT_FALSE(data->IsFinite());
462+
EXPECT_FALSE(data->GetCompleted());
463+
EXPECT_EQ(data->GetTotal(), Progress::kNonDeterministicTotal);
464+
EXPECT_EQ(data->GetMessage(), "Progress report 1: Starting report 1");
465+
466+
ASSERT_TRUE(listener_sp->GetEvent(event_sp, TIMEOUT));
467+
data = ProgressEventData::GetEventDataFromEvent(event_sp.get());
468+
469+
EXPECT_EQ(data->GetDetails(), "Starting report 2");
470+
EXPECT_FALSE(data->IsFinite());
471+
EXPECT_FALSE(data->GetCompleted());
472+
EXPECT_EQ(data->GetTotal(), Progress::kNonDeterministicTotal);
473+
EXPECT_EQ(data->GetMessage(), "Progress report 2: Starting report 2");
474+
475+
ASSERT_TRUE(listener_sp->GetEvent(event_sp, TIMEOUT));
476+
data = ProgressEventData::GetEventDataFromEvent(event_sp.get());
477+
478+
EXPECT_EQ(data->GetDetails(), "Starting report 3");
479+
EXPECT_FALSE(data->IsFinite());
480+
EXPECT_FALSE(data->GetCompleted());
481+
EXPECT_EQ(data->GetTotal(), Progress::kNonDeterministicTotal);
482+
EXPECT_EQ(data->GetMessage(), "Progress report 3: Starting report 3");
483+
484+
// Progress report objects should be destroyed at this point so
485+
// get each report from the queue and check that they've been
486+
// destroyed in reverse order.
487+
ASSERT_TRUE(listener_sp->GetEvent(event_sp, TIMEOUT));
488+
data = ProgressEventData::GetEventDataFromEvent(event_sp.get());
489+
490+
EXPECT_EQ(data->GetTitle(), "Progress report 3");
491+
EXPECT_TRUE(data->GetCompleted());
492+
EXPECT_FALSE(data->IsFinite());
493+
EXPECT_EQ(data->GetMessage(), "Progress report 3: Starting report 3");
494+
495+
ASSERT_TRUE(listener_sp->GetEvent(event_sp, TIMEOUT));
496+
data = ProgressEventData::GetEventDataFromEvent(event_sp.get());
497+
498+
EXPECT_EQ(data->GetTitle(), "Progress report 2");
499+
EXPECT_TRUE(data->GetCompleted());
500+
EXPECT_FALSE(data->IsFinite());
501+
EXPECT_EQ(data->GetMessage(), "Progress report 2: Starting report 2");
502+
503+
ASSERT_TRUE(listener_sp->GetEvent(event_sp, TIMEOUT));
504+
data = ProgressEventData::GetEventDataFromEvent(event_sp.get());
505+
506+
EXPECT_EQ(data->GetTitle(), "Progress report 1");
507+
EXPECT_TRUE(data->GetCompleted());
508+
EXPECT_FALSE(data->IsFinite());
509+
EXPECT_EQ(data->GetMessage(), "Progress report 1: Starting report 1");
510+
}
511+
512+
TEST_F(ProgressReportTest, TestExternalReportNotReceived) {
513+
ListenerSP listener_sp = CreateListenerFor(lldb::eBroadcastBitProgress);
514+
EventSP event_sp;
515+
516+
// Scope this for RAII on the progress objects.
517+
// Create progress reports and check that their respective events for having
518+
// started and ended are broadcasted.
519+
{
520+
Progress progress1("External Progress report 1",
521+
"Starting external report 1",
522+
/*total=*/std::nullopt, /*debugger=*/nullptr,
523+
/*minimum_report_time=*/std::chrono::seconds(0),
524+
Progress::Origin::eExternal);
525+
}
526+
527+
ASSERT_FALSE(listener_sp->GetEvent(event_sp, TIMEOUT));
528+
}

0 commit comments

Comments
 (0)