-
Notifications
You must be signed in to change notification settings - Fork 13.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[lldb] Emit progress events in SymbolFileDWARFDebugMap #133211
[lldb] Emit progress events in SymbolFileDWARFDebugMap #133211
Conversation
@llvm/pr-subscribers-lldb Author: Jonas Devlieghere (JDevlieghere) ChangesEmit progress events from SymbolFileDWARFDebugMap. Because we know the number of OSOs, we can show determinate progress. This is based on a patch from Adrian, and part of what prompted me to look into improving how LLDB shows progress events. Before the statusline, all these progress events would get shadowed and never displayed on the command line. Full diff: https://github.com/llvm/llvm-project/pull/133211.diff 2 Files Affected:
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
index 0ecf47a3c7869..f529a76f65c1f 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
@@ -14,6 +14,7 @@
#include "lldb/Core/Module.h"
#include "lldb/Core/ModuleList.h"
#include "lldb/Core/PluginManager.h"
+#include "lldb/Core/Progress.h"
#include "lldb/Core/Section.h"
#include "lldb/Host/FileSystem.h"
#include "lldb/Utility/RangeMap.h"
@@ -31,6 +32,7 @@
#include "lldb/Symbol/TypeMap.h"
#include "lldb/Symbol/VariableList.h"
#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/StringRef.h"
#include "llvm/Support/ScopedPrinter.h"
#include "lldb/Target/StackFrame.h"
@@ -716,6 +718,27 @@ bool SymbolFileDWARFDebugMap::ParseDebugMacros(CompileUnit &comp_unit) {
return false;
}
+void SymbolFileDWARFDebugMap::ForEachSymbolFile(
+ std::string description,
+ std::function<IterationAction(SymbolFileDWARF *)> closure) {
+ const size_t num_oso_idxs = m_compile_unit_infos.size();
+ const size_t update_rate = std::max<size_t>(1, num_oso_idxs / 100);
+ Progress progress(std::move(description), "", num_oso_idxs);
+ for (uint32_t oso_idx = 0; oso_idx < num_oso_idxs; ++oso_idx) {
+ if (SymbolFileDWARF *oso_dwarf = GetSymbolFileByOSOIndex(oso_idx)) {
+ if (oso_idx % update_rate == 0)
+ progress.Increment(oso_idx, oso_dwarf->GetObjectFile()
+ ? oso_dwarf->GetObjectFile()
+ ->GetFileSpec()
+ .GetFilename()
+ .GetString()
+ : std::string());
+ if (closure(oso_dwarf) == IterationAction::Stop)
+ return;
+ }
+ }
+}
+
bool SymbolFileDWARFDebugMap::ForEachExternalModule(
CompileUnit &comp_unit,
llvm::DenseSet<lldb_private::SymbolFile *> &visited_symbol_files,
@@ -804,7 +827,7 @@ SymbolFileDWARFDebugMap::GetDynamicArrayInfoForUID(
bool SymbolFileDWARFDebugMap::CompleteType(CompilerType &compiler_type) {
bool success = false;
if (compiler_type) {
- ForEachSymbolFile([&](SymbolFileDWARF *oso_dwarf) {
+ ForEachSymbolFile("Completing type", [&](SymbolFileDWARF *oso_dwarf) {
if (oso_dwarf->HasForwardDeclForCompilerType(compiler_type)) {
oso_dwarf->CompleteType(compiler_type);
success = true;
@@ -924,29 +947,31 @@ void SymbolFileDWARFDebugMap::FindGlobalVariables(
std::lock_guard<std::recursive_mutex> guard(GetModuleMutex());
uint32_t total_matches = 0;
- ForEachSymbolFile([&](SymbolFileDWARF *oso_dwarf) {
- const uint32_t old_size = variables.GetSize();
- oso_dwarf->FindGlobalVariables(name, parent_decl_ctx, max_matches,
- variables);
- const uint32_t oso_matches = variables.GetSize() - old_size;
- if (oso_matches > 0) {
- total_matches += oso_matches;
-
- // Are we getting all matches?
- if (max_matches == UINT32_MAX)
- return IterationAction::Continue; // Yep, continue getting everything
-
- // If we have found enough matches, lets get out
- if (max_matches >= total_matches)
- return IterationAction::Stop;
-
- // Update the max matches for any subsequent calls to find globals in any
- // other object files with DWARF
- max_matches -= oso_matches;
- }
+ ForEachSymbolFile(
+ "Looking up global variables", [&](SymbolFileDWARF *oso_dwarf) {
+ const uint32_t old_size = variables.GetSize();
+ oso_dwarf->FindGlobalVariables(name, parent_decl_ctx, max_matches,
+ variables);
+ const uint32_t oso_matches = variables.GetSize() - old_size;
+ if (oso_matches > 0) {
+ total_matches += oso_matches;
+
+ // Are we getting all matches?
+ if (max_matches == UINT32_MAX)
+ return IterationAction::Continue; // Yep, continue getting
+ // everything
+
+ // If we have found enough matches, lets get out
+ if (max_matches >= total_matches)
+ return IterationAction::Stop;
+
+ // Update the max matches for any subsequent calls to find globals in
+ // any other object files with DWARF
+ max_matches -= oso_matches;
+ }
- return IterationAction::Continue;
- });
+ return IterationAction::Continue;
+ });
}
void SymbolFileDWARFDebugMap::FindGlobalVariables(
@@ -954,29 +979,31 @@ void SymbolFileDWARFDebugMap::FindGlobalVariables(
VariableList &variables) {
std::lock_guard<std::recursive_mutex> guard(GetModuleMutex());
uint32_t total_matches = 0;
- ForEachSymbolFile([&](SymbolFileDWARF *oso_dwarf) {
- const uint32_t old_size = variables.GetSize();
- oso_dwarf->FindGlobalVariables(regex, max_matches, variables);
-
- const uint32_t oso_matches = variables.GetSize() - old_size;
- if (oso_matches > 0) {
- total_matches += oso_matches;
-
- // Are we getting all matches?
- if (max_matches == UINT32_MAX)
- return IterationAction::Continue; // Yep, continue getting everything
-
- // If we have found enough matches, lets get out
- if (max_matches >= total_matches)
- return IterationAction::Stop;
-
- // Update the max matches for any subsequent calls to find globals in any
- // other object files with DWARF
- max_matches -= oso_matches;
- }
+ ForEachSymbolFile(
+ "Looking up global variables", [&](SymbolFileDWARF *oso_dwarf) {
+ const uint32_t old_size = variables.GetSize();
+ oso_dwarf->FindGlobalVariables(regex, max_matches, variables);
+
+ const uint32_t oso_matches = variables.GetSize() - old_size;
+ if (oso_matches > 0) {
+ total_matches += oso_matches;
+
+ // Are we getting all matches?
+ if (max_matches == UINT32_MAX)
+ return IterationAction::Continue; // Yep, continue getting
+ // everything
+
+ // If we have found enough matches, lets get out
+ if (max_matches >= total_matches)
+ return IterationAction::Stop;
+
+ // Update the max matches for any subsequent calls to find globals in
+ // any other object files with DWARF
+ max_matches -= oso_matches;
+ }
- return IterationAction::Continue;
- });
+ return IterationAction::Continue;
+ });
}
int SymbolFileDWARFDebugMap::SymbolContainsSymbolWithIndex(
@@ -1079,7 +1106,7 @@ void SymbolFileDWARFDebugMap::FindFunctions(
LLDB_SCOPED_TIMERF("SymbolFileDWARFDebugMap::FindFunctions (name = %s)",
lookup_info.GetLookupName().GetCString());
- ForEachSymbolFile([&](SymbolFileDWARF *oso_dwarf) {
+ ForEachSymbolFile("Looking up functions", [&](SymbolFileDWARF *oso_dwarf) {
uint32_t sc_idx = sc_list.GetSize();
oso_dwarf->FindFunctions(lookup_info, parent_decl_ctx, include_inlines,
sc_list);
@@ -1098,7 +1125,7 @@ void SymbolFileDWARFDebugMap::FindFunctions(const RegularExpression ®ex,
LLDB_SCOPED_TIMERF("SymbolFileDWARFDebugMap::FindFunctions (regex = '%s')",
regex.GetText().str().c_str());
- ForEachSymbolFile([&](SymbolFileDWARF *oso_dwarf) {
+ ForEachSymbolFile("Looking up functions", [&](SymbolFileDWARF *oso_dwarf) {
uint32_t sc_idx = sc_list.GetSize();
oso_dwarf->FindFunctions(regex, include_inlines, sc_list);
@@ -1129,7 +1156,7 @@ void SymbolFileDWARFDebugMap::GetTypes(SymbolContextScope *sc_scope,
oso_dwarf->GetTypes(sc_scope, type_mask, type_list);
}
} else {
- ForEachSymbolFile([&](SymbolFileDWARF *oso_dwarf) {
+ ForEachSymbolFile("Looking up types", [&](SymbolFileDWARF *oso_dwarf) {
oso_dwarf->GetTypes(sc_scope, type_mask, type_list);
return IterationAction::Continue;
});
@@ -1148,16 +1175,16 @@ SymbolFileDWARFDebugMap::ParseCallEdgesInFunction(
DWARFDIE SymbolFileDWARFDebugMap::FindDefinitionDIE(const DWARFDIE &die) {
DWARFDIE result;
- ForEachSymbolFile([&](SymbolFileDWARF *oso_dwarf) {
- result = oso_dwarf->FindDefinitionDIE(die);
- return result ? IterationAction::Stop : IterationAction::Continue;
- });
+ ForEachSymbolFile(
+ "Looking up type definition", [&](SymbolFileDWARF *oso_dwarf) {
+ result = oso_dwarf->FindDefinitionDIE(die);
+ return result ? IterationAction::Stop : IterationAction::Continue;
+ });
return result;
}
TypeSP SymbolFileDWARFDebugMap::FindCompleteObjCDefinitionTypeForDIE(
- const DWARFDIE &die, ConstString type_name,
- bool must_be_implementation) {
+ const DWARFDIE &die, ConstString type_name, bool must_be_implementation) {
// If we have a debug map, we will have an Objective-C symbol whose name is
// the type name and whose type is eSymbolTypeObjCClass. If we can find that
// symbol and find its containing parent, we can locate the .o file that will
@@ -1208,11 +1235,12 @@ TypeSP SymbolFileDWARFDebugMap::FindCompleteObjCDefinitionTypeForDIE(
if (!must_be_implementation) {
TypeSP type_sp;
- ForEachSymbolFile([&](SymbolFileDWARF *oso_dwarf) {
- type_sp = oso_dwarf->FindCompleteObjCDefinitionTypeForDIE(
- die, type_name, must_be_implementation);
- return type_sp ? IterationAction::Stop : IterationAction::Continue;
- });
+ ForEachSymbolFile(
+ "Looking up Objective-C definition", [&](SymbolFileDWARF *oso_dwarf) {
+ type_sp = oso_dwarf->FindCompleteObjCDefinitionTypeForDIE(
+ die, type_name, must_be_implementation);
+ return type_sp ? IterationAction::Stop : IterationAction::Continue;
+ });
return type_sp;
}
@@ -1222,7 +1250,7 @@ TypeSP SymbolFileDWARFDebugMap::FindCompleteObjCDefinitionTypeForDIE(
void SymbolFileDWARFDebugMap::FindTypes(const TypeQuery &query,
TypeResults &results) {
std::lock_guard<std::recursive_mutex> guard(GetModuleMutex());
- ForEachSymbolFile([&](SymbolFileDWARF *oso_dwarf) {
+ ForEachSymbolFile("Looking up type", [&](SymbolFileDWARF *oso_dwarf) {
oso_dwarf->FindTypes(query, results);
return results.Done(query) ? IterationAction::Stop
: IterationAction::Continue;
@@ -1235,7 +1263,7 @@ CompilerDeclContext SymbolFileDWARFDebugMap::FindNamespace(
std::lock_guard<std::recursive_mutex> guard(GetModuleMutex());
CompilerDeclContext matching_namespace;
- ForEachSymbolFile([&](SymbolFileDWARF *oso_dwarf) {
+ ForEachSymbolFile("Looking up namespace", [&](SymbolFileDWARF *oso_dwarf) {
matching_namespace =
oso_dwarf->FindNamespace(name, parent_decl_ctx, only_root_namespaces);
@@ -1247,7 +1275,7 @@ CompilerDeclContext SymbolFileDWARFDebugMap::FindNamespace(
}
void SymbolFileDWARFDebugMap::DumpClangAST(Stream &s) {
- ForEachSymbolFile([&s](SymbolFileDWARF *oso_dwarf) {
+ ForEachSymbolFile("Dumping clang AST", [&s](SymbolFileDWARF *oso_dwarf) {
oso_dwarf->DumpClangAST(s);
// The underlying assumption is that DumpClangAST(...) will obtain the
// AST from the underlying TypeSystem and therefore we only need to do
@@ -1294,7 +1322,8 @@ bool SymbolFileDWARFDebugMap::GetSeparateDebugInfo(
}
lldb::CompUnitSP
-SymbolFileDWARFDebugMap::GetCompileUnit(SymbolFileDWARF *oso_dwarf, DWARFCompileUnit &dwarf_cu) {
+SymbolFileDWARFDebugMap::GetCompileUnit(SymbolFileDWARF *oso_dwarf,
+ DWARFCompileUnit &dwarf_cu) {
if (oso_dwarf) {
const uint32_t cu_count = GetNumCompileUnits();
for (uint32_t cu_idx = 0; cu_idx < cu_count; ++cu_idx) {
@@ -1344,7 +1373,8 @@ void SymbolFileDWARFDebugMap::SetCompileUnit(SymbolFileDWARF *oso_dwarf,
} else {
assert(cu_sp->GetID() == 0 &&
"Setting first compile unit but with id different than 0!");
- auto &compile_units_sps = m_compile_unit_infos[cu_idx].compile_units_sps;
+ auto &compile_units_sps =
+ m_compile_unit_infos[cu_idx].compile_units_sps;
compile_units_sps.push_back(cu_sp);
m_compile_unit_infos[cu_idx].id_to_index_map.insert(
{cu_sp->GetID(), compile_units_sps.size() - 1});
@@ -1382,7 +1412,7 @@ SymbolFileDWARFDebugMap::GetCompilerContextForUID(lldb::user_id_t type_uid) {
void SymbolFileDWARFDebugMap::ParseDeclsForContext(
lldb_private::CompilerDeclContext decl_ctx) {
- ForEachSymbolFile([&](SymbolFileDWARF *oso_dwarf) {
+ ForEachSymbolFile("Parsing declarations", [&](SymbolFileDWARF *oso_dwarf) {
oso_dwarf->ParseDeclsForContext(decl_ctx);
return IterationAction::Continue;
});
@@ -1512,7 +1542,7 @@ SymbolFileDWARFDebugMap::AddOSOARanges(SymbolFileDWARF *dwarf2Data,
ModuleList SymbolFileDWARFDebugMap::GetDebugInfoModules() {
ModuleList oso_modules;
- ForEachSymbolFile([&](SymbolFileDWARF *oso_dwarf) {
+ ForEachSymbolFile("Parsing modules", [&](SymbolFileDWARF *oso_dwarf) {
ObjectFile *oso_objfile = oso_dwarf->GetObjectFile();
if (oso_objfile) {
ModuleSP module_sp = oso_objfile->GetModule();
@@ -1573,7 +1603,7 @@ Status SymbolFileDWARFDebugMap::CalculateFrameVariableError(StackFrame &frame) {
void SymbolFileDWARFDebugMap::GetCompileOptions(
std::unordered_map<lldb::CompUnitSP, lldb_private::Args> &args) {
- ForEachSymbolFile([&](SymbolFileDWARF *oso_dwarf) {
+ ForEachSymbolFile("Parsing compile options", [&](SymbolFileDWARF *oso_dwarf) {
oso_dwarf->GetCompileOptions(args);
return IterationAction::Continue;
});
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
index df41d6a2a4e42..fb9af8f3ccb79 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
@@ -237,15 +237,8 @@ class SymbolFileDWARFDebugMap : public SymbolFileCommon {
/// If closure returns \ref IterationAction::Continue, iteration
/// continues. Otherwise, iteration terminates.
void
- ForEachSymbolFile(std::function<IterationAction(SymbolFileDWARF *)> closure) {
- for (uint32_t oso_idx = 0, num_oso_idxs = m_compile_unit_infos.size();
- oso_idx < num_oso_idxs; ++oso_idx) {
- if (SymbolFileDWARF *oso_dwarf = GetSymbolFileByOSOIndex(oso_idx)) {
- if (closure(oso_dwarf) == IterationAction::Stop)
- return;
- }
- }
- }
+ ForEachSymbolFile(std::string description,
+ std::function<IterationAction(SymbolFileDWARF *)> closure);
CompileUnitInfo *GetCompileUnitInfoForSymbolWithIndex(uint32_t symbol_idx,
uint32_t *oso_idx_ptr);
|
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
Outdated
Show resolved
Hide resolved
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
Outdated
Show resolved
Hide resolved
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will be great when debugging LLDB itself!
Emit progress events from SymbolFileDWARFDebugMap. Because we know the number of OSOs, we can show determinate progress. This is based on a patch from Adrian, and what prompted me to look into improving how LLDB shows progress events. Before the statusline, all these progress events would get shadowed.
5dc6392
to
1de35b6
Compare
progress.Increment(oso_idx, oso_dwarf->GetObjectFile() | ||
? oso_dwarf->GetObjectFile() | ||
->GetFileSpec() | ||
.GetFilename() | ||
.GetString() | ||
: ""); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this just show a path to the .a file for all .o files within a .a file? We might want to check the object name in the lldb_private::Module:
ConstString object_name = oso_dwarf->GetModule().GetObjectName();
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right. I created #133370
const size_t num_oso_idxs = m_compile_unit_infos.size(); | ||
Progress progress(std::move(description), "", num_oso_idxs, | ||
/*debugger=*/nullptr, | ||
/*minimum_report_time=*/std::chrono::milliseconds(20)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might be nice to have a setting in lldb for the time between notifcations. 500 per second is a bit high IMHO, but it would be nice to have a setting and also use it in lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp in a follow up patch?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I used 20ms because that's what Pavel used in the manual DWARF index. That's 50 times per second, which is still fast but not as bad as 500. That said, having a setting that both these things query doesn't sound like a bad idea. It could even be the default for all progress events. WDYT?
…ss events In llvm#133211, Greg pointed out that the old code would print the static archive (the .a file) rather than the actual object file inside of it.
In llvm#133211, Greg suggested making the rate limit configurable through a setting. Although adding the setting is easy, the two places where we currently use rate limiting aren't tied to a particular debugger. I still think it's a good idea to be consistent and make it easy to pick the same rate limiting value, so I've moved it into a constant in the Progress class.
In #133211, Greg suggested making the rate limit configurable through a setting. Although adding the setting is easy, the two places where we currently use rate limiting aren't tied to a particular debugger. Although it'd be possible to hook up, given how few progress events currently implement rate limiting, I don't think it's worth threading this through, if that's even possible. I still think it's a good idea to be consistent and make it easy to pick the same rate limiting value, so I've moved it into a constant in the Progress class.
Add ObjectFile::GetObjectName and SymbolFile::GetObjectName to retrieve the name of the object file, including the `.a` for static libraries. We currently do something similar in CommandObjectTarget, but the code for dumping this is a lot more involved than what's being offered by the new method. We have options to print he full path, the base name, and the directoy of the path and trim it to a specific width. This is motivated by #133211, where Greg pointed out that the old code would print the static archive (the .a file) rather than the actual object file inside of it.
Emit progress events from SymbolFileDWARFDebugMap. Because we know the number of OSOs, we can show determinate progress. This is based on a patch from Adrian, and part of what prompted me to look into improving how LLDB shows progress events. Before the statusline, all these progress events would get shadowed and never displayed on the command line.