Skip to content

Commit a1b14db

Browse files
authored
[lldb] Remove progress report coalescing (#130329)
Remove support for coalescing progress reports in LLDB. This functionality was motivated by Xcode, which wanted to listen for less frequent, aggregated progress events at the cost of losing some detail. See the original RFC [1] for more details. Since then, they've reevaluated this trade-off and opted to listen for the regular, full fidelity progress events and do any post processing on their end. rdar://146425487
1 parent b15e7f3 commit a1b14db

File tree

13 files changed

+35
-851
lines changed

13 files changed

+35
-851
lines changed

lldb/include/lldb/Core/Progress.h

Lines changed: 13 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
#ifndef LLDB_CORE_PROGRESS_H
1010
#define LLDB_CORE_PROGRESS_H
1111

12-
#include "lldb/Host/Alarm.h"
1312
#include "lldb/Utility/Timeout.h"
1413
#include "lldb/lldb-forward.h"
1514
#include "lldb/lldb-types.h"
@@ -18,6 +17,7 @@
1817
#include <cstdint>
1918
#include <mutex>
2019
#include <optional>
20+
#include <string>
2121

2222
namespace lldb_private {
2323

@@ -115,21 +115,6 @@ class Progress {
115115
/// Used to indicate a non-deterministic progress report
116116
static constexpr uint64_t kNonDeterministicTotal = UINT64_MAX;
117117

118-
/// Data belonging to this Progress event that is used for bookkeeping by
119-
/// ProgressManager.
120-
struct ProgressData {
121-
/// The title of the progress activity, also used as a category.
122-
std::string title;
123-
/// A unique integer identifier for progress reporting.
124-
uint64_t progress_id;
125-
/// The optional debugger ID to report progress to. If this has no value
126-
/// then all debuggers will receive this event.
127-
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;
131-
};
132-
133118
private:
134119
void ReportProgress();
135120
static std::atomic<uint64_t> g_id;
@@ -141,8 +126,18 @@ class Progress {
141126
// Minimum amount of time between two progress reports.
142127
const Timeout<std::nano> m_minimum_report_time;
143128

144-
/// Data needed by the debugger to broadcast a progress event.
145-
const ProgressData m_progress_data;
129+
/// The title of the progress activity, also used as a category.
130+
const std::string m_title;
131+
132+
/// A unique integer identifier for progress reporting.
133+
const uint64_t m_progress_id;
134+
135+
/// The optional debugger ID to report progress to. If this has no value
136+
/// then all debuggers will receive this event.
137+
const std::optional<lldb::user_id_t> m_debugger_id;
138+
139+
/// The origin of the progress event, whether it is internal or external.
140+
const Origin m_origin;
146141

147142
/// How much work ([0...m_total]) that has been completed.
148143
std::atomic<uint64_t> m_completed = 0;
@@ -161,60 +156,6 @@ class Progress {
161156
std::optional<uint64_t> m_prev_completed;
162157
};
163158

164-
/// A class used to group progress reports by category. This is done by using a
165-
/// map that maintains a refcount of each category of progress reports that have
166-
/// come in. Keeping track of progress reports this way will be done if a
167-
/// debugger is listening to the eBroadcastBitProgressByCategory broadcast bit.
168-
class ProgressManager {
169-
public:
170-
ProgressManager();
171-
~ProgressManager();
172-
173-
/// Control the refcount of the progress report category as needed.
174-
void Increment(const Progress::ProgressData &);
175-
void Decrement(const Progress::ProgressData &);
176-
177-
static void Initialize();
178-
static void Terminate();
179-
static bool Enabled();
180-
static ProgressManager &Instance();
181-
182-
protected:
183-
enum class EventType {
184-
Begin,
185-
End,
186-
};
187-
static void ReportProgress(const Progress::ProgressData &progress_data,
188-
EventType type);
189-
190-
static std::optional<ProgressManager> &InstanceImpl();
191-
192-
/// Helper function for reporting progress when the alarm in the corresponding
193-
/// entry in the map expires.
194-
void Expire(llvm::StringRef key);
195-
196-
/// Entry used for bookkeeping.
197-
struct Entry {
198-
/// Reference count used for overlapping events.
199-
uint64_t refcount = 0;
200-
201-
/// Data used to emit progress events.
202-
Progress::ProgressData data;
203-
204-
/// Alarm handle used when the refcount reaches zero.
205-
Alarm::Handle handle = Alarm::INVALID_HANDLE;
206-
};
207-
208-
/// Map used for bookkeeping.
209-
llvm::StringMap<Entry> m_entries;
210-
211-
/// Mutex to provide the map.
212-
std::mutex m_entries_mutex;
213-
214-
/// Alarm instance to coalesce progress events.
215-
Alarm m_alarm;
216-
};
217-
218159
} // namespace lldb_private
219160

220161
#endif // LLDB_CORE_PROGRESS_H

lldb/include/lldb/Host/Alarm.h

Lines changed: 0 additions & 115 deletions
This file was deleted.

lldb/include/lldb/lldb-enumerations.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1356,9 +1356,9 @@ enum DebuggerBroadcastBit {
13561356
eBroadcastBitWarning = (1 << 1),
13571357
eBroadcastBitError = (1 << 2),
13581358
eBroadcastSymbolChange = (1 << 3),
1359-
eBroadcastBitProgressCategory = (1 << 4),
1359+
eBroadcastBitProgressCategory = (1 << 4), ///< Deprecated
13601360
eBroadcastBitExternalProgress = (1 << 5),
1361-
eBroadcastBitExternalProgressCategory = (1 << 6),
1361+
eBroadcastBitExternalProgressCategory = (1 << 6), ///< Deprecated
13621362
};
13631363

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

lldb/source/API/SystemInitializerFull.cpp

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -68,9 +68,6 @@ llvm::Error SystemInitializerFull::Initialize() {
6868
const char *arg0 = "lldb";
6969
llvm::cl::ParseCommandLineOptions(1, &arg0);
7070

71-
// Initialize the progress manager.
72-
ProgressManager::Initialize();
73-
7471
#define LLDB_PLUGIN(p) LLDB_PLUGIN_INITIALIZE(p);
7572
#include "Plugins/Plugins.def"
7673

@@ -104,9 +101,6 @@ void SystemInitializerFull::Terminate() {
104101
#define LLDB_PLUGIN(p) LLDB_PLUGIN_TERMINATE(p);
105102
#include "Plugins/Plugins.def"
106103

107-
// Terminate the progress manager.
108-
ProgressManager::Terminate();
109-
110104
// Now shutdown the common parts, in reverse order.
111105
SystemInitializerCommon::Terminate();
112106
}

lldb/source/Core/Debugger.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1548,7 +1548,7 @@ void Debugger::ReportProgress(uint64_t progress_id, std::string title,
15481548
std::string details, uint64_t completed,
15491549
uint64_t total,
15501550
std::optional<lldb::user_id_t> debugger_id,
1551-
uint32_t progress_category_bit) {
1551+
uint32_t progress_broadcast_bit) {
15521552
// Check if this progress is for a specific debugger.
15531553
if (debugger_id) {
15541554
// It is debugger specific, grab it and deliver the event if the debugger
@@ -1558,7 +1558,7 @@ void Debugger::ReportProgress(uint64_t progress_id, std::string title,
15581558
PrivateReportProgress(*debugger_sp, progress_id, std::move(title),
15591559
std::move(details), completed, total,
15601560
/*is_debugger_specific*/ true,
1561-
progress_category_bit);
1561+
progress_broadcast_bit);
15621562
return;
15631563
}
15641564
// The progress event is not debugger specific, iterate over all debuggers
@@ -1569,7 +1569,7 @@ void Debugger::ReportProgress(uint64_t progress_id, std::string title,
15691569
for (pos = g_debugger_list_ptr->begin(); pos != end; ++pos)
15701570
PrivateReportProgress(*(*pos), progress_id, title, details, completed,
15711571
total, /*is_debugger_specific*/ false,
1572-
progress_category_bit);
1572+
progress_broadcast_bit);
15731573
}
15741574
}
15751575

0 commit comments

Comments
 (0)