Skip to content

Commit 77f8335

Browse files
GeorgeHuyuboGeorge Hu
GeorgeHuyubo
and
George Hu
authored
Add symbol locator time for each module in statistics (#137379)
Identical PR to: #134563 Previous PR was approved and landed but broke the build due to bad merge. Manually resolve the merge conflict and try to land again. Co-authored-by: George Hu <[email protected]>
1 parent 3e4e365 commit 77f8335

File tree

16 files changed

+114
-36
lines changed

16 files changed

+114
-36
lines changed

‎lldb/include/lldb/Core/Module.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -885,6 +885,10 @@ class Module : public std::enable_shared_from_this<Module>,
885885
/// ElapsedTime RAII object.
886886
StatsDuration &GetSymtabIndexTime() { return m_symtab_index_time; }
887887

888+
StatisticsMap &GetSymbolLocatorStatistics() {
889+
return m_symbol_locator_duration_map;
890+
}
891+
888892
void ResetStatistics();
889893

890894
/// \class LookupInfo Module.h "lldb/Core/Module.h"
@@ -1064,6 +1068,8 @@ class Module : public std::enable_shared_from_this<Module>,
10641068
/// time for the symbol tables can be aggregated here.
10651069
StatsDuration m_symtab_index_time;
10661070

1071+
StatisticsMap m_symbol_locator_duration_map;
1072+
10671073
/// A set of hashes of all warnings and errors, to avoid reporting them
10681074
/// multiple times to the same Debugger.
10691075
llvm::DenseMap<llvm::stable_hash, std::unique_ptr<std::once_flag>>

‎lldb/include/lldb/Core/PluginManager.h

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
#include "lldb/Core/Architecture.h"
1313
#include "lldb/Interpreter/Interfaces/ScriptedInterfaceUsages.h"
1414
#include "lldb/Symbol/TypeSystem.h"
15+
#include "lldb/Target/Statistics.h"
1516
#include "lldb/Utility/CompletionRequest.h"
1617
#include "lldb/Utility/FileSpec.h"
1718
#include "lldb/Utility/Status.h"
@@ -379,11 +380,13 @@ class PluginManager {
379380
static SymbolLocatorCreateInstance
380381
GetSymbolLocatorCreateCallbackAtIndex(uint32_t idx);
381382

382-
static ModuleSpec LocateExecutableObjectFile(const ModuleSpec &module_spec);
383+
static ModuleSpec LocateExecutableObjectFile(const ModuleSpec &module_spec,
384+
StatisticsMap &map);
383385

384386
static FileSpec
385387
LocateExecutableSymbolFile(const ModuleSpec &module_spec,
386-
const FileSpecList &default_search_paths);
388+
const FileSpecList &default_search_paths,
389+
StatisticsMap &map);
387390

388391
static bool DownloadObjectAndSymbolFile(ModuleSpec &module_spec,
389392
Status &error,

‎lldb/include/lldb/Target/Statistics.h

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,26 @@ class ElapsedTime {
9090
}
9191
};
9292

93+
/// A class to count time for plugins
94+
class StatisticsMap {
95+
public:
96+
void add(llvm::StringRef key, double value) {
97+
if (key.empty())
98+
return;
99+
auto it = map.find(key);
100+
if (it == map.end())
101+
map.try_emplace(key, value);
102+
else
103+
it->second += value;
104+
}
105+
void merge(StatisticsMap map_to_merge) {
106+
for (const auto &entry : map_to_merge.map) {
107+
add(entry.first(), entry.second);
108+
}
109+
}
110+
llvm::StringMap<double> map;
111+
};
112+
93113
/// A class to count success/fail statistics.
94114
struct StatsSuccessFail {
95115
StatsSuccessFail(llvm::StringRef n) : name(n.str()) {}
@@ -118,6 +138,7 @@ struct ModuleStats {
118138
// track down all of the stats that contribute to this module.
119139
std::vector<intptr_t> symfile_modules;
120140
llvm::StringMap<llvm::json::Value> type_system_stats;
141+
StatisticsMap symbol_locator_time;
121142
double symtab_parse_time = 0.0;
122143
double symtab_index_time = 0.0;
123144
uint32_t symtab_symbol_count = 0;

‎lldb/source/Core/DynamicLoader.cpp

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -243,15 +243,22 @@ ModuleSP DynamicLoader::LoadBinaryWithUUIDAndAddress(
243243
// find an executable and symbol file.
244244
if (!module_sp) {
245245
FileSpecList search_paths = Target::GetDefaultDebugFileSearchPaths();
246+
StatisticsMap symbol_locator_map;
246247
module_spec.GetSymbolFileSpec() =
247-
PluginManager::LocateExecutableSymbolFile(module_spec, search_paths);
248+
PluginManager::LocateExecutableSymbolFile(module_spec, search_paths,
249+
symbol_locator_map);
248250
ModuleSpec objfile_module_spec =
249-
PluginManager::LocateExecutableObjectFile(module_spec);
251+
PluginManager::LocateExecutableObjectFile(module_spec,
252+
symbol_locator_map);
250253
module_spec.GetFileSpec() = objfile_module_spec.GetFileSpec();
251254
if (FileSystem::Instance().Exists(module_spec.GetFileSpec()) &&
252255
FileSystem::Instance().Exists(module_spec.GetSymbolFileSpec())) {
253256
module_sp = std::make_shared<Module>(module_spec);
254257
}
258+
259+
if (module_sp) {
260+
module_sp->GetSymbolLocatorStatistics().merge(symbol_locator_map);
261+
}
255262
}
256263

257264
// If we haven't found a binary, or we don't have a SymbolFile, see

‎lldb/source/Core/ModuleList.cpp

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -917,9 +917,10 @@ ModuleList::GetSharedModule(const ModuleSpec &module_spec, ModuleSP &module_sp,
917917

918918
// Fixup the incoming path in case the path points to a valid file, yet the
919919
// arch or UUID (if one was passed in) don't match.
920-
ModuleSpec located_binary_modulespec =
921-
PluginManager::LocateExecutableObjectFile(module_spec);
922-
920+
ModuleSpec located_binary_modulespec;
921+
StatisticsMap symbol_locator_map;
922+
located_binary_modulespec = PluginManager::LocateExecutableObjectFile(
923+
module_spec, symbol_locator_map);
923924
// Don't look for the file if it appears to be the same one we already
924925
// checked for above...
925926
if (located_binary_modulespec.GetFileSpec() != module_file_spec) {
@@ -992,6 +993,7 @@ ModuleList::GetSharedModule(const ModuleSpec &module_spec, ModuleSP &module_sp,
992993
// By getting the object file we can guarantee that the architecture
993994
// matches
994995
if (module_sp && module_sp->GetObjectFile()) {
996+
module_sp->GetSymbolLocatorStatistics().merge(symbol_locator_map);
995997
if (module_sp->GetObjectFile()->GetType() ==
996998
ObjectFile::eTypeStubLibrary) {
997999
module_sp.reset();

‎lldb/source/Core/PluginManager.cpp

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1218,12 +1218,18 @@ PluginManager::GetSymbolLocatorCreateCallbackAtIndex(uint32_t idx) {
12181218
}
12191219

12201220
ModuleSpec
1221-
PluginManager::LocateExecutableObjectFile(const ModuleSpec &module_spec) {
1221+
PluginManager::LocateExecutableObjectFile(const ModuleSpec &module_spec,
1222+
StatisticsMap &map) {
12221223
auto instances = GetSymbolLocatorInstances().GetSnapshot();
12231224
for (auto &instance : instances) {
12241225
if (instance.locate_executable_object_file) {
1225-
std::optional<ModuleSpec> result =
1226-
instance.locate_executable_object_file(module_spec);
1226+
StatsDuration time;
1227+
std::optional<ModuleSpec> result;
1228+
{
1229+
ElapsedTime elapsed(time);
1230+
result = instance.locate_executable_object_file(module_spec);
1231+
}
1232+
map.add(instance.name, time.get().count());
12271233
if (result)
12281234
return *result;
12291235
}
@@ -1232,12 +1238,19 @@ PluginManager::LocateExecutableObjectFile(const ModuleSpec &module_spec) {
12321238
}
12331239

12341240
FileSpec PluginManager::LocateExecutableSymbolFile(
1235-
const ModuleSpec &module_spec, const FileSpecList &default_search_paths) {
1241+
const ModuleSpec &module_spec, const FileSpecList &default_search_paths,
1242+
StatisticsMap &map) {
12361243
auto instances = GetSymbolLocatorInstances().GetSnapshot();
12371244
for (auto &instance : instances) {
12381245
if (instance.locate_executable_symbol_file) {
1239-
std::optional<FileSpec> result = instance.locate_executable_symbol_file(
1240-
module_spec, default_search_paths);
1246+
StatsDuration time;
1247+
std::optional<FileSpec> result;
1248+
{
1249+
ElapsedTime elapsed(time);
1250+
result = instance.locate_executable_symbol_file(module_spec,
1251+
default_search_paths);
1252+
}
1253+
map.add(instance.name, time.get().count());
12411254
if (result)
12421255
return *result;
12431256
}

‎lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -814,8 +814,8 @@ Status PlatformDarwinKernel::GetSharedModuleKernel(
814814
// append ".dSYM" to the filename for the SymbolFile.
815815
FileSpecList search_paths =
816816
process->GetTarget().GetDebugFileSearchPaths();
817-
FileSpec dsym_fspec =
818-
PluginManager::LocateExecutableSymbolFile(kern_spec, search_paths);
817+
FileSpec dsym_fspec = PluginManager::LocateExecutableSymbolFile(
818+
kern_spec, search_paths, module_sp->GetSymbolLocatorStatistics());
819819
if (FileSystem::Instance().Exists(dsym_fspec))
820820
module_sp->SetSymbolFileFileSpec(dsym_fspec);
821821
if (did_create_ptr)

‎lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -276,12 +276,15 @@ Status ProcessKDP::DoConnectRemote(llvm::StringRef remote_url) {
276276
// Lookup UUID locally, before attempting dsymForUUID like action
277277
FileSpecList search_paths =
278278
Target::GetDefaultDebugFileSearchPaths();
279+
280+
StatisticsMap symbol_locator_map;
279281
module_spec.GetSymbolFileSpec() =
280-
PluginManager::LocateExecutableSymbolFile(module_spec,
281-
search_paths);
282+
PluginManager::LocateExecutableSymbolFile(
283+
module_spec, search_paths, symbol_locator_map);
282284
if (module_spec.GetSymbolFileSpec()) {
283285
ModuleSpec executable_module_spec =
284-
PluginManager::LocateExecutableObjectFile(module_spec);
286+
PluginManager::LocateExecutableObjectFile(
287+
module_spec, symbol_locator_map);
285288
if (FileSystem::Instance().Exists(
286289
executable_module_spec.GetFileSpec())) {
287290
module_spec.GetFileSpec() =
@@ -297,6 +300,8 @@ Status ProcessKDP::DoConnectRemote(llvm::StringRef remote_url) {
297300

298301
if (FileSystem::Instance().Exists(module_spec.GetFileSpec())) {
299302
ModuleSP module_sp(new Module(module_spec));
303+
module_sp->GetSymbolLocatorStatistics().merge(
304+
symbol_locator_map);
300305
if (module_sp.get() && module_sp->GetObjectFile()) {
301306
// Get the current target executable
302307
ModuleSP exe_module_sp(target.GetExecutableModule());

‎lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4245,8 +4245,9 @@ const std::shared_ptr<SymbolFileDWARFDwo> &SymbolFileDWARF::GetDwpSymbolFile() {
42454245
FileSpec(symfile.GetPath() + ".dwp", symfile.GetPathStyle());
42464246
LLDB_LOG(log, "Searching for DWP using: \"{0}\"",
42474247
module_spec.GetSymbolFileSpec());
4248-
dwp_filespec =
4249-
PluginManager::LocateExecutableSymbolFile(module_spec, search_paths);
4248+
dwp_filespec = PluginManager::LocateExecutableSymbolFile(
4249+
module_spec, search_paths,
4250+
m_objfile_sp->GetModule()->GetSymbolLocatorStatistics());
42504251
if (FileSystem::Instance().Exists(dwp_filespec)) {
42514252
break;
42524253
}
@@ -4257,8 +4258,9 @@ const std::shared_ptr<SymbolFileDWARFDwo> &SymbolFileDWARF::GetDwpSymbolFile() {
42574258
// find the correct DWP file, as the Debuginfod plugin uses *only* this
42584259
// data to correctly match the DWP file with the binary.
42594260
module_spec.GetUUID() = m_objfile_sp->GetUUID();
4260-
dwp_filespec =
4261-
PluginManager::LocateExecutableSymbolFile(module_spec, search_paths);
4261+
dwp_filespec = PluginManager::LocateExecutableSymbolFile(
4262+
module_spec, search_paths,
4263+
m_objfile_sp->GetModule()->GetSymbolLocatorStatistics());
42624264
}
42634265
if (FileSystem::Instance().Exists(dwp_filespec)) {
42644266
LLDB_LOG(log, "Found DWP file: \"{0}\"", dwp_filespec);

‎lldb/source/Plugins/SymbolVendor/ELF/SymbolVendorELF.cpp

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -103,14 +103,14 @@ SymbolVendorELF::CreateInstance(const lldb::ModuleSP &module_sp,
103103
module_spec.GetSymbolFileSpec() = fspec;
104104
module_spec.GetUUID() = uuid;
105105
FileSpecList search_paths = Target::GetDefaultDebugFileSearchPaths();
106-
FileSpec dsym_fspec =
107-
PluginManager::LocateExecutableSymbolFile(module_spec, search_paths);
106+
FileSpec dsym_fspec = PluginManager::LocateExecutableSymbolFile(
107+
module_spec, search_paths, module_sp->GetSymbolLocatorStatistics());
108108
if (!dsym_fspec || IsDwpSymbolFile(module_sp, dsym_fspec)) {
109109
// If we have a stripped binary or if we have a DWP file, SymbolLocator
110110
// plugins may be able to give us an unstripped binary or an
111111
// 'only-keep-debug' stripped file.
112-
ModuleSpec unstripped_spec =
113-
PluginManager::LocateExecutableObjectFile(module_spec);
112+
ModuleSpec unstripped_spec = PluginManager::LocateExecutableObjectFile(
113+
module_spec, module_sp->GetSymbolLocatorStatistics());
114114
if (!unstripped_spec)
115115
return nullptr;
116116
// The default SymbolLocator plugin returns the original binary if no other
@@ -127,7 +127,6 @@ SymbolVendorELF::CreateInstance(const lldb::ModuleSP &module_sp,
127127
dsym_file_data_sp, dsym_file_data_offset);
128128
if (!dsym_objfile_sp)
129129
return nullptr;
130-
131130
// This objfile is for debugging purposes. Sadly, ObjectFileELF won't
132131
// be able to figure this out consistently as the symbol file may not
133132
// have stripped the code sections, etc.

‎lldb/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -134,8 +134,9 @@ SymbolVendorMacOSX::CreateInstance(const lldb::ModuleSP &module_sp,
134134
ModuleSpec module_spec(file_spec, module_sp->GetArchitecture());
135135
module_spec.GetUUID() = module_sp->GetUUID();
136136
FileSpecList search_paths = Target::GetDefaultDebugFileSearchPaths();
137-
dsym_fspec =
138-
PluginManager::LocateExecutableSymbolFile(module_spec, search_paths);
137+
138+
dsym_fspec = PluginManager::LocateExecutableSymbolFile(
139+
module_spec, search_paths, module_sp->GetSymbolLocatorStatistics());
139140
if (module_spec.GetSourceMappingList().GetSize())
140141
module_sp->GetSourceMappingList().Append(
141142
module_spec.GetSourceMappingList(), true);

‎lldb/source/Plugins/SymbolVendor/PECOFF/SymbolVendorPECOFF.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,8 +85,8 @@ SymbolVendorPECOFF::CreateInstance(const lldb::ModuleSP &module_sp,
8585
module_spec.GetSymbolFileSpec() = fspec;
8686
module_spec.GetUUID() = uuid;
8787
FileSpecList search_paths = Target::GetDefaultDebugFileSearchPaths();
88-
FileSpec dsym_fspec =
89-
PluginManager::LocateExecutableSymbolFile(module_spec, search_paths);
88+
FileSpec dsym_fspec = PluginManager::LocateExecutableSymbolFile(
89+
module_spec, search_paths, module_sp->GetSymbolLocatorStatistics());
9090
if (!dsym_fspec)
9191
return nullptr;
9292

‎lldb/source/Plugins/SymbolVendor/wasm/SymbolVendorWasm.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,8 +85,8 @@ SymbolVendorWasm::CreateInstance(const lldb::ModuleSP &module_sp,
8585
module_spec.GetSymbolFileSpec() = *symbol_file_spec;
8686

8787
FileSpecList search_paths = Target::GetDefaultDebugFileSearchPaths();
88-
FileSpec sym_fspec =
89-
PluginManager::LocateExecutableSymbolFile(module_spec, search_paths);
88+
FileSpec sym_fspec = PluginManager::LocateExecutableSymbolFile(
89+
module_spec, search_paths, module_sp->GetSymbolLocatorStatistics());
9090
if (!sym_fspec)
9191
return nullptr;
9292

‎lldb/source/Target/Statistics.cpp

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,14 @@ json::Value ModuleStats::ToJSON() const {
7272
debug_info_had_incomplete_types);
7373
module.try_emplace("symbolTableStripped", symtab_stripped);
7474
module.try_emplace("symbolTableSymbolCount", symtab_symbol_count);
75+
76+
if (!symbol_locator_time.map.empty()) {
77+
json::Object obj;
78+
for (const auto &entry : symbol_locator_time.map)
79+
obj.try_emplace(entry.first().str(), entry.second);
80+
module.try_emplace("symbolLocatorTime", std::move(obj));
81+
}
82+
7583
if (!symfile_path.empty())
7684
module.try_emplace("symbolFilePath", symfile_path);
7785

@@ -289,6 +297,7 @@ llvm::json::Value DebuggerStats::ReportStatistics(
289297

290298
json::Array json_targets;
291299
json::Array json_modules;
300+
StatisticsMap symbol_locator_total_time;
292301
double symtab_parse_time = 0.0;
293302
double symtab_index_time = 0.0;
294303
double debug_parse_time = 0.0;
@@ -319,6 +328,8 @@ llvm::json::Value DebuggerStats::ReportStatistics(
319328
ModuleStats module_stat;
320329
module_stat.symtab_parse_time = module->GetSymtabParseTime().get().count();
321330
module_stat.symtab_index_time = module->GetSymtabIndexTime().get().count();
331+
module_stat.symbol_locator_time = module->GetSymbolLocatorStatistics();
332+
symbol_locator_total_time.merge(module_stat.symbol_locator_time);
322333
Symtab *symtab = module->GetSymtab(/*can_create=*/false);
323334
if (symtab) {
324335
module_stat.symtab_symbol_count = symtab->GetNumSymbols();
@@ -427,6 +438,13 @@ llvm::json::Value DebuggerStats::ReportStatistics(
427438
global_stats.try_emplace("targets", std::move(json_targets));
428439
}
429440

441+
if (!symbol_locator_total_time.map.empty()) {
442+
json::Object obj;
443+
for (const auto &entry : symbol_locator_total_time.map)
444+
obj.try_emplace(entry.first().str(), entry.second);
445+
global_stats.try_emplace("totalSymbolLocatorTime", std::move(obj));
446+
}
447+
430448
ConstStringStats const_string_stats;
431449
json::Object json_memory{
432450
{"strings", const_string_stats.ToJSON()},

‎lldb/test/Shell/Commands/command-statistics-dump.test

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323
# CHECK: "modules": [
2424
# CHECK: {
2525
# CHECK: "path": {{.*}}-main.exe
26-
# CHECK-NOT: }
2726

2827
# PRELOAD_TRUE: "symbolTableParseTime":
2928
# PRELOAD_TRUE-SAME: {{[1-9]+}}

‎lldb/unittests/Symbol/LocateSymbolFileTest.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,9 @@ TEST_F(
2929
TerminateLocateExecutableSymbolFileForUnknownExecutableAndUnknownSymbolFile) {
3030
ModuleSpec module_spec;
3131
FileSpecList search_paths = Target::GetDefaultDebugFileSearchPaths();
32+
StatisticsMap map;
3233
FileSpec symbol_file_spec =
33-
PluginManager::LocateExecutableSymbolFile(module_spec, search_paths);
34+
PluginManager::LocateExecutableSymbolFile(module_spec, search_paths, map);
3435
EXPECT_TRUE(symbol_file_spec.GetFilename().IsEmpty());
3536
}
3637

@@ -41,7 +42,8 @@ TEST_F(SymbolsTest,
4142
module_spec.GetSymbolFileSpec().SetFile(
4243
"4A524676-B24B-4F4E-968A-551D465EBAF1.so", FileSpec::Style::native);
4344
FileSpecList search_paths = Target::GetDefaultDebugFileSearchPaths();
45+
StatisticsMap map;
4446
FileSpec symbol_file_spec =
45-
PluginManager::LocateExecutableSymbolFile(module_spec, search_paths);
47+
PluginManager::LocateExecutableSymbolFile(module_spec, search_paths, map);
4648
EXPECT_TRUE(symbol_file_spec.GetFilename().IsEmpty());
4749
}

0 commit comments

Comments
 (0)