Skip to content

Commit 4dd0782

Browse files
oontvoooontvoo
committed
addressed review comments
1 parent 83bb7c4 commit 4dd0782

File tree

3 files changed

+85
-81
lines changed

3 files changed

+85
-81
lines changed

‎lldb/include/lldb/Core/Telemetry.h‎

Lines changed: 39 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
#include <memory>
2424
#include <optional>
2525
#include <string>
26-
#include <unordered_map>
26+
#include <stack>
2727

2828
namespace lldb_private {
2929
namespace telemetry {
@@ -91,11 +91,12 @@ class TelemetryManager : public llvm::telemetry::Manager {
9191
public:
9292
llvm::Error preDispatch(llvm::telemetry::TelemetryInfo *entry) override;
9393

94-
const llvm::telemetry::Config *getConfig();
9594

9695
virtual void AtDebuggerStartup(DebuggerInfo *entry);
9796
virtual void AtDebuggerExit(DebuggerInfo *entry);
9897

98+
const llvm::telemetry::Config *GetConfig();
99+
99100
virtual llvm::StringRef GetInstanceName() const = 0;
100101
static TelemetryManager *GetInstance();
101102

@@ -112,6 +113,42 @@ class TelemetryManager : public llvm::telemetry::Manager {
112113
static std::unique_ptr<TelemetryManager> g_instance;
113114
};
114115

116+
/// Helper RAII class for collecting telemetry.
117+
class ScopeTelemetryCollector {
118+
public:
119+
ScopeTelemetryCollector () {
120+
if (TelemetryEnabled())
121+
m_start = std::chrono::steady_clock::now();
122+
}
123+
124+
~ScopeTelemetryCollector() {
125+
while(! m_exit_funcs.empty()) {
126+
(m_exit_funcs.top())();
127+
m_exit_funcs.pop();
128+
}
129+
}
130+
131+
bool TelemetryEnabled() {
132+
TelemetryManager* instance = TelemetryManager::GetInstance();
133+
return instance != nullptr && instance->GetConfig()->EnableTelemetry;
134+
}
135+
136+
137+
SteadyTimePoint GetStartTime() {return m_start;}
138+
SteadyTimePoint GetCurrentTime() { return std::chrono::steady_clock::now(); }
139+
140+
template <typename Fp>
141+
void RunAtScopeExit(Fp&& F){
142+
assert(llvm::telemetry::Config::BuildTimeEnableTelemetry && "Telemetry should have been enabled");
143+
m_exit_funcs.push(std::forward<Fp>(F));
144+
}
145+
146+
private:
147+
SteadyTimePoint m_start;
148+
std::stack<std::function<void()>> m_exit_funcs;
149+
150+
};
151+
115152
} // namespace telemetry
116153
} // namespace lldb_private
117154
#endif // LLDB_CORE_TELEMETRY_H

‎lldb/source/Core/Debugger.cpp‎

Lines changed: 30 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@
5252
#include "lldb/Utility/State.h"
5353
#include "lldb/Utility/Stream.h"
5454
#include "lldb/Utility/StreamString.h"
55+
#include "lldb/Core/Telemetry.h"
5556
#include "lldb/lldb-enumerations.h"
5657

5758
#if defined(_WIN32)
@@ -70,11 +71,8 @@
7071
#include "llvm/Support/Threading.h"
7172
#include "llvm/Support/raw_ostream.h"
7273

73-
#ifdef LLVM_BUILD_TELEMETRY
74-
#include "lldb/Core/Telemetry.h"
75-
#include <chrono>
76-
#endif
7774

75+
#include <chrono>
7876
#include <cstdio>
7977
#include <cstdlib>
8078
#include <cstring>
@@ -767,29 +765,27 @@ void Debugger::InstanceInitialize() {
767765

768766
DebuggerSP Debugger::CreateInstance(lldb::LogOutputCallback log_callback,
769767
void *baton) {
770-
#ifdef LLVM_BUILD_TELEMETRY
771-
lldb_private::telemetry::SteadyTimePoint start_time =
772-
std::chrono::steady_clock::now();
773-
#endif
768+
lldb_private::telemetry::ScopeTelemetryCollector helper;
774769
DebuggerSP debugger_sp(new Debugger(log_callback, baton));
770+
771+
if (helper.TelemetryEnabled()) {
772+
helper.RunAtScopeExit([&](){
773+
lldb_private::telemetry::TelemetryManager * manager = lldb_private::telemetry::TelemetryManager::GetInstance();
774+
lldb_private::telemetry::DebuggerInfo entry;
775+
entry.debugger = debugger_sp.get();
776+
entry.start_time = helper.GetStartTime();
777+
entry.end_time = helper.GetCurrentTime();
778+
manager->AtDebuggerStartup(&entry);
779+
});
780+
781+
}
782+
775783
if (g_debugger_list_ptr && g_debugger_list_mutex_ptr) {
776784
std::lock_guard<std::recursive_mutex> guard(*g_debugger_list_mutex_ptr);
777785
g_debugger_list_ptr->push_back(debugger_sp);
778786
}
779787
debugger_sp->InstanceInitialize();
780788

781-
#ifdef LLVM_BUILD_TELEMETRY
782-
if (auto *telemetry_manager = telemetry::TelemetryManager::getInstance()) {
783-
if (telemetry_manager->getConfig()->EnableTelemetry) {
784-
lldb_private::telemetry::DebuggerTelemetryInfo entry;
785-
entry.start_time = start_time;
786-
entry.end_time = std::chrono::steady_clock::now();
787-
entry.debugger = debugger_sp.get();
788-
telemetry_manager->atDebuggerStartup(&entry);
789-
}
790-
}
791-
#endif
792-
793789
return debugger_sp;
794790
}
795791

@@ -1010,10 +1006,20 @@ void Debugger::Clear() {
10101006
// static void Debugger::Destroy(lldb::DebuggerSP &debugger_sp);
10111007
// static void Debugger::Terminate();
10121008
llvm::call_once(m_clear_once, [this]() {
1013-
#ifdef LLVM_BUILD_TELEMETRY
1014-
lldb_private::telemetry::SteadyTimePoint quit_start_time =
1015-
std::chrono::steady_clock::now();
1016-
#endif
1009+
lldb_private::telemetry::ScopeTelemetryCollector helper;
1010+
if (helper.TelemetryEnabled()) {
1011+
// TBD: We *may* have to send off the log BEFORE the ClearIOHanders()?
1012+
helper.RunAtScopeExit([&helper, this](){
1013+
lldb_private::telemetry::TelemetryManager* manager
1014+
= lldb_private::telemetry::TelemetryManager::GetInstance();
1015+
lldb_private::telemetry::DebuggerInfo entry;
1016+
entry.debugger = this;
1017+
entry.exit_desc = {0, ""}; // If we are here, there was no error.
1018+
entry.start_time = helper.GetStartTime();
1019+
entry.end_time = helper.GetCurrentTime();
1020+
manager->AtDebuggerExit(&entry);
1021+
});
1022+
}
10171023
ClearIOHandlers();
10181024
StopIOHandlerThread();
10191025
StopEventHandlerThread();
@@ -1036,19 +1042,6 @@ void Debugger::Clear() {
10361042

10371043
if (Diagnostics::Enabled())
10381044
Diagnostics::Instance().RemoveCallback(m_diagnostics_callback_id);
1039-
1040-
#ifdef LLVM_BUILD_TELEMETRY
1041-
if (auto telemetry_manager = telemetry::TelemetryManager::getInstance()) {
1042-
if (telemetry_manager->getConfig()->EnableTelemetry) {
1043-
// TBD: We *may* have to send off the log BEFORE the ClearIOHanders()?
1044-
lldb_private::telemetry::DebuggerTelemetryInfo entry;
1045-
entry.start_time = quit_start_time;
1046-
entry.end_time = std::chrono::steady_clock::now();
1047-
entry.debugger = this;
1048-
telemetry_manager->atDebuggerExit(&entry);
1049-
}
1050-
}
1051-
#endif
10521045
});
10531046
}
10541047

‎lldb/source/Core/Telemetry.cpp‎

Lines changed: 16 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -68,18 +68,15 @@ void LLDBBaseTelemetryInfo::serialize(Serializer &serializer) const {
6868
void DebuggerInfo::serialize(Serializer &serializer) const {
6969
LLDBBaseTelemetryInfo::serialize(serializer);
7070

71-
serializer.write("username", username);
72-
serializer.write("lldb_git_sha", lldb_git_sha);
73-
serializer.write("lldb_path", lldb_path);
74-
serializer.write("cwd", cwd);
71+
serializer.write("lldb_version", lldb_version);
7572
if (exit_desc.has_value()) {
7673
serializer.write("exit_code", exit_desc->exit_code);
7774
serializer.write("exit_desc", exit_desc->description);
7875
}
7976
}
8077

8178
TelemetryManager::TelemetryManager(std::unique_ptr<Config> config)
82-
: m_config(std::move(config)), m_id(MakeUUID) {}
79+
: m_config(std::move(config)), m_id(MakeUUID()) {}
8380

8481
llvm::Error TelemetryManager::preDispatch(TelemetryInfo *entry) {
8582
// Assign the manager_id, and debugger_id, if available, to this entry.
@@ -97,44 +94,20 @@ void TelemetryManager::AtDebuggerStartup(DebuggerInfo *entry) {
9794
LLDB_LOG_ERROR(GetLog(LLDBLog::Object), std::move(er),
9895
"Failed to dispatch entry at debugger startup: {0}");
9996
}
97+
}
98+
99+
void TelemetryManager::AtDebuggerExit(DebuggerInfo * entry) {
100+
// There must be a reference to the debugger at this point.
101+
assert(entry->debugger != nullptr);
102+
103+
if (llvm::Error er = dispatch(entry)) {
104+
LLDB_LOG_ERROR(GetLog(LLDBLog::Object), std::move(er),
105+
"Failed to dispatch entry at debugger exit: {0}");
106+
}
107+
}
108+
109+
const Config *TelemetryManager::GetConfig() { return m_config.get(); }
100110

101-
void TelemetryManager::AtDebuggerExit(DebuggerInfo * entry) {
102-
// There must be a reference to the debugger at this point.
103-
assert(entry->debugger != nullptr);
104-
105-
if (auto *selected_target =
106-
entry->debugger->GetSelectedExecutionContext().GetTargetPtr()) {
107-
if (!selected_target->IsDummyTarget()) {
108-
const lldb::ProcessSP proc = selected_target->GetProcessSP();
109-
if (proc == nullptr) {
110-
// no process has been launched yet.
111-
entry->exit_desc = {-1, "no process launched."};
112-
} else {
113-
entry->exit_desc = {proc->GetExitStatus(), ""};
114-
if (const char *description = proc->GetExitDescription())
115-
entry->exit_desc->description = std::string(description);
116-
}
117-
}
118-
}
119-
120-
if (llvm::Error er = dispatch(entry)) {
121-
LLDB_LOG_ERROR(GetLog(LLDBLog::Object), std::move(er),
122-
"Failed to dispatch entry at debugger exit: {0}");
123-
}
124-
125-
std::unique_ptr<TelemetryManager> TelemetryManager::g_instance = nullptr;
126-
TelemetryManager *TelemetryManager::getInstance() {
127-
return g_instance.get();
128-
}
129-
130-
void TelemetryManager::setInstance(
131-
std::unique_ptr<TelemetryManager> manager) {
132-
g_instance = std::move(manager);
133-
}
134-
135-
136-
const Config *getConfig() { return m_config.get(); }
137-
138111
std::unique_ptr<TelemetryManager> TelemetryManager::g_instance = nullptr;
139112
TelemetryManager *TelemetryManager::GetInstance() {
140113
if (!Config::BuildTimeEnableTelemetry)
@@ -146,5 +119,6 @@ void TelemetryManager::SetInstance(std::unique_ptr<TelemetryManager> manager) {
146119
g_instance = std::move(manager);
147120
}
148121

122+
149123
} // namespace telemetry
150124
} // namespace lldb_private

0 commit comments

Comments
 (0)