Skip to content

Commit

Permalink
Improve namespace lookup using .debug_names parent chain (llvm#110062)
Browse files Browse the repository at this point in the history
## Summary
This PR is a continuation of
llvm#108907 by using `.debug_names`
parent chain faster lookup for namespaces.


## Implementation
Similar to llvm#108907. This PR
adds a new API: `GetNamespacesWithParents` in `DWARFIndex` base class.
The API performs the same function as `GetNamespaces()` with additional
filtering using parents `CompilerDeclContext`. A default implementation
is given in `DWARFIndex` class which parses debug info and performs the
matching. In the `DebugNameDWARFIndex` override, parents
`CompilerDeclContext` is cross checked with parent chain in
`.debug_names` for much faster filtering before fallback to base
implementation for final filtering.

## Performance Results
For the same benchmark used in
llvm#108907, this PR improves: 48s
=> 28s

---------

Co-authored-by: jeffreytan81 <jeffreytan@fb.com>
  • Loading branch information
jeffreytan81 and jeffreytan81 authored Oct 30, 2024
1 parent 0d94c7b commit cb04d33
Show file tree
Hide file tree
Showing 5 changed files with 85 additions and 12 deletions.
16 changes: 16 additions & 0 deletions lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -151,3 +151,19 @@ bool DWARFIndex::ProcessTypeDIEMatchQuery(
return true;
return callback(die);
}

void DWARFIndex::GetNamespacesWithParents(
ConstString name, const CompilerDeclContext &parent_decl_ctx,
llvm::function_ref<bool(DWARFDIE die)> callback) {
GetNamespaces(name, [&](DWARFDIE die) {
return ProcessNamespaceDieMatchParents(parent_decl_ctx, die, callback);
});
}

bool DWARFIndex::ProcessNamespaceDieMatchParents(
const CompilerDeclContext &parent_decl_ctx, DWARFDIE die,
llvm::function_ref<bool(DWARFDIE die)> callback) {
if (!SymbolFileDWARF::DIEInDeclContext(parent_decl_ctx, die))
return true;
return callback(die);
}
11 changes: 11 additions & 0 deletions lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,14 @@ class DWARFIndex {
virtual void
GetTypesWithQuery(TypeQuery &query,
llvm::function_ref<bool(DWARFDIE die)> callback);
/// Get namespace DIEs whose base name match \param name with \param
/// parent_decl_ctx in its decl parent chain. A base implementation
/// is provided. Specializations should override this if they are able to
/// provide a faster implementation.
virtual void
GetNamespacesWithParents(ConstString name,
const CompilerDeclContext &parent_decl_ctx,
llvm::function_ref<bool(DWARFDIE die)> callback);
virtual void
GetFunctions(const Module::LookupInfo &lookup_info, SymbolFileDWARF &dwarf,
const CompilerDeclContext &parent_decl_ctx,
Expand Down Expand Up @@ -127,6 +135,9 @@ class DWARFIndex {
bool
ProcessTypeDIEMatchQuery(TypeQuery &query, DWARFDIE die,
llvm::function_ref<bool(DWARFDIE die)> callback);
bool ProcessNamespaceDieMatchParents(
const CompilerDeclContext &parent_decl_ctx, DWARFDIE die,
llvm::function_ref<bool(DWARFDIE die)> callback);
};
} // namespace dwarf
} // namespace lldb_private::plugin
Expand Down
64 changes: 54 additions & 10 deletions lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -368,9 +368,10 @@ void DebugNamesDWARFIndex::GetFullyQualifiedType(
continue;
}

if (SameParentChain(parent_names, *parent_chain) &&
!ProcessEntry(entry, callback))
return;
if (SameParentChain(parent_names, *parent_chain)) {
if (!ProcessEntry(entry, callback))
return;
}
}
m_fallback.GetFullyQualifiedType(context, callback);
}
Expand Down Expand Up @@ -554,17 +555,60 @@ void DebugNamesDWARFIndex::GetTypesWithQuery(
continue;
}

if (WithinParentChain(parent_contexts, *parent_chain) &&
!ProcessEntry(entry, [&](DWARFDIE die) {
// After .debug_names filtering still sending to base class for
// further filtering before calling the callback.
return ProcessTypeDIEMatchQuery(query, die, callback);
}))
return;
if (WithinParentChain(parent_contexts, *parent_chain)) {
if (!ProcessEntry(entry, [&](DWARFDIE die) {
// After .debug_names filtering still sending to base class for
// further filtering before calling the callback.
return ProcessTypeDIEMatchQuery(query, die, callback);
}))
// If the callback returns false, we're done.
return;
}
}
m_fallback.GetTypesWithQuery(query, callback);
}

void DebugNamesDWARFIndex::GetNamespacesWithParents(
ConstString name, const CompilerDeclContext &parent_decl_ctx,
llvm::function_ref<bool(DWARFDIE die)> callback) {
std::vector<lldb_private::CompilerContext> parent_contexts =
parent_decl_ctx.GetCompilerContext();
llvm::SmallVector<CompilerContext> parent_named_contexts;
std::copy_if(parent_contexts.rbegin(), parent_contexts.rend(),
std::back_inserter(parent_named_contexts),
[](const CompilerContext &ctx) { return !ctx.name.IsEmpty(); });
for (const DebugNames::Entry &entry :
m_debug_names_up->equal_range(name.GetStringRef())) {
lldb_private::dwarf::Tag entry_tag = entry.tag();
if (entry_tag == DW_TAG_namespace ||
entry_tag == DW_TAG_imported_declaration) {
std::optional<llvm::SmallVector<Entry, 4>> parent_chain =
getParentChain(entry);
if (!parent_chain) {
// Fallback: use the base class implementation.
if (!ProcessEntry(entry, [&](DWARFDIE die) {
return ProcessNamespaceDieMatchParents(parent_decl_ctx, die,
callback);
}))
return;
continue;
}

if (WithinParentChain(parent_named_contexts, *parent_chain)) {
if (!ProcessEntry(entry, [&](DWARFDIE die) {
// After .debug_names filtering still sending to base class for
// further filtering before calling the callback.
return ProcessNamespaceDieMatchParents(parent_decl_ctx, die,
callback);
}))
// If the callback returns false, we're done.
return;
}
}
}
m_fallback.GetNamespacesWithParents(name, parent_decl_ctx, callback);
}

void DebugNamesDWARFIndex::GetFunctions(
const Module::LookupInfo &lookup_info, SymbolFileDWARF &dwarf,
const CompilerDeclContext &parent_decl_ctx,
Expand Down
4 changes: 3 additions & 1 deletion lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,9 @@ class DebugNamesDWARFIndex : public DWARFIndex {
void
GetTypesWithQuery(TypeQuery &query,
llvm::function_ref<bool(DWARFDIE die)> callback) override;

void GetNamespacesWithParents(
ConstString name, const CompilerDeclContext &parent_decl_ctx,
llvm::function_ref<bool(DWARFDIE die)> callback) override;
void GetFunctions(const Module::LookupInfo &lookup_info,
SymbolFileDWARF &dwarf,
const CompilerDeclContext &parent_decl_ctx,
Expand Down
2 changes: 1 addition & 1 deletion lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2900,7 +2900,7 @@ SymbolFileDWARF::FindNamespace(ConstString name,
if (!DeclContextMatchesThisSymbolFile(parent_decl_ctx))
return namespace_decl_ctx;

m_index->GetNamespaces(name, [&](DWARFDIE die) {
m_index->GetNamespacesWithParents(name, parent_decl_ctx, [&](DWARFDIE die) {
if (!DIEInDeclContext(parent_decl_ctx, die, only_root_namespaces))
return true; // The containing decl contexts don't match

Expand Down

0 comments on commit cb04d33

Please sign in to comment.