Skip to content

Commit 0a2505a

Browse files
committed
Addressed review comments:
- Use separate entries for Process and Main Executable module - Rename for clarity
1 parent ae73ebe commit 0a2505a

File tree

5 files changed

+93
-59
lines changed

5 files changed

+93
-59
lines changed

lldb/include/lldb/Core/Telemetry.h

Lines changed: 57 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -55,10 +55,11 @@ struct LLDBConfig : public ::llvm::telemetry::Config {
5555
// and differ by the last two bits)
5656
struct LLDBEntryKind : public ::llvm::telemetry::EntryKind {
5757
// clang-format off
58-
static const llvm::telemetry::KindType BaseInfo = 0b11000000;
59-
static const llvm::telemetry::KindType CommandInfo = 0b11010000;
60-
static const llvm::telemetry::KindType DebuggerInfo = 0b11001000;
61-
static const llvm::telemetry::KindType TargetInfo = 0b11000100;
58+
static const llvm::telemetry::KindType BaseInfo = 0b11000000;
59+
static const llvm::telemetry::KindType CommandInfo = 0b11010000;
60+
static const llvm::telemetry::KindType DebuggerInfo = 0b11001000;
61+
static const llvm::telemetry::KindType ExecModuleInfo = 0b11000100;
62+
static const llvm::telemetry::KindType ProcessExitInfo = 0b11001100;
6263
// clang-format on
6364
};
6465

@@ -88,42 +89,6 @@ struct LLDBBaseTelemetryInfo : public llvm::telemetry::TelemetryInfo {
8889
void serialize(llvm::telemetry::Serializer &serializer) const override;
8990
};
9091

91-
/// Describes an exit status.
92-
struct ExitDescription {
93-
int exit_code;
94-
std::string description;
95-
};
96-
97-
struct TargetInfo : public LLDBBaseTelemetryInfo {
98-
lldb::ModuleSP exec_mod;
99-
100-
/// The same as the executable-module's UUID.
101-
UUID target_uuid;
102-
/// PID of the process owned by this target.
103-
lldb::pid_t pid;
104-
std::string arch_name;
105-
106-
/// If true, this entry was emitted at the beginning of an event (eg., before
107-
/// the executable is loaded). Otherwise, it was emitted at the end of an
108-
/// event (eg., after the module and any dependency were loaded.)
109-
bool is_start_entry;
110-
111-
/// Describes the exit of the executable module.
112-
std::optional<ExitDescription> exit_desc;
113-
TargetInfo() = default;
114-
115-
llvm::telemetry::KindType getKind() const override {
116-
return LLDBEntryKind::TargetInfo;
117-
}
118-
119-
static bool classof(const TelemetryInfo *T) {
120-
// Subclasses of this is also acceptable
121-
return (T->getKind() & LLDBEntryKind::TargetInfo) ==
122-
LLDBEntryKind::TargetInfo;
123-
}
124-
void serialize(llvm::telemetry::Serializer &serializer) const override;
125-
};
126-
12792
struct CommandInfo : public LLDBBaseTelemetryInfo {
12893
/// If the command is/can be associated with a target entry this field
12994
/// contains that target's UUID. <EMPTY> otherwise.
@@ -190,6 +155,58 @@ struct DebuggerInfo : public LLDBBaseTelemetryInfo {
190155
void serialize(llvm::telemetry::Serializer &serializer) const override;
191156
};
192157

158+
struct ExecModuleInfo : public LLDBBaseTelemetryInfo {
159+
lldb::ModuleSP exec_mod;
160+
161+
/// The same as the executable-module's UUID.
162+
UUID exec_uuid;
163+
/// PID of the process owned by this target.
164+
lldb::pid_t pid;
165+
std::string arch_name;
166+
167+
/// If true, this entry was emitted at the beginning of an event (eg., before
168+
/// the executable is set). Otherwise, it was emitted at the end of an
169+
/// event (eg., after the module and any dependency were loaded.)
170+
bool is_start_entry;
171+
172+
ExecModuleInfo() = default;
173+
174+
llvm::telemetry::KindType getKind() const override {
175+
return LLDBEntryKind::ExecModuleInfo;
176+
}
177+
178+
static bool classof(const TelemetryInfo *T) {
179+
// Subclasses of this is also acceptable
180+
return (T->getKind() & LLDBEntryKind::ExecModuleInfo) ==
181+
LLDBEntryKind::ExecModuleInfo;
182+
}
183+
void serialize(llvm::telemetry::Serializer &serializer) const override;
184+
};
185+
186+
/// Describes an exit status.
187+
struct ExitDescription {
188+
int exit_code;
189+
std::string description;
190+
};
191+
192+
struct ProcessExitInfo : public LLDBBaseTelemetryInfo {
193+
UUID exec_uuid;
194+
lldb::pid_t pid;
195+
bool is_start_entry;
196+
std::optional<ExitDescription> exit_desc;
197+
198+
llvm::telemetry::KindType getKind() const override {
199+
return LLDBEntryKind::ProcessExitInfo;
200+
}
201+
202+
static bool classof(const TelemetryInfo *T) {
203+
// Subclasses of this is also acceptable
204+
return (T->getKind() & LLDBEntryKind::ProcessExitInfo) ==
205+
LLDBEntryKind::ProcessExitInfo;
206+
}
207+
void serialize(llvm::telemetry::Serializer &serializer) const override;
208+
};
209+
193210
/// The base Telemetry manager instance in LLDB.
194211
/// This class declares additional instrumentation points
195212
/// applicable to LLDB.

lldb/include/lldb/Target/Target.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -544,6 +544,8 @@ class Target : public std::enable_shared_from_this<Target>,
544544
return GetStaticBroadcasterClass();
545545
}
546546

547+
UUID GetExecModuleUUID() const { return m_current_exec_mod_uuid; }
548+
547549
// This event data class is for use by the TargetList to broadcast new target
548550
// notifications.
549551
class TargetEventData : public EventData {
@@ -1640,6 +1642,8 @@ class Target : public std::enable_shared_from_this<Target>,
16401642
private:
16411643
// Target metrics storage.
16421644
TargetStats m_stats;
1645+
// The current executable module UUID.
1646+
UUID m_current_exec_mod_uuid;
16431647

16441648
public:
16451649
/// Get metrics associated with this target in JSON format.

lldb/source/Core/Telemetry.cpp

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -87,13 +87,21 @@ void DebuggerInfo::serialize(Serializer &serializer) const {
8787
serializer.write("is_exit_entry", is_exit_entry);
8888
}
8989

90-
void TargetInfo::serialize(Serializer &serializer) const {
90+
void ExecModuleInfo::serialize(Serializer &serializer) const {
9191
LLDBBaseTelemetryInfo::serialize(serializer);
9292

93-
serializer.write("target_uuid", target_uuid.GetAsString());
93+
serializer.write("exec_uuid", exec_uuid.GetAsString());
9494
serializer.write("pid", pid);
9595
serializer.write("arch_name", arch_name);
9696
serializer.write("is_start_entry", is_start_entry);
97+
}
98+
99+
void ProcessExitInfo::serialize(Serializer &serializer) const {
100+
LLDBBaseTelemetryInfo::serialize(serializer);
101+
102+
serializer.write("exec_uuid", exec_uuid.GetAsString());
103+
serializer.write("pid", pid);
104+
serializer.write("is_start_entry", is_start_entry);
97105
if (exit_desc.has_value()) {
98106
serializer.write("exit_code", exit_desc->exit_code);
99107
serializer.write("exit_desc", exit_desc->description);

lldb/source/Target/Process.cpp

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1065,25 +1065,29 @@ const char *Process::GetExitDescription() {
10651065
bool Process::SetExitStatus(int status, llvm::StringRef exit_string) {
10661066
// Use a mutex to protect setting the exit status.
10671067
std::lock_guard<std::mutex> guard(m_exit_status_mutex);
1068-
telemetry::ScopedDispatcher<telemetry::TargetInfo> helper;
1068+
telemetry::ScopedDispatcher<telemetry::ProcessExitInfo> helper;
10691069

1070-
// TODO: Find the executable-module's UUID somehow. (Maybe save the UUID in
1071-
// Target?) We might not have a valid executable-mod anymore.
1072-
helper.DispatchNow([&](telemetry::TargetInfo *info) {
1073-
// Check if there is (still) a valid target and get the debugger from it.
1074-
TargetSP target_sp(Debugger::FindTargetWithProcessID(m_pid));
1075-
if (target_sp)
1076-
helper.SetDebugger(&(target_sp->GetDebugger()));
1070+
// Find the executable-module's UUID, if available.
1071+
UUID exec_uuid;
1072+
// Check if there is (still) a valid target and get the debugger and exec_uuid
1073+
// from it.
1074+
TargetSP target_sp(Debugger::FindTargetWithProcessID(m_pid));
1075+
if (target_sp) {
1076+
helper.SetDebugger(&(target_sp->GetDebugger()));
1077+
if (ModuleSP exec_mod = target_sp->GetExecutableModule())
1078+
exec_uuid = exec_mod->GetUUID();
1079+
}
10771080

1078-
// info->target_uuid = target_uuid;
1081+
helper.DispatchNow([&](telemetry::ProcessExitInfo *info) {
1082+
info->exec_uuid = exec_uuid;
10791083
info->pid = m_pid;
10801084
info->is_start_entry = true;
1085+
info->exit_desc = {status, exit_string.str()};
10811086
});
10821087

1083-
helper.DispatchOnExit([&](telemetry::TargetInfo *info) {
1084-
// info->target_uuid = target_uuid;
1088+
helper.DispatchOnExit([&](telemetry::ProcessExitInfo *info) {
1089+
info->exec_uuid = exec_uuid;
10851090
info->pid = m_pid;
1086-
info->exit_desc = {status, exit_string.str()};
10871091
});
10881092

10891093
Log *log(GetLog(LLDBLog::State | LLDBLog::Process));

lldb/source/Target/Target.cpp

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1560,25 +1560,26 @@ void Target::DidExec() {
15601560

15611561
void Target::SetExecutableModule(ModuleSP &executable_sp,
15621562
LoadDependentFiles load_dependent_files) {
1563-
telemetry::ScopedDispatcher<telemetry::TargetInfo> helper(&m_debugger);
1563+
telemetry::ScopedDispatcher<telemetry::ExecModuleInfo> helper(&m_debugger);
15641564
Log *log = GetLog(LLDBLog::Target);
15651565
ClearModules(false);
15661566

15671567
if (executable_sp) {
1568+
m_current_exec_mod_uuid = executable_sp->GetUUID();
15681569
lldb::pid_t pid;
15691570
if (ProcessSP proc = GetProcessSP())
15701571
pid = proc->GetID();
1571-
helper.DispatchNow([&](telemetry::TargetInfo *info) {
1572+
helper.DispatchNow([&](telemetry::ExecModuleInfo *info) {
15721573
info->exec_mod = executable_sp;
1573-
info->target_uuid = executable_sp->GetUUID();
1574+
info->exec_uuid = m_current_exec_mod_uuid;
15741575
info->pid = pid;
15751576
info->arch_name = executable_sp->GetArchitecture().GetArchitectureName();
15761577
info->is_start_entry = true;
15771578
});
15781579

1579-
helper.DispatchOnExit([&](telemetry::TargetInfo *info) {
1580+
helper.DispatchOnExit([&](telemetry::ExecModuleInfo *info) {
15801581
info->exec_mod = executable_sp;
1581-
info->target_uuid = executable_sp->GetUUID();
1582+
info->exec_uuid = executable_sp->GetUUID();
15821583
info->pid = pid;
15831584
});
15841585

0 commit comments

Comments
 (0)