[Lldb-commits] [lldb] Remove redundant symbol lookups in IRExecutionUnit::FindInSymbols (PR #102835)
https://github.com/DmT021 created
https://github.com/llvm/llvm-project/pull/102835
When we search for a symbol, we first check if it is in the module_sp of the
current SymbolContext, and if not, we check in the target's modules. However,
the target's ModuleList also includes the already checked module, which leads
to a redundant search in it.
>From c417f458aed8a177e75ae83ead687d7b64a0 Mon Sep 17 00:00:00 2001
From: Dmitrii Galimzianov
Date: Mon, 12 Aug 2024 01:09:33 +0200
Subject: [PATCH] Remove redundant symbol lookups in
IRExecutionUnit::FindInSymbols
When we search for a symbol, we first check if it is in the module_sp of the
current SymbolContext, and if not, we check in the target's modules. However,
the target's ModuleList also includes the already checked module, which leads
to a redundant search in it.
---
lldb/source/Expression/IRExecutionUnit.cpp | 21 +
1 file changed, 13 insertions(+), 8 deletions(-)
diff --git a/lldb/source/Expression/IRExecutionUnit.cpp
b/lldb/source/Expression/IRExecutionUnit.cpp
index f220704423627d..ad089e36eba7d6 100644
--- a/lldb/source/Expression/IRExecutionUnit.cpp
+++ b/lldb/source/Expression/IRExecutionUnit.cpp
@@ -785,6 +785,10 @@ IRExecutionUnit::FindInSymbols(const
std::vector &names,
return LLDB_INVALID_ADDRESS;
}
+ ModuleList images = target->GetImages();
+ // We'll process module_sp separately, before the other modules.
+ images.Remove(sc.module_sp);
+
LoadAddressResolver resolver(target, symbol_was_missing_weak);
ModuleFunctionSearchOptions function_options;
@@ -799,20 +803,21 @@ IRExecutionUnit::FindInSymbols(const
std::vector &names,
sc_list);
if (auto load_addr = resolver.Resolve(sc_list))
return *load_addr;
-}
-if (sc.target_sp) {
- SymbolContextList sc_list;
- sc.target_sp->GetImages().FindFunctions(name,
lldb::eFunctionNameTypeFull,
- function_options, sc_list);
+ sc.module_sp->FindSymbolsWithNameAndType(name, lldb::eSymbolTypeAny,
+ sc_list);
if (auto load_addr = resolver.Resolve(sc_list))
return *load_addr;
}
-if (sc.target_sp) {
+{
SymbolContextList sc_list;
- sc.target_sp->GetImages().FindSymbolsWithNameAndType(
- name, lldb::eSymbolTypeAny, sc_list);
+ images.FindFunctions(name, lldb::eFunctionNameTypeFull, function_options,
+ sc_list);
+ if (auto load_addr = resolver.Resolve(sc_list))
+return *load_addr;
+
+ images.FindSymbolsWithNameAndType(name, lldb::eSymbolTypeAny, sc_list);
if (auto load_addr = resolver.Resolve(sc_list))
return *load_addr;
}
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Remove redundant symbol lookups in IRExecutionUnit::FindInSymbols (PR #102835)
DmT021 wrote: @augusto2112 Please take a look https://github.com/llvm/llvm-project/pull/102835 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Remove redundant symbol lookups in IRExecutionUnit::FindInSymbols (PR #102835)
@@ -785,6 +785,10 @@ IRExecutionUnit::FindInSymbols(const
std::vector &names,
return LLDB_INVALID_ADDRESS;
}
+ ModuleList images = target->GetImages();
DmT021 wrote:
It was already checked 4 lines above
```
if (!target) {
// We shouldn't be doing any symbol lookup at all without a target.
return LLDB_INVALID_ADDRESS;
}
ModuleList images = target->GetImages();
```
https://github.com/llvm/llvm-project/pull/102835
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Remove redundant symbol lookups in IRExecutionUnit::FindInSymbols (PR #102835)
DmT021 wrote: > Could you run `ninja check-lldb` from your build folder to make sure this > change doesn't break anything? Already did. All tests are ok. https://github.com/llvm/llvm-project/pull/102835 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Remove redundant symbol lookups in IRExecutionUnit::FindInSymbols (PR #102835)
@@ -799,20 +803,21 @@ IRExecutionUnit::FindInSymbols(const
std::vector &names,
sc_list);
if (auto load_addr = resolver.Resolve(sc_list))
return *load_addr;
-}
-if (sc.target_sp) {
- SymbolContextList sc_list;
- sc.target_sp->GetImages().FindFunctions(name,
lldb::eFunctionNameTypeFull,
- function_options, sc_list);
+ sc.module_sp->FindSymbolsWithNameAndType(name, lldb::eSymbolTypeAny,
+ sc_list);
if (auto load_addr = resolver.Resolve(sc_list))
return *load_addr;
}
-if (sc.target_sp) {
+{
DmT021 wrote:
Ok, I just wanted to emphasize that these two lookups are similar, but
intentionally separeted.
https://github.com/llvm/llvm-project/pull/102835
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Remove redundant symbol lookups in IRExecutionUnit::FindInSymbols (PR #102835)
https://github.com/DmT021 updated
https://github.com/llvm/llvm-project/pull/102835
>From 45c91e403a36360727a5cd51e1a84f86027a77bc Mon Sep 17 00:00:00 2001
From: Dmitrii Galimzianov
Date: Mon, 12 Aug 2024 01:09:33 +0200
Subject: [PATCH] Remove redundant symbol lookups in
IRExecutionUnit::FindInSymbols
When we search for a symbol, we first check if it is in the module_sp of the
current SymbolContext, and if not, we check in the target's modules. However,
the target's ModuleList also includes the already checked module, which leads
to a redundant search in it.
---
lldb/source/Expression/IRExecutionUnit.cpp | 27 --
1 file changed, 15 insertions(+), 12 deletions(-)
diff --git a/lldb/source/Expression/IRExecutionUnit.cpp
b/lldb/source/Expression/IRExecutionUnit.cpp
index f220704423627d..aaf9cc3cfc9a23 100644
--- a/lldb/source/Expression/IRExecutionUnit.cpp
+++ b/lldb/source/Expression/IRExecutionUnit.cpp
@@ -785,6 +785,10 @@ IRExecutionUnit::FindInSymbols(const
std::vector &names,
return LLDB_INVALID_ADDRESS;
}
+ ModuleList images = target->GetImages();
+ // We'll process module_sp separately, before the other modules.
+ images.Remove(sc.module_sp);
+
LoadAddressResolver resolver(target, symbol_was_missing_weak);
ModuleFunctionSearchOptions function_options;
@@ -799,23 +803,22 @@ IRExecutionUnit::FindInSymbols(const
std::vector &names,
sc_list);
if (auto load_addr = resolver.Resolve(sc_list))
return *load_addr;
-}
-if (sc.target_sp) {
- SymbolContextList sc_list;
- sc.target_sp->GetImages().FindFunctions(name,
lldb::eFunctionNameTypeFull,
- function_options, sc_list);
+ sc.module_sp->FindSymbolsWithNameAndType(name, lldb::eSymbolTypeAny,
+ sc_list);
if (auto load_addr = resolver.Resolve(sc_list))
return *load_addr;
}
-if (sc.target_sp) {
- SymbolContextList sc_list;
- sc.target_sp->GetImages().FindSymbolsWithNameAndType(
- name, lldb::eSymbolTypeAny, sc_list);
- if (auto load_addr = resolver.Resolve(sc_list))
-return *load_addr;
-}
+SymbolContextList sc_list;
+images.FindFunctions(name, lldb::eFunctionNameTypeFull, function_options,
+ sc_list);
+if (auto load_addr = resolver.Resolve(sc_list))
+ return *load_addr;
+
+images.FindSymbolsWithNameAndType(name, lldb::eSymbolTypeAny, sc_list);
+if (auto load_addr = resolver.Resolve(sc_list))
+ return *load_addr;
lldb::addr_t best_internal_load_address =
resolver.GetBestInternalLoadAddress();
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Remove redundant symbol lookups in IRExecutionUnit::FindInSymbols (PR #102835)
https://github.com/DmT021 ready_for_review https://github.com/llvm/llvm-project/pull/102835 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Remove redundant symbol lookups in IRExecutionUnit::FindInSymbols (PR #102835)
DmT021 wrote: > Out of curiosity, could you provide some context around where/how this came > up? I just noticed this by accident in the `dwarf all` log when I was trying to figure out why LLDB was evaluating expressions slowly in my project. Here's a swift forum thread with some logs. https://forums.swift.org/t/symbolfiledwarf-findtypes-is-called-too-many-times-apparently/72902 Notice for example how `swift_release` is searched twice in `F1.o` https://github.com/llvm/llvm-project/pull/102835 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Remove redundant symbol lookups in IRExecutionUnit::FindInSymbols (PR #102835)
@@ -285,7 +285,7 @@ class ModuleList {
/// \see Module::FindFunctions ()
void FindFunctions(ConstString name, lldb::FunctionNameType name_type_mask,
const ModuleFunctionSearchOptions &options,
- SymbolContextList &sc_list) const;
+ const SymbolContext &sc, SymbolContextList &sc_list)
const;
DmT021 wrote:
> How about we call sc, search_first instead, or something more
> self-documenting like that? (which is what FindTypes also does).
The word "hint" came up a couple times in the discussion as well. Maybe
`search_hint` ?
> could we add documentation to the function describing what the parameter does?
Sure.
https://github.com/llvm/llvm-project/pull/102835
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Remove redundant symbol lookups in IRExecutionUnit::FindInSymbols (PR #102835)
https://github.com/DmT021 edited https://github.com/llvm/llvm-project/pull/102835 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Remove redundant symbol lookups in IRExecutionUnit::FindInSymbols (PR #102835)
DmT021 wrote: Oh, wait a sec. I actually changed the behavior of the `IRExecutionUnit::FindInSymbols`. It used to exit early if the function was found in `module_sp`, but now we will always scan through the whole ModuleList. And we can't change the behavior `ModuleList::FindFunctions` to return after the first match, because it might be not what we want in general case. Maybe I should add more generic versions of these functions taking a callback to invoke on each match instead of SymbolContextList? Something like ``` void ModuleList::FindSymbolsWithNameAndType( ConstString name, lldb::SymbolType symbol_type, const SymbolContext *search_hint, llvm::function_ref callback ) ``` where the result of `callback` indicates whether the search should be continued or not. https://github.com/llvm/llvm-project/pull/102835 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Remove redundant symbol lookups in IRExecutionUnit::FindInSymbols (PR #102835)
DmT021 wrote: > > > Oh, wait a sec. I actually changed the behavior of the > > > `IRExecutionUnit::FindInSymbols`. It used to exit early if the function > > > was found in `module_sp`, but now we will always scan through the whole > > > ModuleList. And we can't change the behavior of the > > > `ModuleList::FindFunctions` to return after the first match, because it > > > might be not what we want in general case. Maybe I should add more > > > generic versions of these functions taking a callback to invoke on each > > > match instead of SymbolContextList? Something like > > > ``` > > > void ModuleList::FindSymbolsWithNameAndType( > > > ConstString name, > > > lldb::SymbolType symbol_type, > > > const SymbolContext *search_hint, > > > llvm::function_ref callback > > > ) > > > ``` > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > where the result of `callback` indicates whether the search should be > > > continued or not. > > > > > > I think it'd be fine to return early if the type if found with the > > `search_hint`, as long as this is clearly documented in the API. My > > impression is that this would be a more useful behavior than just > > reordering the search results so the `search_hint` results are first, and I > > don't think we need the extra flexibility provided by that callback at the > > moment. I don't touch this code very often though so if @clayborg or > > @Michael137 disagree I'd defer to them. > > Probably fine returning early unconditionally tbh, but only if the > `search_hint` is specified. It shouldn't break anything? It wouldn't be that good API then, because the function takes an optional argument that changes the behavior significantly: - when it's provided the function returns a single result(which means we don't really need `SymbolContextList`) - when it's NULL, the function will find all the matches https://github.com/llvm/llvm-project/pull/102835 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Remove redundant symbol lookups in IRExecutionUnit::FindInSymbols (PR #102835)
DmT021 wrote: Could you please explain to me why we use different rules in symbol lookups? Namely: - [ClangExpressionDeclMap::GetSymbolAddress](https://github.com/llvm/llvm-project/pull/102835/files#diff-5d2da8306a4f4991885836925979f188658789adc8041c37811c243f2cdca24c) doesn't search in the ModuleList if a module is given even if the search provides no results. - [SymbolContext::FindBestGlobalDataSymbol](https://github.com/llvm/llvm-project/pull/102835/files#diff-da1a374f5098c39acfebae4b87a261f143a842e613082c2296de7ee28df12a33) searches in the module first, then checks the results; if exactly one external or internal symbol is found returns it. Otherwise, if more than 1 is found it produces an error. But if nothing is found in the module it searches in the module list and repeats the checks. So theoretically there could be multiple results in the module list that would trigger an error, but we don't check for them. Also, it seems an external symbol takes precedence over an internal symbol. If we found an internal symbol in the module, we still could find an external symbol in the module list, but we don't search for it. Is this correct? Does an internal symbol of the module actually take precedence over an external symbol from the module list? - [IRExecutionUnit::FindInSymbols](https://github.com/llvm/llvm-project/pull/102835/files#diff-717a850c4315286c025e2739ebe9dacbf27e626b7679c72479b05c996d721112) looks similar to the previous one, but actually very different. It returns early if and only if the found symbol is external(see LoadAddressResolver::Resolve), otherwise it does a full search in the module list. And only then if an external symbol isn't found - returns the first internal symbol(if any). It's hard to generalize the optimization since all the callers post-process the results differently. https://github.com/llvm/llvm-project/pull/102835 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Remove redundant symbol lookups in IRExecutionUnit::FindInSymbols (PR #102835)
https://github.com/DmT021 updated
https://github.com/llvm/llvm-project/pull/102835
>From ffe47e8dfe71b31951913bc571cd6830eb3f2426 Mon Sep 17 00:00:00 2001
From: Dmitrii Galimzianov
Date: Sun, 18 Aug 2024 04:36:19 +0200
Subject: [PATCH] Remove redundant symbol lookups in
IRExecutionUnit::FindInSymbols
---
lldb/include/lldb/Core/ModuleList.h| 43 ++
lldb/include/lldb/Symbol/SymbolContext.h | 2 +-
lldb/source/Core/ModuleList.cpp| 68 ++
lldb/source/Expression/IRExecutionUnit.cpp | 55 +
lldb/source/Symbol/Symbol.cpp | 8 +++
lldb/source/Symbol/SymbolContext.cpp | 68 ++
6 files changed, 194 insertions(+), 50 deletions(-)
diff --git a/lldb/include/lldb/Core/ModuleList.h
b/lldb/include/lldb/Core/ModuleList.h
index 43d931a8447406..171850e59baa35 100644
--- a/lldb/include/lldb/Core/ModuleList.h
+++ b/lldb/include/lldb/Core/ModuleList.h
@@ -287,6 +287,24 @@ class ModuleList {
const ModuleFunctionSearchOptions &options,
SymbolContextList &sc_list) const;
+ /// \see Module::FindFunctions ()
+ ///
+ /// \param[in] search_hint
+ /// If the value is NULL, then all modules will be searched in
+ /// order. If the value is a valid pointer and if a module is specified
in
+ /// the symbol context, that module will be searched first followed by
all
+ /// other modules in the list.
+ /// \param[in] partial_result_handler
+ /// A callback that will be called for each module in which at least one
+ /// match was found.
+ void FindFunctions(ConstString name, lldb::FunctionNameType name_type_mask,
+ const ModuleFunctionSearchOptions &options,
+ const SymbolContext *search_hint,
+ llvm::function_ref
+ partial_result_handler) const;
+
/// \see Module::FindFunctionSymbols ()
void FindFunctionSymbols(ConstString name,
lldb::FunctionNameType name_type_mask,
@@ -357,6 +375,31 @@ class ModuleList {
lldb::SymbolType symbol_type,
SymbolContextList &sc_list) const;
+ /// Find symbols by name and SymbolType.
+ ///
+ /// \param[in] name
+ /// A name of the symbol we are looking for.
+ ///
+ /// \param[in] symbol_type
+ /// A SymbolType the symbol we are looking for.
+ ///
+ /// \param[in] search_hint
+ /// If the value is NULL, then all modules will be searched in
+ /// order. If the value is a valid pointer and if a module is specified
in
+ /// the symbol context, that module will be searched first followed by
all
+ /// other modules in the list.
+ ///
+ /// \param[in] partial_result_handler
+ /// A callback that will be called for each module in which at least one
+ /// match was found.
+ void FindSymbolsWithNameAndType(ConstString name,
+ lldb::SymbolType symbol_type,
+ const SymbolContext *search_hint,
+ llvm::function_ref
+ partial_result_handler) const;
+
void FindSymbolsMatchingRegExAndType(const RegularExpression ®ex,
lldb::SymbolType symbol_type,
SymbolContextList &sc_list) const;
diff --git a/lldb/include/lldb/Symbol/SymbolContext.h
b/lldb/include/lldb/Symbol/SymbolContext.h
index 0bc707070f8504..66622b5c15f7c9 100644
--- a/lldb/include/lldb/Symbol/SymbolContext.h
+++ b/lldb/include/lldb/Symbol/SymbolContext.h
@@ -471,7 +471,7 @@ class SymbolContextList {
typedef AdaptedIterable
SymbolContextIterable;
- SymbolContextIterable SymbolContexts() {
+ SymbolContextIterable SymbolContexts() const {
return SymbolContextIterable(m_symbol_contexts);
}
};
diff --git a/lldb/source/Core/ModuleList.cpp b/lldb/source/Core/ModuleList.cpp
index b03490bacf9593..9b45334fb65a15 100644
--- a/lldb/source/Core/ModuleList.cpp
+++ b/lldb/source/Core/ModuleList.cpp
@@ -466,6 +466,41 @@ void ModuleList::FindFunctions(ConstString name,
}
}
+void ModuleList::FindFunctions(
+ConstString name, lldb::FunctionNameType name_type_mask,
+const ModuleFunctionSearchOptions &options,
+const SymbolContext *search_hint,
+llvm::function_ref
+partial_result_handler) const {
+ SymbolContextList sc_list_partial{};
+ auto FindInModule = [&](const ModuleSP &module) {
+module->FindFunctions(name, CompilerDeclContext(), name_type_mask, options,
+ sc_list_partial);
+if (!sc_list_partial.IsEmpty()) {
+ auto iteration_action = partial_result_handler(module, sc_list_partial);
+ sc_list_partial.Clear();
+ return iteration_action;
+}
+return IterationAction::Continue;
+ };
+
+ auto hinted_module = search_hint ? search_h
[Lldb-commits] [lldb] Remove redundant symbol lookups in IRExecutionUnit::FindInSymbols (PR #102835)
DmT021 wrote: I implemented the solution with a callback and used it in `IRExecutionUnit::FindInSymbols` and `SymbolContext::FindBestGlobalDataSymbol`. But it's not very suitable for `ClangExpressionDeclMap::GetSymbolAddress` :-/ Also, for some reason I'm getting `UNEXPECTED SUCCESS: test_shadowed (TestConflictingSymbol.TestConflictingSymbols.test_shadowed)`. I tried to stick to the original algorithm of `SymbolContext::FindBestGlobalDataSymbol` as close as possible. I'll check if it fails on CI as well. https://github.com/llvm/llvm-project/pull/102835 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Remove redundant symbol lookups in IRExecutionUnit::FindInSymbols (PR #102835)
https://github.com/DmT021 updated
https://github.com/llvm/llvm-project/pull/102835
>From c0b9be1b4ef436bbf79bd3877a58e6b598b19940 Mon Sep 17 00:00:00 2001
From: Dmitrii Galimzianov
Date: Sun, 18 Aug 2024 04:36:19 +0200
Subject: [PATCH] Remove redundant symbol lookups in
IRExecutionUnit::FindInSymbols
---
lldb/include/lldb/Core/ModuleList.h| 43 +
lldb/include/lldb/Symbol/SymbolContext.h | 2 +-
lldb/source/Core/ModuleList.cpp| 68 +
lldb/source/Expression/IRExecutionUnit.cpp | 55 +
lldb/source/Symbol/Symbol.cpp | 8 +++
lldb/source/Symbol/SymbolContext.cpp | 71 ++
6 files changed, 197 insertions(+), 50 deletions(-)
diff --git a/lldb/include/lldb/Core/ModuleList.h
b/lldb/include/lldb/Core/ModuleList.h
index 43d931a8447406..171850e59baa35 100644
--- a/lldb/include/lldb/Core/ModuleList.h
+++ b/lldb/include/lldb/Core/ModuleList.h
@@ -287,6 +287,24 @@ class ModuleList {
const ModuleFunctionSearchOptions &options,
SymbolContextList &sc_list) const;
+ /// \see Module::FindFunctions ()
+ ///
+ /// \param[in] search_hint
+ /// If the value is NULL, then all modules will be searched in
+ /// order. If the value is a valid pointer and if a module is specified
in
+ /// the symbol context, that module will be searched first followed by
all
+ /// other modules in the list.
+ /// \param[in] partial_result_handler
+ /// A callback that will be called for each module in which at least one
+ /// match was found.
+ void FindFunctions(ConstString name, lldb::FunctionNameType name_type_mask,
+ const ModuleFunctionSearchOptions &options,
+ const SymbolContext *search_hint,
+ llvm::function_ref
+ partial_result_handler) const;
+
/// \see Module::FindFunctionSymbols ()
void FindFunctionSymbols(ConstString name,
lldb::FunctionNameType name_type_mask,
@@ -357,6 +375,31 @@ class ModuleList {
lldb::SymbolType symbol_type,
SymbolContextList &sc_list) const;
+ /// Find symbols by name and SymbolType.
+ ///
+ /// \param[in] name
+ /// A name of the symbol we are looking for.
+ ///
+ /// \param[in] symbol_type
+ /// A SymbolType the symbol we are looking for.
+ ///
+ /// \param[in] search_hint
+ /// If the value is NULL, then all modules will be searched in
+ /// order. If the value is a valid pointer and if a module is specified
in
+ /// the symbol context, that module will be searched first followed by
all
+ /// other modules in the list.
+ ///
+ /// \param[in] partial_result_handler
+ /// A callback that will be called for each module in which at least one
+ /// match was found.
+ void FindSymbolsWithNameAndType(ConstString name,
+ lldb::SymbolType symbol_type,
+ const SymbolContext *search_hint,
+ llvm::function_ref
+ partial_result_handler) const;
+
void FindSymbolsMatchingRegExAndType(const RegularExpression ®ex,
lldb::SymbolType symbol_type,
SymbolContextList &sc_list) const;
diff --git a/lldb/include/lldb/Symbol/SymbolContext.h
b/lldb/include/lldb/Symbol/SymbolContext.h
index 0bc707070f8504..66622b5c15f7c9 100644
--- a/lldb/include/lldb/Symbol/SymbolContext.h
+++ b/lldb/include/lldb/Symbol/SymbolContext.h
@@ -471,7 +471,7 @@ class SymbolContextList {
typedef AdaptedIterable
SymbolContextIterable;
- SymbolContextIterable SymbolContexts() {
+ SymbolContextIterable SymbolContexts() const {
return SymbolContextIterable(m_symbol_contexts);
}
};
diff --git a/lldb/source/Core/ModuleList.cpp b/lldb/source/Core/ModuleList.cpp
index b03490bacf9593..9b45334fb65a15 100644
--- a/lldb/source/Core/ModuleList.cpp
+++ b/lldb/source/Core/ModuleList.cpp
@@ -466,6 +466,41 @@ void ModuleList::FindFunctions(ConstString name,
}
}
+void ModuleList::FindFunctions(
+ConstString name, lldb::FunctionNameType name_type_mask,
+const ModuleFunctionSearchOptions &options,
+const SymbolContext *search_hint,
+llvm::function_ref
+partial_result_handler) const {
+ SymbolContextList sc_list_partial{};
+ auto FindInModule = [&](const ModuleSP &module) {
+module->FindFunctions(name, CompilerDeclContext(), name_type_mask, options,
+ sc_list_partial);
+if (!sc_list_partial.IsEmpty()) {
+ auto iteration_action = partial_result_handler(module, sc_list_partial);
+ sc_list_partial.Clear();
+ return iteration_action;
+}
+return IterationAction::Continue;
+ };
+
+ auto hinted_module = search_hint ? search_hin
[Lldb-commits] [lldb] Remove redundant symbol lookups in IRExecutionUnit::FindInSymbols (PR #102835)
https://github.com/DmT021 updated
https://github.com/llvm/llvm-project/pull/102835
>From c0b9be1b4ef436bbf79bd3877a58e6b598b19940 Mon Sep 17 00:00:00 2001
From: Dmitrii Galimzianov
Date: Sun, 18 Aug 2024 04:36:19 +0200
Subject: [PATCH 1/2] Remove redundant symbol lookups in
IRExecutionUnit::FindInSymbols
---
lldb/include/lldb/Core/ModuleList.h| 43 +
lldb/include/lldb/Symbol/SymbolContext.h | 2 +-
lldb/source/Core/ModuleList.cpp| 68 +
lldb/source/Expression/IRExecutionUnit.cpp | 55 +
lldb/source/Symbol/Symbol.cpp | 8 +++
lldb/source/Symbol/SymbolContext.cpp | 71 ++
6 files changed, 197 insertions(+), 50 deletions(-)
diff --git a/lldb/include/lldb/Core/ModuleList.h
b/lldb/include/lldb/Core/ModuleList.h
index 43d931a8447406..171850e59baa35 100644
--- a/lldb/include/lldb/Core/ModuleList.h
+++ b/lldb/include/lldb/Core/ModuleList.h
@@ -287,6 +287,24 @@ class ModuleList {
const ModuleFunctionSearchOptions &options,
SymbolContextList &sc_list) const;
+ /// \see Module::FindFunctions ()
+ ///
+ /// \param[in] search_hint
+ /// If the value is NULL, then all modules will be searched in
+ /// order. If the value is a valid pointer and if a module is specified
in
+ /// the symbol context, that module will be searched first followed by
all
+ /// other modules in the list.
+ /// \param[in] partial_result_handler
+ /// A callback that will be called for each module in which at least one
+ /// match was found.
+ void FindFunctions(ConstString name, lldb::FunctionNameType name_type_mask,
+ const ModuleFunctionSearchOptions &options,
+ const SymbolContext *search_hint,
+ llvm::function_ref
+ partial_result_handler) const;
+
/// \see Module::FindFunctionSymbols ()
void FindFunctionSymbols(ConstString name,
lldb::FunctionNameType name_type_mask,
@@ -357,6 +375,31 @@ class ModuleList {
lldb::SymbolType symbol_type,
SymbolContextList &sc_list) const;
+ /// Find symbols by name and SymbolType.
+ ///
+ /// \param[in] name
+ /// A name of the symbol we are looking for.
+ ///
+ /// \param[in] symbol_type
+ /// A SymbolType the symbol we are looking for.
+ ///
+ /// \param[in] search_hint
+ /// If the value is NULL, then all modules will be searched in
+ /// order. If the value is a valid pointer and if a module is specified
in
+ /// the symbol context, that module will be searched first followed by
all
+ /// other modules in the list.
+ ///
+ /// \param[in] partial_result_handler
+ /// A callback that will be called for each module in which at least one
+ /// match was found.
+ void FindSymbolsWithNameAndType(ConstString name,
+ lldb::SymbolType symbol_type,
+ const SymbolContext *search_hint,
+ llvm::function_ref
+ partial_result_handler) const;
+
void FindSymbolsMatchingRegExAndType(const RegularExpression ®ex,
lldb::SymbolType symbol_type,
SymbolContextList &sc_list) const;
diff --git a/lldb/include/lldb/Symbol/SymbolContext.h
b/lldb/include/lldb/Symbol/SymbolContext.h
index 0bc707070f8504..66622b5c15f7c9 100644
--- a/lldb/include/lldb/Symbol/SymbolContext.h
+++ b/lldb/include/lldb/Symbol/SymbolContext.h
@@ -471,7 +471,7 @@ class SymbolContextList {
typedef AdaptedIterable
SymbolContextIterable;
- SymbolContextIterable SymbolContexts() {
+ SymbolContextIterable SymbolContexts() const {
return SymbolContextIterable(m_symbol_contexts);
}
};
diff --git a/lldb/source/Core/ModuleList.cpp b/lldb/source/Core/ModuleList.cpp
index b03490bacf9593..9b45334fb65a15 100644
--- a/lldb/source/Core/ModuleList.cpp
+++ b/lldb/source/Core/ModuleList.cpp
@@ -466,6 +466,41 @@ void ModuleList::FindFunctions(ConstString name,
}
}
+void ModuleList::FindFunctions(
+ConstString name, lldb::FunctionNameType name_type_mask,
+const ModuleFunctionSearchOptions &options,
+const SymbolContext *search_hint,
+llvm::function_ref
+partial_result_handler) const {
+ SymbolContextList sc_list_partial{};
+ auto FindInModule = [&](const ModuleSP &module) {
+module->FindFunctions(name, CompilerDeclContext(), name_type_mask, options,
+ sc_list_partial);
+if (!sc_list_partial.IsEmpty()) {
+ auto iteration_action = partial_result_handler(module, sc_list_partial);
+ sc_list_partial.Clear();
+ return iteration_action;
+}
+return IterationAction::Continue;
+ };
+
+ auto hinted_module = search_hint ? search
[Lldb-commits] [lldb] Remove redundant symbol lookups in IRExecutionUnit::FindInSymbols (PR #102835)
https://github.com/DmT021 updated
https://github.com/llvm/llvm-project/pull/102835
>From c0b9be1b4ef436bbf79bd3877a58e6b598b19940 Mon Sep 17 00:00:00 2001
From: Dmitrii Galimzianov
Date: Sun, 18 Aug 2024 04:36:19 +0200
Subject: [PATCH 1/2] Remove redundant symbol lookups in
IRExecutionUnit::FindInSymbols
---
lldb/include/lldb/Core/ModuleList.h| 43 +
lldb/include/lldb/Symbol/SymbolContext.h | 2 +-
lldb/source/Core/ModuleList.cpp| 68 +
lldb/source/Expression/IRExecutionUnit.cpp | 55 +
lldb/source/Symbol/Symbol.cpp | 8 +++
lldb/source/Symbol/SymbolContext.cpp | 71 ++
6 files changed, 197 insertions(+), 50 deletions(-)
diff --git a/lldb/include/lldb/Core/ModuleList.h
b/lldb/include/lldb/Core/ModuleList.h
index 43d931a844740..171850e59baa3 100644
--- a/lldb/include/lldb/Core/ModuleList.h
+++ b/lldb/include/lldb/Core/ModuleList.h
@@ -287,6 +287,24 @@ class ModuleList {
const ModuleFunctionSearchOptions &options,
SymbolContextList &sc_list) const;
+ /// \see Module::FindFunctions ()
+ ///
+ /// \param[in] search_hint
+ /// If the value is NULL, then all modules will be searched in
+ /// order. If the value is a valid pointer and if a module is specified
in
+ /// the symbol context, that module will be searched first followed by
all
+ /// other modules in the list.
+ /// \param[in] partial_result_handler
+ /// A callback that will be called for each module in which at least one
+ /// match was found.
+ void FindFunctions(ConstString name, lldb::FunctionNameType name_type_mask,
+ const ModuleFunctionSearchOptions &options,
+ const SymbolContext *search_hint,
+ llvm::function_ref
+ partial_result_handler) const;
+
/// \see Module::FindFunctionSymbols ()
void FindFunctionSymbols(ConstString name,
lldb::FunctionNameType name_type_mask,
@@ -357,6 +375,31 @@ class ModuleList {
lldb::SymbolType symbol_type,
SymbolContextList &sc_list) const;
+ /// Find symbols by name and SymbolType.
+ ///
+ /// \param[in] name
+ /// A name of the symbol we are looking for.
+ ///
+ /// \param[in] symbol_type
+ /// A SymbolType the symbol we are looking for.
+ ///
+ /// \param[in] search_hint
+ /// If the value is NULL, then all modules will be searched in
+ /// order. If the value is a valid pointer and if a module is specified
in
+ /// the symbol context, that module will be searched first followed by
all
+ /// other modules in the list.
+ ///
+ /// \param[in] partial_result_handler
+ /// A callback that will be called for each module in which at least one
+ /// match was found.
+ void FindSymbolsWithNameAndType(ConstString name,
+ lldb::SymbolType symbol_type,
+ const SymbolContext *search_hint,
+ llvm::function_ref
+ partial_result_handler) const;
+
void FindSymbolsMatchingRegExAndType(const RegularExpression ®ex,
lldb::SymbolType symbol_type,
SymbolContextList &sc_list) const;
diff --git a/lldb/include/lldb/Symbol/SymbolContext.h
b/lldb/include/lldb/Symbol/SymbolContext.h
index 0bc707070f850..66622b5c15f7c 100644
--- a/lldb/include/lldb/Symbol/SymbolContext.h
+++ b/lldb/include/lldb/Symbol/SymbolContext.h
@@ -471,7 +471,7 @@ class SymbolContextList {
typedef AdaptedIterable
SymbolContextIterable;
- SymbolContextIterable SymbolContexts() {
+ SymbolContextIterable SymbolContexts() const {
return SymbolContextIterable(m_symbol_contexts);
}
};
diff --git a/lldb/source/Core/ModuleList.cpp b/lldb/source/Core/ModuleList.cpp
index b03490bacf959..9b45334fb65a1 100644
--- a/lldb/source/Core/ModuleList.cpp
+++ b/lldb/source/Core/ModuleList.cpp
@@ -466,6 +466,41 @@ void ModuleList::FindFunctions(ConstString name,
}
}
+void ModuleList::FindFunctions(
+ConstString name, lldb::FunctionNameType name_type_mask,
+const ModuleFunctionSearchOptions &options,
+const SymbolContext *search_hint,
+llvm::function_ref
+partial_result_handler) const {
+ SymbolContextList sc_list_partial{};
+ auto FindInModule = [&](const ModuleSP &module) {
+module->FindFunctions(name, CompilerDeclContext(), name_type_mask, options,
+ sc_list_partial);
+if (!sc_list_partial.IsEmpty()) {
+ auto iteration_action = partial_result_handler(module, sc_list_partial);
+ sc_list_partial.Clear();
+ return iteration_action;
+}
+return IterationAction::Continue;
+ };
+
+ auto hinted_module = search_hint ? search_hint-
[Lldb-commits] [lldb] Remove redundant symbol lookups in IRExecutionUnit::FindInSymbols (PR #102835)
https://github.com/DmT021 updated
https://github.com/llvm/llvm-project/pull/102835
>From c0b9be1b4ef436bbf79bd3877a58e6b598b19940 Mon Sep 17 00:00:00 2001
From: Dmitrii Galimzianov
Date: Sun, 18 Aug 2024 04:36:19 +0200
Subject: [PATCH 1/2] Remove redundant symbol lookups in
IRExecutionUnit::FindInSymbols
---
lldb/include/lldb/Core/ModuleList.h| 43 +
lldb/include/lldb/Symbol/SymbolContext.h | 2 +-
lldb/source/Core/ModuleList.cpp| 68 +
lldb/source/Expression/IRExecutionUnit.cpp | 55 +
lldb/source/Symbol/Symbol.cpp | 8 +++
lldb/source/Symbol/SymbolContext.cpp | 71 ++
6 files changed, 197 insertions(+), 50 deletions(-)
diff --git a/lldb/include/lldb/Core/ModuleList.h
b/lldb/include/lldb/Core/ModuleList.h
index 43d931a8447406..171850e59baa35 100644
--- a/lldb/include/lldb/Core/ModuleList.h
+++ b/lldb/include/lldb/Core/ModuleList.h
@@ -287,6 +287,24 @@ class ModuleList {
const ModuleFunctionSearchOptions &options,
SymbolContextList &sc_list) const;
+ /// \see Module::FindFunctions ()
+ ///
+ /// \param[in] search_hint
+ /// If the value is NULL, then all modules will be searched in
+ /// order. If the value is a valid pointer and if a module is specified
in
+ /// the symbol context, that module will be searched first followed by
all
+ /// other modules in the list.
+ /// \param[in] partial_result_handler
+ /// A callback that will be called for each module in which at least one
+ /// match was found.
+ void FindFunctions(ConstString name, lldb::FunctionNameType name_type_mask,
+ const ModuleFunctionSearchOptions &options,
+ const SymbolContext *search_hint,
+ llvm::function_ref
+ partial_result_handler) const;
+
/// \see Module::FindFunctionSymbols ()
void FindFunctionSymbols(ConstString name,
lldb::FunctionNameType name_type_mask,
@@ -357,6 +375,31 @@ class ModuleList {
lldb::SymbolType symbol_type,
SymbolContextList &sc_list) const;
+ /// Find symbols by name and SymbolType.
+ ///
+ /// \param[in] name
+ /// A name of the symbol we are looking for.
+ ///
+ /// \param[in] symbol_type
+ /// A SymbolType the symbol we are looking for.
+ ///
+ /// \param[in] search_hint
+ /// If the value is NULL, then all modules will be searched in
+ /// order. If the value is a valid pointer and if a module is specified
in
+ /// the symbol context, that module will be searched first followed by
all
+ /// other modules in the list.
+ ///
+ /// \param[in] partial_result_handler
+ /// A callback that will be called for each module in which at least one
+ /// match was found.
+ void FindSymbolsWithNameAndType(ConstString name,
+ lldb::SymbolType symbol_type,
+ const SymbolContext *search_hint,
+ llvm::function_ref
+ partial_result_handler) const;
+
void FindSymbolsMatchingRegExAndType(const RegularExpression ®ex,
lldb::SymbolType symbol_type,
SymbolContextList &sc_list) const;
diff --git a/lldb/include/lldb/Symbol/SymbolContext.h
b/lldb/include/lldb/Symbol/SymbolContext.h
index 0bc707070f8504..66622b5c15f7c9 100644
--- a/lldb/include/lldb/Symbol/SymbolContext.h
+++ b/lldb/include/lldb/Symbol/SymbolContext.h
@@ -471,7 +471,7 @@ class SymbolContextList {
typedef AdaptedIterable
SymbolContextIterable;
- SymbolContextIterable SymbolContexts() {
+ SymbolContextIterable SymbolContexts() const {
return SymbolContextIterable(m_symbol_contexts);
}
};
diff --git a/lldb/source/Core/ModuleList.cpp b/lldb/source/Core/ModuleList.cpp
index b03490bacf9593..9b45334fb65a15 100644
--- a/lldb/source/Core/ModuleList.cpp
+++ b/lldb/source/Core/ModuleList.cpp
@@ -466,6 +466,41 @@ void ModuleList::FindFunctions(ConstString name,
}
}
+void ModuleList::FindFunctions(
+ConstString name, lldb::FunctionNameType name_type_mask,
+const ModuleFunctionSearchOptions &options,
+const SymbolContext *search_hint,
+llvm::function_ref
+partial_result_handler) const {
+ SymbolContextList sc_list_partial{};
+ auto FindInModule = [&](const ModuleSP &module) {
+module->FindFunctions(name, CompilerDeclContext(), name_type_mask, options,
+ sc_list_partial);
+if (!sc_list_partial.IsEmpty()) {
+ auto iteration_action = partial_result_handler(module, sc_list_partial);
+ sc_list_partial.Clear();
+ return iteration_action;
+}
+return IterationAction::Continue;
+ };
+
+ auto hinted_module = search_hint ? search
[Lldb-commits] [lldb] Remove redundant symbol lookups in IRExecutionUnit::FindInSymbols (PR #102835)
https://github.com/DmT021 updated
https://github.com/llvm/llvm-project/pull/102835
>From c0b9be1b4ef436bbf79bd3877a58e6b598b19940 Mon Sep 17 00:00:00 2001
From: Dmitrii Galimzianov
Date: Sun, 18 Aug 2024 04:36:19 +0200
Subject: [PATCH 1/2] Remove redundant symbol lookups in
IRExecutionUnit::FindInSymbols
---
lldb/include/lldb/Core/ModuleList.h| 43 +
lldb/include/lldb/Symbol/SymbolContext.h | 2 +-
lldb/source/Core/ModuleList.cpp| 68 +
lldb/source/Expression/IRExecutionUnit.cpp | 55 +
lldb/source/Symbol/Symbol.cpp | 8 +++
lldb/source/Symbol/SymbolContext.cpp | 71 ++
6 files changed, 197 insertions(+), 50 deletions(-)
diff --git a/lldb/include/lldb/Core/ModuleList.h
b/lldb/include/lldb/Core/ModuleList.h
index 43d931a844740..171850e59baa3 100644
--- a/lldb/include/lldb/Core/ModuleList.h
+++ b/lldb/include/lldb/Core/ModuleList.h
@@ -287,6 +287,24 @@ class ModuleList {
const ModuleFunctionSearchOptions &options,
SymbolContextList &sc_list) const;
+ /// \see Module::FindFunctions ()
+ ///
+ /// \param[in] search_hint
+ /// If the value is NULL, then all modules will be searched in
+ /// order. If the value is a valid pointer and if a module is specified
in
+ /// the symbol context, that module will be searched first followed by
all
+ /// other modules in the list.
+ /// \param[in] partial_result_handler
+ /// A callback that will be called for each module in which at least one
+ /// match was found.
+ void FindFunctions(ConstString name, lldb::FunctionNameType name_type_mask,
+ const ModuleFunctionSearchOptions &options,
+ const SymbolContext *search_hint,
+ llvm::function_ref
+ partial_result_handler) const;
+
/// \see Module::FindFunctionSymbols ()
void FindFunctionSymbols(ConstString name,
lldb::FunctionNameType name_type_mask,
@@ -357,6 +375,31 @@ class ModuleList {
lldb::SymbolType symbol_type,
SymbolContextList &sc_list) const;
+ /// Find symbols by name and SymbolType.
+ ///
+ /// \param[in] name
+ /// A name of the symbol we are looking for.
+ ///
+ /// \param[in] symbol_type
+ /// A SymbolType the symbol we are looking for.
+ ///
+ /// \param[in] search_hint
+ /// If the value is NULL, then all modules will be searched in
+ /// order. If the value is a valid pointer and if a module is specified
in
+ /// the symbol context, that module will be searched first followed by
all
+ /// other modules in the list.
+ ///
+ /// \param[in] partial_result_handler
+ /// A callback that will be called for each module in which at least one
+ /// match was found.
+ void FindSymbolsWithNameAndType(ConstString name,
+ lldb::SymbolType symbol_type,
+ const SymbolContext *search_hint,
+ llvm::function_ref
+ partial_result_handler) const;
+
void FindSymbolsMatchingRegExAndType(const RegularExpression ®ex,
lldb::SymbolType symbol_type,
SymbolContextList &sc_list) const;
diff --git a/lldb/include/lldb/Symbol/SymbolContext.h
b/lldb/include/lldb/Symbol/SymbolContext.h
index 0bc707070f850..66622b5c15f7c 100644
--- a/lldb/include/lldb/Symbol/SymbolContext.h
+++ b/lldb/include/lldb/Symbol/SymbolContext.h
@@ -471,7 +471,7 @@ class SymbolContextList {
typedef AdaptedIterable
SymbolContextIterable;
- SymbolContextIterable SymbolContexts() {
+ SymbolContextIterable SymbolContexts() const {
return SymbolContextIterable(m_symbol_contexts);
}
};
diff --git a/lldb/source/Core/ModuleList.cpp b/lldb/source/Core/ModuleList.cpp
index b03490bacf959..9b45334fb65a1 100644
--- a/lldb/source/Core/ModuleList.cpp
+++ b/lldb/source/Core/ModuleList.cpp
@@ -466,6 +466,41 @@ void ModuleList::FindFunctions(ConstString name,
}
}
+void ModuleList::FindFunctions(
+ConstString name, lldb::FunctionNameType name_type_mask,
+const ModuleFunctionSearchOptions &options,
+const SymbolContext *search_hint,
+llvm::function_ref
+partial_result_handler) const {
+ SymbolContextList sc_list_partial{};
+ auto FindInModule = [&](const ModuleSP &module) {
+module->FindFunctions(name, CompilerDeclContext(), name_type_mask, options,
+ sc_list_partial);
+if (!sc_list_partial.IsEmpty()) {
+ auto iteration_action = partial_result_handler(module, sc_list_partial);
+ sc_list_partial.Clear();
+ return iteration_action;
+}
+return IterationAction::Continue;
+ };
+
+ auto hinted_module = search_hint ? search_hint-
[Lldb-commits] [lldb] Remove redundant symbol lookups in IRExecutionUnit::FindInSymbols (PR #102835)
https://github.com/DmT021 updated
https://github.com/llvm/llvm-project/pull/102835
>From c0b9be1b4ef436bbf79bd3877a58e6b598b19940 Mon Sep 17 00:00:00 2001
From: Dmitrii Galimzianov
Date: Sun, 18 Aug 2024 04:36:19 +0200
Subject: [PATCH 1/2] Remove redundant symbol lookups in
IRExecutionUnit::FindInSymbols
---
lldb/include/lldb/Core/ModuleList.h| 43 +
lldb/include/lldb/Symbol/SymbolContext.h | 2 +-
lldb/source/Core/ModuleList.cpp| 68 +
lldb/source/Expression/IRExecutionUnit.cpp | 55 +
lldb/source/Symbol/Symbol.cpp | 8 +++
lldb/source/Symbol/SymbolContext.cpp | 71 ++
6 files changed, 197 insertions(+), 50 deletions(-)
diff --git a/lldb/include/lldb/Core/ModuleList.h
b/lldb/include/lldb/Core/ModuleList.h
index 43d931a844740..171850e59baa3 100644
--- a/lldb/include/lldb/Core/ModuleList.h
+++ b/lldb/include/lldb/Core/ModuleList.h
@@ -287,6 +287,24 @@ class ModuleList {
const ModuleFunctionSearchOptions &options,
SymbolContextList &sc_list) const;
+ /// \see Module::FindFunctions ()
+ ///
+ /// \param[in] search_hint
+ /// If the value is NULL, then all modules will be searched in
+ /// order. If the value is a valid pointer and if a module is specified
in
+ /// the symbol context, that module will be searched first followed by
all
+ /// other modules in the list.
+ /// \param[in] partial_result_handler
+ /// A callback that will be called for each module in which at least one
+ /// match was found.
+ void FindFunctions(ConstString name, lldb::FunctionNameType name_type_mask,
+ const ModuleFunctionSearchOptions &options,
+ const SymbolContext *search_hint,
+ llvm::function_ref
+ partial_result_handler) const;
+
/// \see Module::FindFunctionSymbols ()
void FindFunctionSymbols(ConstString name,
lldb::FunctionNameType name_type_mask,
@@ -357,6 +375,31 @@ class ModuleList {
lldb::SymbolType symbol_type,
SymbolContextList &sc_list) const;
+ /// Find symbols by name and SymbolType.
+ ///
+ /// \param[in] name
+ /// A name of the symbol we are looking for.
+ ///
+ /// \param[in] symbol_type
+ /// A SymbolType the symbol we are looking for.
+ ///
+ /// \param[in] search_hint
+ /// If the value is NULL, then all modules will be searched in
+ /// order. If the value is a valid pointer and if a module is specified
in
+ /// the symbol context, that module will be searched first followed by
all
+ /// other modules in the list.
+ ///
+ /// \param[in] partial_result_handler
+ /// A callback that will be called for each module in which at least one
+ /// match was found.
+ void FindSymbolsWithNameAndType(ConstString name,
+ lldb::SymbolType symbol_type,
+ const SymbolContext *search_hint,
+ llvm::function_ref
+ partial_result_handler) const;
+
void FindSymbolsMatchingRegExAndType(const RegularExpression ®ex,
lldb::SymbolType symbol_type,
SymbolContextList &sc_list) const;
diff --git a/lldb/include/lldb/Symbol/SymbolContext.h
b/lldb/include/lldb/Symbol/SymbolContext.h
index 0bc707070f850..66622b5c15f7c 100644
--- a/lldb/include/lldb/Symbol/SymbolContext.h
+++ b/lldb/include/lldb/Symbol/SymbolContext.h
@@ -471,7 +471,7 @@ class SymbolContextList {
typedef AdaptedIterable
SymbolContextIterable;
- SymbolContextIterable SymbolContexts() {
+ SymbolContextIterable SymbolContexts() const {
return SymbolContextIterable(m_symbol_contexts);
}
};
diff --git a/lldb/source/Core/ModuleList.cpp b/lldb/source/Core/ModuleList.cpp
index b03490bacf959..9b45334fb65a1 100644
--- a/lldb/source/Core/ModuleList.cpp
+++ b/lldb/source/Core/ModuleList.cpp
@@ -466,6 +466,41 @@ void ModuleList::FindFunctions(ConstString name,
}
}
+void ModuleList::FindFunctions(
+ConstString name, lldb::FunctionNameType name_type_mask,
+const ModuleFunctionSearchOptions &options,
+const SymbolContext *search_hint,
+llvm::function_ref
+partial_result_handler) const {
+ SymbolContextList sc_list_partial{};
+ auto FindInModule = [&](const ModuleSP &module) {
+module->FindFunctions(name, CompilerDeclContext(), name_type_mask, options,
+ sc_list_partial);
+if (!sc_list_partial.IsEmpty()) {
+ auto iteration_action = partial_result_handler(module, sc_list_partial);
+ sc_list_partial.Clear();
+ return iteration_action;
+}
+return IterationAction::Continue;
+ };
+
+ auto hinted_module = search_hint ? search_hint-
[Lldb-commits] [lldb] Remove redundant symbol lookups in IRExecutionUnit::FindInSymbols (PR #102835)
https://github.com/DmT021 updated
https://github.com/llvm/llvm-project/pull/102835
>From c0b9be1b4ef436bbf79bd3877a58e6b598b19940 Mon Sep 17 00:00:00 2001
From: Dmitrii Galimzianov
Date: Sun, 18 Aug 2024 04:36:19 +0200
Subject: [PATCH 1/2] Remove redundant symbol lookups in
IRExecutionUnit::FindInSymbols
---
lldb/include/lldb/Core/ModuleList.h| 43 +
lldb/include/lldb/Symbol/SymbolContext.h | 2 +-
lldb/source/Core/ModuleList.cpp| 68 +
lldb/source/Expression/IRExecutionUnit.cpp | 55 +
lldb/source/Symbol/Symbol.cpp | 8 +++
lldb/source/Symbol/SymbolContext.cpp | 71 ++
6 files changed, 197 insertions(+), 50 deletions(-)
diff --git a/lldb/include/lldb/Core/ModuleList.h
b/lldb/include/lldb/Core/ModuleList.h
index 43d931a8447406..171850e59baa35 100644
--- a/lldb/include/lldb/Core/ModuleList.h
+++ b/lldb/include/lldb/Core/ModuleList.h
@@ -287,6 +287,24 @@ class ModuleList {
const ModuleFunctionSearchOptions &options,
SymbolContextList &sc_list) const;
+ /// \see Module::FindFunctions ()
+ ///
+ /// \param[in] search_hint
+ /// If the value is NULL, then all modules will be searched in
+ /// order. If the value is a valid pointer and if a module is specified
in
+ /// the symbol context, that module will be searched first followed by
all
+ /// other modules in the list.
+ /// \param[in] partial_result_handler
+ /// A callback that will be called for each module in which at least one
+ /// match was found.
+ void FindFunctions(ConstString name, lldb::FunctionNameType name_type_mask,
+ const ModuleFunctionSearchOptions &options,
+ const SymbolContext *search_hint,
+ llvm::function_ref
+ partial_result_handler) const;
+
/// \see Module::FindFunctionSymbols ()
void FindFunctionSymbols(ConstString name,
lldb::FunctionNameType name_type_mask,
@@ -357,6 +375,31 @@ class ModuleList {
lldb::SymbolType symbol_type,
SymbolContextList &sc_list) const;
+ /// Find symbols by name and SymbolType.
+ ///
+ /// \param[in] name
+ /// A name of the symbol we are looking for.
+ ///
+ /// \param[in] symbol_type
+ /// A SymbolType the symbol we are looking for.
+ ///
+ /// \param[in] search_hint
+ /// If the value is NULL, then all modules will be searched in
+ /// order. If the value is a valid pointer and if a module is specified
in
+ /// the symbol context, that module will be searched first followed by
all
+ /// other modules in the list.
+ ///
+ /// \param[in] partial_result_handler
+ /// A callback that will be called for each module in which at least one
+ /// match was found.
+ void FindSymbolsWithNameAndType(ConstString name,
+ lldb::SymbolType symbol_type,
+ const SymbolContext *search_hint,
+ llvm::function_ref
+ partial_result_handler) const;
+
void FindSymbolsMatchingRegExAndType(const RegularExpression ®ex,
lldb::SymbolType symbol_type,
SymbolContextList &sc_list) const;
diff --git a/lldb/include/lldb/Symbol/SymbolContext.h
b/lldb/include/lldb/Symbol/SymbolContext.h
index 0bc707070f8504..66622b5c15f7c9 100644
--- a/lldb/include/lldb/Symbol/SymbolContext.h
+++ b/lldb/include/lldb/Symbol/SymbolContext.h
@@ -471,7 +471,7 @@ class SymbolContextList {
typedef AdaptedIterable
SymbolContextIterable;
- SymbolContextIterable SymbolContexts() {
+ SymbolContextIterable SymbolContexts() const {
return SymbolContextIterable(m_symbol_contexts);
}
};
diff --git a/lldb/source/Core/ModuleList.cpp b/lldb/source/Core/ModuleList.cpp
index b03490bacf9593..9b45334fb65a15 100644
--- a/lldb/source/Core/ModuleList.cpp
+++ b/lldb/source/Core/ModuleList.cpp
@@ -466,6 +466,41 @@ void ModuleList::FindFunctions(ConstString name,
}
}
+void ModuleList::FindFunctions(
+ConstString name, lldb::FunctionNameType name_type_mask,
+const ModuleFunctionSearchOptions &options,
+const SymbolContext *search_hint,
+llvm::function_ref
+partial_result_handler) const {
+ SymbolContextList sc_list_partial{};
+ auto FindInModule = [&](const ModuleSP &module) {
+module->FindFunctions(name, CompilerDeclContext(), name_type_mask, options,
+ sc_list_partial);
+if (!sc_list_partial.IsEmpty()) {
+ auto iteration_action = partial_result_handler(module, sc_list_partial);
+ sc_list_partial.Clear();
+ return iteration_action;
+}
+return IterationAction::Continue;
+ };
+
+ auto hinted_module = search_hint ? search
[Lldb-commits] [lldb] Remove redundant symbol lookups in IRExecutionUnit::FindInSymbols (PR #102835)
https://github.com/DmT021 updated
https://github.com/llvm/llvm-project/pull/102835
>From c0b9be1b4ef436bbf79bd3877a58e6b598b19940 Mon Sep 17 00:00:00 2001
From: Dmitrii Galimzianov
Date: Sun, 18 Aug 2024 04:36:19 +0200
Subject: [PATCH 1/2] Remove redundant symbol lookups in
IRExecutionUnit::FindInSymbols
---
lldb/include/lldb/Core/ModuleList.h| 43 +
lldb/include/lldb/Symbol/SymbolContext.h | 2 +-
lldb/source/Core/ModuleList.cpp| 68 +
lldb/source/Expression/IRExecutionUnit.cpp | 55 +
lldb/source/Symbol/Symbol.cpp | 8 +++
lldb/source/Symbol/SymbolContext.cpp | 71 ++
6 files changed, 197 insertions(+), 50 deletions(-)
diff --git a/lldb/include/lldb/Core/ModuleList.h
b/lldb/include/lldb/Core/ModuleList.h
index 43d931a8447406..171850e59baa35 100644
--- a/lldb/include/lldb/Core/ModuleList.h
+++ b/lldb/include/lldb/Core/ModuleList.h
@@ -287,6 +287,24 @@ class ModuleList {
const ModuleFunctionSearchOptions &options,
SymbolContextList &sc_list) const;
+ /// \see Module::FindFunctions ()
+ ///
+ /// \param[in] search_hint
+ /// If the value is NULL, then all modules will be searched in
+ /// order. If the value is a valid pointer and if a module is specified
in
+ /// the symbol context, that module will be searched first followed by
all
+ /// other modules in the list.
+ /// \param[in] partial_result_handler
+ /// A callback that will be called for each module in which at least one
+ /// match was found.
+ void FindFunctions(ConstString name, lldb::FunctionNameType name_type_mask,
+ const ModuleFunctionSearchOptions &options,
+ const SymbolContext *search_hint,
+ llvm::function_ref
+ partial_result_handler) const;
+
/// \see Module::FindFunctionSymbols ()
void FindFunctionSymbols(ConstString name,
lldb::FunctionNameType name_type_mask,
@@ -357,6 +375,31 @@ class ModuleList {
lldb::SymbolType symbol_type,
SymbolContextList &sc_list) const;
+ /// Find symbols by name and SymbolType.
+ ///
+ /// \param[in] name
+ /// A name of the symbol we are looking for.
+ ///
+ /// \param[in] symbol_type
+ /// A SymbolType the symbol we are looking for.
+ ///
+ /// \param[in] search_hint
+ /// If the value is NULL, then all modules will be searched in
+ /// order. If the value is a valid pointer and if a module is specified
in
+ /// the symbol context, that module will be searched first followed by
all
+ /// other modules in the list.
+ ///
+ /// \param[in] partial_result_handler
+ /// A callback that will be called for each module in which at least one
+ /// match was found.
+ void FindSymbolsWithNameAndType(ConstString name,
+ lldb::SymbolType symbol_type,
+ const SymbolContext *search_hint,
+ llvm::function_ref
+ partial_result_handler) const;
+
void FindSymbolsMatchingRegExAndType(const RegularExpression ®ex,
lldb::SymbolType symbol_type,
SymbolContextList &sc_list) const;
diff --git a/lldb/include/lldb/Symbol/SymbolContext.h
b/lldb/include/lldb/Symbol/SymbolContext.h
index 0bc707070f8504..66622b5c15f7c9 100644
--- a/lldb/include/lldb/Symbol/SymbolContext.h
+++ b/lldb/include/lldb/Symbol/SymbolContext.h
@@ -471,7 +471,7 @@ class SymbolContextList {
typedef AdaptedIterable
SymbolContextIterable;
- SymbolContextIterable SymbolContexts() {
+ SymbolContextIterable SymbolContexts() const {
return SymbolContextIterable(m_symbol_contexts);
}
};
diff --git a/lldb/source/Core/ModuleList.cpp b/lldb/source/Core/ModuleList.cpp
index b03490bacf9593..9b45334fb65a15 100644
--- a/lldb/source/Core/ModuleList.cpp
+++ b/lldb/source/Core/ModuleList.cpp
@@ -466,6 +466,41 @@ void ModuleList::FindFunctions(ConstString name,
}
}
+void ModuleList::FindFunctions(
+ConstString name, lldb::FunctionNameType name_type_mask,
+const ModuleFunctionSearchOptions &options,
+const SymbolContext *search_hint,
+llvm::function_ref
+partial_result_handler) const {
+ SymbolContextList sc_list_partial{};
+ auto FindInModule = [&](const ModuleSP &module) {
+module->FindFunctions(name, CompilerDeclContext(), name_type_mask, options,
+ sc_list_partial);
+if (!sc_list_partial.IsEmpty()) {
+ auto iteration_action = partial_result_handler(module, sc_list_partial);
+ sc_list_partial.Clear();
+ return iteration_action;
+}
+return IterationAction::Continue;
+ };
+
+ auto hinted_module = search_hint ? search
[Lldb-commits] [lldb] Remove redundant symbol lookups in IRExecutionUnit::FindInSymbols (PR #102835)
https://github.com/DmT021 updated
https://github.com/llvm/llvm-project/pull/102835
>From c0b9be1b4ef436bbf79bd3877a58e6b598b19940 Mon Sep 17 00:00:00 2001
From: Dmitrii Galimzianov
Date: Sun, 18 Aug 2024 04:36:19 +0200
Subject: [PATCH 1/2] Remove redundant symbol lookups in
IRExecutionUnit::FindInSymbols
---
lldb/include/lldb/Core/ModuleList.h| 43 +
lldb/include/lldb/Symbol/SymbolContext.h | 2 +-
lldb/source/Core/ModuleList.cpp| 68 +
lldb/source/Expression/IRExecutionUnit.cpp | 55 +
lldb/source/Symbol/Symbol.cpp | 8 +++
lldb/source/Symbol/SymbolContext.cpp | 71 ++
6 files changed, 197 insertions(+), 50 deletions(-)
diff --git a/lldb/include/lldb/Core/ModuleList.h
b/lldb/include/lldb/Core/ModuleList.h
index 43d931a8447406..171850e59baa35 100644
--- a/lldb/include/lldb/Core/ModuleList.h
+++ b/lldb/include/lldb/Core/ModuleList.h
@@ -287,6 +287,24 @@ class ModuleList {
const ModuleFunctionSearchOptions &options,
SymbolContextList &sc_list) const;
+ /// \see Module::FindFunctions ()
+ ///
+ /// \param[in] search_hint
+ /// If the value is NULL, then all modules will be searched in
+ /// order. If the value is a valid pointer and if a module is specified
in
+ /// the symbol context, that module will be searched first followed by
all
+ /// other modules in the list.
+ /// \param[in] partial_result_handler
+ /// A callback that will be called for each module in which at least one
+ /// match was found.
+ void FindFunctions(ConstString name, lldb::FunctionNameType name_type_mask,
+ const ModuleFunctionSearchOptions &options,
+ const SymbolContext *search_hint,
+ llvm::function_ref
+ partial_result_handler) const;
+
/// \see Module::FindFunctionSymbols ()
void FindFunctionSymbols(ConstString name,
lldb::FunctionNameType name_type_mask,
@@ -357,6 +375,31 @@ class ModuleList {
lldb::SymbolType symbol_type,
SymbolContextList &sc_list) const;
+ /// Find symbols by name and SymbolType.
+ ///
+ /// \param[in] name
+ /// A name of the symbol we are looking for.
+ ///
+ /// \param[in] symbol_type
+ /// A SymbolType the symbol we are looking for.
+ ///
+ /// \param[in] search_hint
+ /// If the value is NULL, then all modules will be searched in
+ /// order. If the value is a valid pointer and if a module is specified
in
+ /// the symbol context, that module will be searched first followed by
all
+ /// other modules in the list.
+ ///
+ /// \param[in] partial_result_handler
+ /// A callback that will be called for each module in which at least one
+ /// match was found.
+ void FindSymbolsWithNameAndType(ConstString name,
+ lldb::SymbolType symbol_type,
+ const SymbolContext *search_hint,
+ llvm::function_ref
+ partial_result_handler) const;
+
void FindSymbolsMatchingRegExAndType(const RegularExpression ®ex,
lldb::SymbolType symbol_type,
SymbolContextList &sc_list) const;
diff --git a/lldb/include/lldb/Symbol/SymbolContext.h
b/lldb/include/lldb/Symbol/SymbolContext.h
index 0bc707070f8504..66622b5c15f7c9 100644
--- a/lldb/include/lldb/Symbol/SymbolContext.h
+++ b/lldb/include/lldb/Symbol/SymbolContext.h
@@ -471,7 +471,7 @@ class SymbolContextList {
typedef AdaptedIterable
SymbolContextIterable;
- SymbolContextIterable SymbolContexts() {
+ SymbolContextIterable SymbolContexts() const {
return SymbolContextIterable(m_symbol_contexts);
}
};
diff --git a/lldb/source/Core/ModuleList.cpp b/lldb/source/Core/ModuleList.cpp
index b03490bacf9593..9b45334fb65a15 100644
--- a/lldb/source/Core/ModuleList.cpp
+++ b/lldb/source/Core/ModuleList.cpp
@@ -466,6 +466,41 @@ void ModuleList::FindFunctions(ConstString name,
}
}
+void ModuleList::FindFunctions(
+ConstString name, lldb::FunctionNameType name_type_mask,
+const ModuleFunctionSearchOptions &options,
+const SymbolContext *search_hint,
+llvm::function_ref
+partial_result_handler) const {
+ SymbolContextList sc_list_partial{};
+ auto FindInModule = [&](const ModuleSP &module) {
+module->FindFunctions(name, CompilerDeclContext(), name_type_mask, options,
+ sc_list_partial);
+if (!sc_list_partial.IsEmpty()) {
+ auto iteration_action = partial_result_handler(module, sc_list_partial);
+ sc_list_partial.Clear();
+ return iteration_action;
+}
+return IterationAction::Continue;
+ };
+
+ auto hinted_module = search_hint ? search
[Lldb-commits] [lldb] Remove redundant symbol lookups in IRExecutionUnit::FindInSymbols (PR #102835)
https://github.com/DmT021 updated
https://github.com/llvm/llvm-project/pull/102835
>From c647b26ad534bb998063722f930ddd07162bfee7 Mon Sep 17 00:00:00 2001
From: Dmitrii Galimzianov
Date: Sun, 18 Aug 2024 04:36:19 +0200
Subject: [PATCH] Remove redundant symbol lookups in
IRExecutionUnit::FindInSymbols
---
lldb/include/lldb/Core/ModuleList.h| 43
lldb/include/lldb/Symbol/SymbolContext.h | 2 +-
lldb/source/Core/ModuleList.cpp| 80 ++
lldb/source/Expression/IRExecutionUnit.cpp | 55 ---
lldb/source/Symbol/SymbolContext.cpp | 71 ---
5 files changed, 201 insertions(+), 50 deletions(-)
diff --git a/lldb/include/lldb/Core/ModuleList.h
b/lldb/include/lldb/Core/ModuleList.h
index 43d931a8447406..171850e59baa35 100644
--- a/lldb/include/lldb/Core/ModuleList.h
+++ b/lldb/include/lldb/Core/ModuleList.h
@@ -287,6 +287,24 @@ class ModuleList {
const ModuleFunctionSearchOptions &options,
SymbolContextList &sc_list) const;
+ /// \see Module::FindFunctions ()
+ ///
+ /// \param[in] search_hint
+ /// If the value is NULL, then all modules will be searched in
+ /// order. If the value is a valid pointer and if a module is specified
in
+ /// the symbol context, that module will be searched first followed by
all
+ /// other modules in the list.
+ /// \param[in] partial_result_handler
+ /// A callback that will be called for each module in which at least one
+ /// match was found.
+ void FindFunctions(ConstString name, lldb::FunctionNameType name_type_mask,
+ const ModuleFunctionSearchOptions &options,
+ const SymbolContext *search_hint,
+ llvm::function_ref
+ partial_result_handler) const;
+
/// \see Module::FindFunctionSymbols ()
void FindFunctionSymbols(ConstString name,
lldb::FunctionNameType name_type_mask,
@@ -357,6 +375,31 @@ class ModuleList {
lldb::SymbolType symbol_type,
SymbolContextList &sc_list) const;
+ /// Find symbols by name and SymbolType.
+ ///
+ /// \param[in] name
+ /// A name of the symbol we are looking for.
+ ///
+ /// \param[in] symbol_type
+ /// A SymbolType the symbol we are looking for.
+ ///
+ /// \param[in] search_hint
+ /// If the value is NULL, then all modules will be searched in
+ /// order. If the value is a valid pointer and if a module is specified
in
+ /// the symbol context, that module will be searched first followed by
all
+ /// other modules in the list.
+ ///
+ /// \param[in] partial_result_handler
+ /// A callback that will be called for each module in which at least one
+ /// match was found.
+ void FindSymbolsWithNameAndType(ConstString name,
+ lldb::SymbolType symbol_type,
+ const SymbolContext *search_hint,
+ llvm::function_ref
+ partial_result_handler) const;
+
void FindSymbolsMatchingRegExAndType(const RegularExpression ®ex,
lldb::SymbolType symbol_type,
SymbolContextList &sc_list) const;
diff --git a/lldb/include/lldb/Symbol/SymbolContext.h
b/lldb/include/lldb/Symbol/SymbolContext.h
index 0bc707070f8504..66622b5c15f7c9 100644
--- a/lldb/include/lldb/Symbol/SymbolContext.h
+++ b/lldb/include/lldb/Symbol/SymbolContext.h
@@ -471,7 +471,7 @@ class SymbolContextList {
typedef AdaptedIterable
SymbolContextIterable;
- SymbolContextIterable SymbolContexts() {
+ SymbolContextIterable SymbolContexts() const {
return SymbolContextIterable(m_symbol_contexts);
}
};
diff --git a/lldb/source/Core/ModuleList.cpp b/lldb/source/Core/ModuleList.cpp
index b03490bacf9593..f190679fcdf7c9 100644
--- a/lldb/source/Core/ModuleList.cpp
+++ b/lldb/source/Core/ModuleList.cpp
@@ -466,6 +466,48 @@ void ModuleList::FindFunctions(ConstString name,
}
}
+void ModuleList::FindFunctions(
+ConstString name, lldb::FunctionNameType name_type_mask,
+const ModuleFunctionSearchOptions &options,
+const SymbolContext *search_hint,
+llvm::function_ref
+partial_result_handler) const {
+ SymbolContextList sc_list_partial{};
+ auto FindInModule = [&](const ModuleSP &module) {
+if (!module)
+ return IterationAction::Continue;
+module->FindFunctions(name, CompilerDeclContext(), name_type_mask, options,
+ sc_list_partial);
+if (!sc_list_partial.IsEmpty()) {
+ auto iteration_action = partial_result_handler(module, sc_list_partial);
+ sc_list_partial.Clear();
+ return iteration_action;
+}
+return IterationAction::Continue;
+ };
+
+ std::vector modules_copy;
+ {
+std::loc
[Lldb-commits] [lldb] Remove redundant symbol lookups in IRExecutionUnit::FindInSymbols (PR #102835)
DmT021 wrote: On CI Linux builds some tests timed out. Probably deadlock, but I couldn't find where exactly. We now call an arbitrary function in Find* functions while the mutex is locked. I think LoadAddressResolver calls something that leads to deadlock. Although it's strange, because we use recursive_mutex in ModuleList. Had to make a defensive copy of the module vector in ModuleList. https://github.com/llvm/llvm-project/pull/102835 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Remove redundant symbol lookups in IRExecutionUnit::FindInSymbols (PR #102835)
DmT021 wrote: The tricky part is in the post-processing. IRExecutionUnit::FindInSymbols wants an external symbol and falls back to an internal one after all the modules are scanned. SymbolContext::FindBestGlobalDataSymbol prefers an internal symbol if it is found in the hinted module. Also resolution of a reexported symbol is part of the post-processing https://github.com/llvm/llvm-project/pull/102835 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Remove redundant symbol lookups in IRExecutionUnit::FindInSymbols (PR #102835)
DmT021 wrote: I'm not sure I understand because this will break the logic that is currently in place. Many nuances in post-processing wouldn't be possible to implement with a simple short-circuit. If a symbol is found in the hinted module, it might be internal. `IRExecutionUnit::FindInSymbols` processes the results by preferring external symbols over internal ones. If I short-circuit the search after the first match in the hinted module `IRExecutionUnit::FindInSymbols` just wouldn't see potentially more preferable results. On the other hand `SymbolContext::FindBestGlobalDataSymbol` prefers symbols of any type from the hinted module over the other symbols. But it still prefers external symbols over internal ones. So the priority order looks like this: `hint external > hint internal > other external > other internal`. Moreover, `SymbolContext::FindBestGlobalDataSymbol` verifies that there are no multiple matches found, but it's tricky as well. It checks if there's a result in the module - it does verification only matches from the module, otherwise it verifies all the results. Perhaps I can't explain all the nuances quite accurately, you should look at the post-processing code itself in these functions. But the most important thing is that we don't have a full understanding of when we can stop inside FindFunction/FindSymbolsWithNameAndType. https://github.com/llvm/llvm-project/pull/102835 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add logs for SymbolFileDWARF::FindTypes (PR #106030)
https://github.com/DmT021 created
https://github.com/llvm/llvm-project/pull/106030
`SymbolFileDWARF::FindTypes` was logged prior to [this
commit](https://github.com/llvm/llvm-project/commit/dd9587795811ba21e6ca6ad52b4531e17e6babd6#diff-edef3a65d5d569bbb75a4158d35b827aa5d42ee03ccd3b0c1d4f354afa12210c).
This is a helpful log message for checking for redundant type searches, so
maybe we should restore it?
>From 07a0426d27bc7f5fb9354e86c528626e3334714e Mon Sep 17 00:00:00 2001
From: Dmitrii Galimzianov
Date: Mon, 26 Aug 2024 05:10:50 +0200
Subject: [PATCH] Add logs for SymbolFileDWARF::FindTypes
---
.../Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index 7e0cf36d0de1b8..53f1276ee07f8b 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -2737,10 +2737,18 @@ void SymbolFileDWARF::FindTypes(const TypeQuery &query,
TypeResults &results) {
if (results.AlreadySearched(this))
return;
+ auto type_basename = query.GetTypeBasename();
+
+ if (Log *log = GetLog(DWARFLog::Lookups)) {
+GetObjectFile()->GetModule()->LogMessage(
+log, "SymbolFileDWARF::FindTypes(type_basename=\"{0}\")",
+type_basename);
+ }
+
std::lock_guard guard(GetModuleMutex());
bool have_index_match = false;
- m_index->GetTypes(query.GetTypeBasename(), [&](DWARFDIE die) {
+ m_index->GetTypes(type_basename, [&](DWARFDIE die) {
// Check the language, but only if we have a language filter.
if (query.HasLanguage()) {
if (!query.LanguageMatches(GetLanguageFamily(*die.GetCU(
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add logs for SymbolFileDWARF::FindTypes (PR #106030)
https://github.com/DmT021 ready_for_review https://github.com/llvm/llvm-project/pull/106030 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add logs for SymbolFileDWARF::FindTypes (PR #106030)
DmT021 wrote: @augusto2112 @clayborg please take a look https://github.com/llvm/llvm-project/pull/106030 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add logs for SymbolFileDWARF::FindTypes (PR #106030)
https://github.com/DmT021 updated
https://github.com/llvm/llvm-project/pull/106030
>From 9cc21bfd87a7836e9264ed6ef958ac5246c5ed53 Mon Sep 17 00:00:00 2001
From: Dmitrii Galimzianov
Date: Mon, 26 Aug 2024 15:52:17 +0200
Subject: [PATCH] Add logs for SymbolFileDWARF::FindTypes
---
.../SymbolFile/DWARF/SymbolFileDWARF.cpp | 32 ---
1 file changed, 28 insertions(+), 4 deletions(-)
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index 7e0cf36d0de1b8..8684e9f3f7e375 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -2737,10 +2737,19 @@ void SymbolFileDWARF::FindTypes(const TypeQuery &query,
TypeResults &results) {
if (results.AlreadySearched(this))
return;
+ auto type_basename = query.GetTypeBasename();
+
+ Log *log = GetLog(DWARFLog::Lookups);
+ if (log) {
+GetObjectFile()->GetModule()->LogMessage(
+log, "SymbolFileDWARF::FindTypes(type_basename=\"{0}\")",
+type_basename);
+ }
+
std::lock_guard guard(GetModuleMutex());
bool have_index_match = false;
- m_index->GetTypes(query.GetTypeBasename(), [&](DWARFDIE die) {
+ m_index->GetTypes(type_basename, [&](DWARFDIE die) {
// Check the language, but only if we have a language filter.
if (query.HasLanguage()) {
if (!query.LanguageMatches(GetLanguageFamily(*die.GetCU(
@@ -2779,7 +2788,19 @@ void SymbolFileDWARF::FindTypes(const TypeQuery &query,
TypeResults &results) {
return !results.Done(query); // Keep iterating if we aren't done.
});
- if (results.Done(query))
+ auto CheckIsDoneAndLog = [&results, &query, log, type_basename, this] {
+if (results.Done(query)) {
+ if (log) {
+GetObjectFile()->GetModule()->LogMessage(
+log, "SymbolFileDWARF::FindTypes(type_basename=\"{0}\") => {1}",
+type_basename, results.GetTypeMap().GetSize());
+ }
+ return true;
+}
+return false;
+ };
+
+ if (CheckIsDoneAndLog())
return;
// With -gsimple-template-names, a templated type's DW_AT_name will not
@@ -2834,7 +2855,7 @@ void SymbolFileDWARF::FindTypes(const TypeQuery &query,
TypeResults &results) {
}
return !results.Done(query); // Keep iterating if we aren't done.
});
- if (results.Done(query))
+ if (CheckIsDoneAndLog())
return;
}
}
@@ -2847,8 +2868,11 @@ void SymbolFileDWARF::FindTypes(const TypeQuery &query,
TypeResults &results) {
for (const auto &pair : m_external_type_modules) {
if (ModuleSP external_module_sp = pair.second) {
external_module_sp->FindTypes(query, results);
- if (results.Done(query))
+ if (results.Done(query)) {
+// don't use CheckIsDoneAndLog because the results are already logged
+// in the nested FindTypes call
return;
+ }
}
}
}
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add logs for SymbolFileDWARF::FindTypes (PR #106030)
DmT021 wrote: @Michael137 Ok, added another message after the search https://github.com/llvm/llvm-project/pull/106030 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add logs for SymbolFileDWARF::FindTypes (PR #106030)
https://github.com/DmT021 updated
https://github.com/llvm/llvm-project/pull/106030
>From a370fbb7b497eb12ca9faf9a760db3c023ee6052 Mon Sep 17 00:00:00 2001
From: Dmitrii Galimzianov
Date: Tue, 27 Aug 2024 01:32:49 +0200
Subject: [PATCH] Add logs for SymbolFileDWARF::FindTypes
---
.../SymbolFile/DWARF/SymbolFileDWARF.cpp | 39 ---
1 file changed, 33 insertions(+), 6 deletions(-)
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index 7e0cf36d0de1b8..d887d81912b27e 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -2737,10 +2737,19 @@ void SymbolFileDWARF::FindTypes(const TypeQuery &query,
TypeResults &results) {
if (results.AlreadySearched(this))
return;
+ auto type_basename = query.GetTypeBasename();
+
+ Log *log = GetLog(DWARFLog::Lookups);
+ if (log) {
+GetObjectFile()->GetModule()->LogMessage(
+log, "SymbolFileDWARF::FindTypes(type_basename=\"{0}\")",
+type_basename);
+ }
+
std::lock_guard guard(GetModuleMutex());
bool have_index_match = false;
- m_index->GetTypes(query.GetTypeBasename(), [&](DWARFDIE die) {
+ m_index->GetTypes(type_basename, [&](DWARFDIE die) {
// Check the language, but only if we have a language filter.
if (query.HasLanguage()) {
if (!query.LanguageMatches(GetLanguageFamily(*die.GetCU(
@@ -2779,8 +2788,14 @@ void SymbolFileDWARF::FindTypes(const TypeQuery &query,
TypeResults &results) {
return !results.Done(query); // Keep iterating if we aren't done.
});
- if (results.Done(query))
+ if (results.Done(query)) {
+if (log) {
+ GetObjectFile()->GetModule()->LogMessage(
+ log, "SymbolFileDWARF::FindTypes(type_basename=\"{0}\") => {1}",
+ type_basename, results.GetTypeMap().GetSize());
+}
return;
+ }
// With -gsimple-template-names, a templated type's DW_AT_name will not
// contain the template parameters. Try again stripping '<' and anything
@@ -2795,10 +2810,10 @@ void SymbolFileDWARF::FindTypes(const TypeQuery &query,
TypeResults &results) {
// it trims any context items down by removing template parameter names.
TypeQuery query_simple(query);
if (UpdateCompilerContextForSimpleTemplateNames(query_simple)) {
-
+ auto type_basename_simple = query_simple.GetTypeBasename();
// Copy our match's context and update the basename we are looking for
// so we can use this only to compare the context correctly.
- m_index->GetTypes(query_simple.GetTypeBasename(), [&](DWARFDIE die) {
+ m_index->GetTypes(type_basename_simple, [&](DWARFDIE die) {
// Check the language, but only if we have a language filter.
if (query.HasLanguage()) {
if (!query.LanguageMatches(GetLanguageFamily(*die.GetCU(
@@ -2834,8 +2849,17 @@ void SymbolFileDWARF::FindTypes(const TypeQuery &query,
TypeResults &results) {
}
return !results.Done(query); // Keep iterating if we aren't done.
});
- if (results.Done(query))
+ if (results.Done(query)) {
+if (log) {
+ GetObjectFile()->GetModule()->LogMessage(
+ log,
+ "SymbolFileDWARF::FindTypes(type_basename=\"{0}\") => {1} "
+ "(simplified as \"{2}\")",
+ type_basename, results.GetTypeMap().GetSize(),
+ type_basename_simple);
+}
return;
+ }
}
}
@@ -2847,8 +2871,11 @@ void SymbolFileDWARF::FindTypes(const TypeQuery &query,
TypeResults &results) {
for (const auto &pair : m_external_type_modules) {
if (ModuleSP external_module_sp = pair.second) {
external_module_sp->FindTypes(query, results);
- if (results.Done(query))
+ if (results.Done(query)) {
+// We don't log the results here as they are already logged in the
+// nested FindTypes call
return;
+ }
}
}
}
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add logs for SymbolFileDWARF::FindTypes (PR #106030)
@@ -2779,7 +2788,19 @@ void SymbolFileDWARF::FindTypes(const TypeQuery &query,
TypeResults &results) {
return !results.Done(query); // Keep iterating if we aren't done.
});
- if (results.Done(query))
+ auto CheckIsDoneAndLog = [&results, &query, log, type_basename, this] {
DmT021 wrote:
Alright, I added "(simplified as )" to the end of the message,
but if you prefer some other format let me know
https://github.com/llvm/llvm-project/pull/106030
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Remove redundant symbol lookups in IRExecutionUnit::FindInSymbols (PR #102835)
DmT021 wrote: > What happens if we stop preferring the external symbols over internal ones in > IRExecutionUnit::FindInSymbols? What tests break? It looks like there are no failing tests. https://github.com/llvm/llvm-project/pull/102835 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Remove redundant symbol lookups in IRExecutionUnit::FindInSymbols (PR #102835)
DmT021 wrote: > We should always prefer symbols from the current module first, probably > external first, then fall back to internal. If we do a search of all modules, > we should prefer external symbols first and then internal, but only if they > are unique. So it's `current module external, current module internal, other modules external, other modules internal`, right? This is how [SymbolContext.cpp](https://github.com/llvm/llvm-project/pull/102835/files#diff-da1a374f5098c39acfebae4b87a261f143a842e613082c2296de7ee28df12a33) implements it. What if we use `SymbolContext::FindBestGlobalDataSymbol` and `SymbolContext::FindBestFunction` (a new function*) in `IRExecutionUnit::FindInSymbols`? This way we can simplify LoadAddressResolver by removing all this logic about tracking internal symbols. \* `SymbolContext::FindBestFunction` will use the same lookup order but search functions instead of symbols. https://github.com/llvm/llvm-project/pull/102835 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Remove redundant symbol lookups in IRExecutionUnit::FindInSymbols (PR #102835)
DmT021 wrote: Nope, `LoadAddressResolver` has its own logic about `eSymbolTypeUndefined` that doesn't match `FindBestGlobalDataSymbol`. And it breaks `TestWeakSymbols.TestWeakSymbolsInExpressions.test_weak_symbol_in_expr`. At this point, I think it would be wiser to return to the original patch where I just remove module `module_sp` from a copy of `ModuleList`. https://github.com/llvm/llvm-project/pull/102835 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Remove redundant symbol lookups in IRExecutionUnit::FindInSymbols (PR #102835)
DmT021 wrote: > Don't let us hold this patch up if returning the the original patch that > fixes things makes things work if you can't easily get the same functionality > from this version of the patch. Will one part of this patch work with the new > stuff and the other part not? If so, maybe we allow the place that works use > the new functionality and just hard code the other one? The current version of the patch does archive the desired result (no redundant scan of the current module) and all the tests are green. But the issue is it's awfully complex. All this callback logic is hard to read and even harder to modify next time. Especially in `SymbolContext`. I'd prefer a slightly repetitive approach. At each lookup function (in this case `SymbolContext::FindBestGlobalDataSymbol` and `IRExecutionUnit::FindInSymbols`): - check the current module - if no suitable result is found: - get a copy of ModuleList and remove `current module` from it - search through the copy. But I don't insist; if you're ok with this implementation, I'm ok too. https://github.com/llvm/llvm-project/pull/102835 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] DynamicLoaderDarwin load images in parallel (PR #110439)
DmT021 wrote: @jasonmolenda If you still have a build with the patch can you please compare the unmodified version with the patched one with `enable-parallel-image-load` disabled? There shouldn't be a noticeable difference. If you don't it's ok, I haven't had time to inspect it yet but I'll try to dig into it today or tomorrow https://github.com/llvm/llvm-project/pull/110439 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] DynamicLoaderDarwin load images in parallel with preload (PR #110646)
https://github.com/DmT021 edited https://github.com/llvm/llvm-project/pull/110646 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] DynamicLoaderDarwin load images in parallel with preload (PR #110646)
DmT021 wrote: @jasonmolenda All the tests are green but I don't have the merge button. Can you merge it, please? https://github.com/llvm/llvm-project/pull/110646 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] DynamicLoaderDarwin load images in parallel with preload (PR #110646)
@@ -642,26 +652,86 @@ ModuleSP DynamicLoaderDarwin::GetDYLDModule() {
void DynamicLoaderDarwin::ClearDYLDModule() { m_dyld_module_wp.reset(); }
+template
+static std::vector parallel_map(
+llvm::ThreadPoolInterface &threadPool, InputIterator first,
+InputIterator last,
+llvm::function_ref::value_type &)>
+transform) {
+ const auto size = std::distance(first, last);
+ std::vector results(size);
+ if (size > 0) {
+llvm::ThreadPoolTaskGroup taskGroup(threadPool);
+auto it = first;
+for (ssize_t i = 0; i < size; ++i, ++it) {
+ taskGroup.async([&, i, it]() { results[i] = transform(*it); });
+}
+taskGroup.wait();
+ }
+ return results;
+}
DmT021 wrote:
We can but it may not be better. The implementations of map and parallel_map
aren't important from the business logic perspective. The only important thing
about PreloadModulesFromImageInfos is checking the setting.
Also, the code will be a bit more complicated than shown in the snippet you
provided:
- we are creating a task group and waiting for it even when we are going to
load the modules sequentially.
- we are checking the setting on each iteration of the loop.
This is probably almost free (I'm not sure if that's the case for
`task_group.wait()` though) but it's still a bit of an eyesore.
But a version of this without these two disadvantages will have a repeating for
loop. Something like
```
std::vector>
DynamicLoaderDarwin::PreloadModulesFromImageInfos(
const ImageInfo::collection &image_infos) {
const auto size = image_infos.size();
std::vector> images(
size);
auto lambda = [&](size_t i, ImageInfo::collection::const_iterator it) {
const auto &image_info = *it;
images[i] = std::make_pair(
image_info, FindTargetModuleForImageInfo(image_info, true, nullptr));
};
auto it = image_infos.begin();
bool is_parallel_load =
DynamicLoaderDarwinProperties::GetGlobal().GetEnableParallelImageLoad();
if (is_parallel_load) {
if (size > 0) {
llvm::ThreadPoolTaskGroup taskGroup(Debugger::GetThreadPool());
for (size_t i = 0; i < size; ++i, ++it) {
taskGroup.async(lambda, i, it);
}
taskGroup.wait();
}
} else {
for (size_t i = 0; i < size; ++i, ++it) {
lambda(i, it);
}
}
return images;
}
```
So now we have a bigger room for error in future refactoring here and the
important part (checking the setting) is less visible in the code as well.
Also, consider how would this code look like if had `std::transform` with
`ExecutionPolicy` available. I think it'd look about the same as the current
implementation with `map` and `parallel_map`.
https://github.com/llvm/llvm-project/pull/110646
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] DynamicLoaderDarwin load images in parallel with preload (PR #110646)
@@ -642,26 +652,86 @@ ModuleSP DynamicLoaderDarwin::GetDYLDModule() {
void DynamicLoaderDarwin::ClearDYLDModule() { m_dyld_module_wp.reset(); }
+template
+static std::vector parallel_map(
+llvm::ThreadPoolInterface &threadPool, InputIterator first,
+InputIterator last,
+llvm::function_ref::value_type &)>
+transform) {
+ const auto size = std::distance(first, last);
+ std::vector results(size);
+ if (size > 0) {
+llvm::ThreadPoolTaskGroup taskGroup(threadPool);
+auto it = first;
+for (ssize_t i = 0; i < size; ++i, ++it) {
+ taskGroup.async([&, i, it]() { results[i] = transform(*it); });
+}
+taskGroup.wait();
+ }
+ return results;
+}
DmT021 wrote:
@jasonmolenda @JDevlieghere I decided to inline the helpers. I still think we
are mixing important and utilitary code here and it's less than ideal. But at
the same time, I fully understand the readability issue. What's also important
here the signatures of the map functions aren't ideal to be considered "general
purpose helpers". Namely, they expect `std::distance` to be O(1) which isn't
always the case with the current template constraints. So I think if we are
going to use the same technique in other places (e.g.
`Target::SetExecutableModule` and other dynamic loader plugins) - we may want
to write better (more general) implementations for these helpers in some common
place.
PS Sorry for the long answer, it took me a while to come up with the decision
here.
https://github.com/llvm/llvm-project/pull/110646
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] DynamicLoaderDarwin load images in parallel (PR #110439)
https://github.com/DmT021 closed https://github.com/llvm/llvm-project/pull/110439 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] DynamicLoaderDarwin load images in parallel (PR #110439)
DmT021 wrote: I'm closing this in favor https://github.com/llvm/llvm-project/pull/110646 https://github.com/llvm/llvm-project/pull/110439 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] DynamicLoaderDarwin load images in parallel (PR #110439)
DmT021 wrote: > I was playing with the performance in a couple of different scenarios. For > some reason that I haven't looked into, we're getting less parallelism when > many of the binaries are in the shared cache in lldb. Maybe there is locking > around the code which finds the binary in lldb's own shared cache, so when 9 > threads try to do it at the same time, we have additional lock contention. There is a lock_guard for `shared_module_list.m_modules_mutex` at the beginning of `ModuleList::GetSharedModule`. I instrumented loading Slack and that lock_guard is responsible for about 7% of the total time. Most of the time is spent on constructing ConstString. Here's a flame graph. It's captured with "Record Waiting Threads" enabled so the locks are visible. [lldb_parallel.svg.zip](https://github.com/user-attachments/files/17337125/lldb_parallel.svg.zip) https://github.com/llvm/llvm-project/pull/110439 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] DynamicLoaderDarwin load images in parallel (PR #110439)
https://github.com/DmT021 edited https://github.com/llvm/llvm-project/pull/110439 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] DynamicLoaderDarwin load images in parallel (PR #110439)
https://github.com/DmT021 created
https://github.com/llvm/llvm-project/pull/110439
When `plugin.dynamic-loader.darwin.enable-parallel-image-load` is enabled
`DynamicLoaderDarwin::AddModulesUsingImageInfos` will load images in parallel
using the thread pool.
>From d6183d6b0e1c755d6adf3f52463742e50a234f16 Mon Sep 17 00:00:00 2001
From: Dmitrii Galimzianov
Date: Sun, 29 Sep 2024 22:24:19 +0200
Subject: [PATCH] DynamicLoaderDarwin load images in parallel
---
.../DynamicLoader/MacOSX-DYLD/CMakeLists.txt | 12 +++
.../MacOSX-DYLD/DynamicLoaderDarwin.cpp | 97 ++-
.../MacOSX-DYLD/DynamicLoaderDarwin.h | 11 +++
.../DynamicLoaderDarwinProperties.td | 8 ++
.../MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp | 8 +-
.../MacOSX-DYLD/DynamicLoaderMacOSXDYLD.h | 2 +
6 files changed, 132 insertions(+), 6 deletions(-)
create mode 100644
lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwinProperties.td
diff --git a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/CMakeLists.txt
b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/CMakeLists.txt
index 7308374c8bfba6..b53699b31252ab 100644
--- a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/CMakeLists.txt
+++ b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/CMakeLists.txt
@@ -1,3 +1,11 @@
+lldb_tablegen(DynamicLoaderDarwinProperties.inc -gen-lldb-property-defs
+ SOURCE DynamicLoaderDarwinProperties.td
+ TARGET LLDBPluginDynamicLoaderDarwinPropertiesGen)
+
+lldb_tablegen(DynamicLoaderDarwinPropertiesEnum.inc
-gen-lldb-property-enum-defs
+ SOURCE DynamicLoaderDarwinProperties.td
+ TARGET LLDBPluginDynamicLoaderDarwinPropertiesEnumGen)
+
add_lldb_library(lldbPluginDynamicLoaderMacOSXDYLD PLUGIN
DynamicLoaderMacOSXDYLD.cpp
DynamicLoaderMacOS.cpp
@@ -16,3 +24,7 @@ add_lldb_library(lldbPluginDynamicLoaderMacOSXDYLD PLUGIN
Support
TargetParser
)
+
+add_dependencies(lldbPluginDynamicLoaderMacOSXDYLD
+ LLDBPluginDynamicLoaderDarwinPropertiesGen
+ LLDBPluginDynamicLoaderDarwinPropertiesEnumGen)
diff --git
a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
index 3863b6b3520db4..b085ccb4fcaa42 100644
--- a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
+++ b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
@@ -34,6 +34,7 @@
#include "Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h"
#include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
+#include "llvm/Support/ThreadPool.h"
//#define ENABLE_DEBUG_PRINTF // COMMENT THIS LINE OUT PRIOR TO CHECKIN
#ifdef ENABLE_DEBUG_PRINTF
@@ -48,6 +49,36 @@
using namespace lldb;
using namespace lldb_private;
+#define LLDB_PROPERTIES_dynamicloaderdarwin
+#include "DynamicLoaderDarwinProperties.inc"
+
+enum {
+#define LLDB_PROPERTIES_dynamicloaderdarwin
+#include "DynamicLoaderDarwinPropertiesEnum.inc"
+};
+
+ConstString &DynamicLoaderDarwinProperties::GetSettingName() {
+ static ConstString g_setting_name("darwin");
+ return g_setting_name;
+}
+
+DynamicLoaderDarwinProperties::DynamicLoaderDarwinProperties() : Properties() {
+ m_collection_sp = std::make_shared(GetSettingName());
+ m_collection_sp->Initialize(g_dynamicloaderdarwin_properties);
+}
+
+bool DynamicLoaderDarwinProperties::GetEnableParallelImageLoad() const {
+ return GetPropertyAtIndexAs(
+ ePropertyEnableParallelImageLoad,
+ g_dynamicloaderdarwin_properties[ePropertyEnableParallelImageLoad]
+ .default_uint_value != 0);
+}
+
+DynamicLoaderDarwinProperties &DynamicLoaderDarwinProperties::GetGlobal() {
+ static DynamicLoaderDarwinProperties g_settings;
+ return g_settings;
+}
+
// Constructor
DynamicLoaderDarwin::DynamicLoaderDarwin(Process *process)
: DynamicLoader(process), m_dyld_module_wp(), m_libpthread_module_wp(),
@@ -77,6 +108,17 @@ void DynamicLoaderDarwin::DidLaunch() {
SetNotificationBreakpoint();
}
+void DynamicLoaderDarwin::CreateSettings(lldb_private::Debugger &debugger) {
+ if (!PluginManager::GetSettingForDynamicLoaderPlugin(
+ debugger, DynamicLoaderDarwinProperties::GetSettingName())) {
+const bool is_global_setting = true;
+PluginManager::CreateSettingForDynamicLoaderPlugin(
+debugger,
+DynamicLoaderDarwinProperties::GetGlobal().GetValueProperties(),
+"Properties for the DynamicLoaderDarwin plug-in.", is_global_setting);
+ }
+}
+
// Clear out the state of this class.
void DynamicLoaderDarwin::Clear(bool clear_process) {
std::lock_guard guard(m_mutex);
@@ -640,6 +682,41 @@ ModuleSP DynamicLoaderDarwin::GetDYLDModule() {
void DynamicLoaderDarwin::ClearDYLDModule() { m_dyld_module_wp.reset(); }
+template
+std::vector
+parallel_map(llvm::ThreadPoolInterface &threadPool, InputIterator first,
+ InputIterator last,
+ llvm::function_ref::value_type &)>
+ transform) {
+ const auto size = std::distance(first, last)
[Lldb-commits] [lldb] DynamicLoaderDarwin load images in parallel (PR #110439)
DmT021 wrote: @augusto2112 Take a look when you have time This is just one possible approach to parallelizing the initial image loading. The other solution I'm checking now is to parallelize loading somewhere earlier, perhaps in `DynamicLoaderMacOS::DoInitialImageFetch`. The difference is that the `UpdateSpecialBinariesFromNewImageInfos` function isn't parallelized right now, so we still load `dyld` and the main executable image sequentially. If we parallelize the loading of all `image_infos` before calling `UpdateSpecialBinariesFromNewImageInfos` and `AddModulesUsingImageInfos` we might gain even better utilization of multithreading. https://github.com/llvm/llvm-project/pull/110439 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] DynamicLoaderDarwin load images in parallel (PR #110439)
DmT021 wrote: > my main concern was that all of the Modules would be trying to add strings to > the constant string pool and lock contention could become a bottleneck. Yeah it sure would be a bottleneck. I didn't measure it precisely but I think I saw something about 30-50% of the time is spent on the mutexes in the string pools. And this is one of the reasons I gated the parallelized implementation behind the flag. One possible approach here would be to split the work done in the `ObjectFileMachO::ParseSymtab` into 3 phases: parsing, `ConstString` creation, and assigning the string to the symbols. So we can create all of the strings as one batch and minimize the locking penalty. `ParseSymtab` is a funny piece of code though and deserves its own pull request to address such things. > Built RelWithDebInfo, unmodified lldb took 4.5 seconds. Parallel took 6.6 > seconds. Parallel with preload took 6.7 seconds. > > Built Debug, unmodified lldb took 27.6 seconds. Parallel took 35.5 seconds. > Parallel plus preload took 35.6 seconds. Oh wow. 4.5 sec is amazingly fast. I'll try to reproduce your results. > I'm curious what your machine/target process looks like, that we're seeing > such different numbers. I'm guessing you were testing an unoptimized build > given the time amounts. Does it look like I missed something with my test > case? The machine is MBP M1 10 cores/32 GB. I'm testing this patch on [the swift fork](https://github.com/swiftlang/llvm-project/tree/stable/20230725) built in the `Release` mode and I'm attaching it to an iOS app(in the simulator) that has 978 modules. I'm running `lldb --local-lldbinit` in Instruments app with the following lldbinit file: ``` settings set plugin.dynamic-loader.darwin.enable-parallel-image-load 1 process attach --name browser_corp_ios continue ``` https://github.com/llvm/llvm-project/pull/110439 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] DynamicLoaderDarwin load images in parallel (PR #110439)
DmT021 wrote: Nice. Also there's no significant difference between `build.release` and `release.parallel-with-preload` with sequential load which is good. I wonder why your parallel runs are slightly less performant than mine relative to the sequential ones. I mean it's almost certainly the contention for the string pools' mutexes. But I still wonder why exactly. https://github.com/llvm/llvm-project/pull/110439 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] DynamicLoaderDarwin load images in parallel (PR #110439)
DmT021 wrote: Anyway, my main goal is iOS apps running in the simulator. And for them, the speed-up is much more noticeable (at least for big apps). Let me know if you'd like me to measure something else. https://github.com/llvm/llvm-project/pull/110439 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] DynamicLoaderDarwin load images in parallel (PR #110439)
DmT021 wrote: @jasonmolenda I tried to reproduce your results but got drastically different numbers for parallel runs. Here's the setup: - Using this (the main) repository of llvm-project. - `cmake -G Ninja -DLLVM_ENABLE_PROJECTS="clang;lldb;lld" -DCMAKE_BUILD_TYPE=RelWithDebInfo -DLLVM_ENABLE_ASSERTIONS=ON -DLLVM_ENABLE_RUNTIMES="libcxx;libcxxabi;libunwind" -DLLDB_BUILD_FRAMEWORK=1 -DLLDB_ENABLE_PYTHON=1 -DLLDB_ENABLE_SWIG=1` - `csrutil enable --without debug` - Attaching to Slack - Using the same cmd line as you did except for the setting name `time ~/w/swift-project/build/lldb-main/bin/lldb -x -b -o 'pro att -n Slack' -o 'det'` `time ~/w/swift-project/build/lldb-main/bin/lldb -x -b -O 'settings set plugin.dynamic-loader.darwin.enable-parallel-image-load true' -o 'pro att -n Slack' -o 'det'` - **Important** I always discard the first run in a sequence because the first launch of `lldb` after a rebuild takes about 3-4 seconds longer than the subsequent ones. For example, the first run of `#110439 enable-parallel-image-load=0` took 8.772 sec instead of ~5.4. | | unmodified | #110439 enable-parallel-image-load=0 | #110439 enable-parallel-image-load=1 | #110646 enable-parallel-image-load=0 | #110646 enable-parallel-image-load=1 | |---|---|---|---|---|---| | run 1 | 5.52 | 5.509 | 2.517 | 5.464 | 2.496 | | run 2 | 5.475 | 5.451 | 2.492 | 5.372 | 2.506 | | run 3 | 5.373 | 5.44 | 2.513 | 5.506 | 2.533 | | run 4 | 5.55 | 5.333 | 2.54 | 5.493 | 2.552 | | run 5 | 5.534 | 5.535 | 2.511 | 5.427 | 2.552 | | avg | 5.4904 | 5.4536 | 2.5146 | 5.4524 | 2.5278 | Did you discard the first runs as well? https://github.com/llvm/llvm-project/pull/110439 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Remove redundant symbol lookups in IRExecutionUnit::FindInSymbols (PR #102835)
DmT021 wrote: @Michael137 I don't have merge rights, can you merge this pls? https://github.com/llvm/llvm-project/pull/102835 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Remove redundant symbol lookups in IRExecutionUnit::FindInSymbols (PR #102835)
DmT021 wrote: @augusto2112 Thank you! https://github.com/llvm/llvm-project/pull/102835 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] DynamicLoaderDarwin load images in parallel (PR #110439)
DmT021 wrote: > it should definitely be enabled by default when we're at the point to merge > it, and the setting should only be a safety mechanism if this turns out to > cause a problem for a configuration we weren't able to test. That's fine with me either. > I would even put it explicitly under an experimental node (e.g. see > target.experimental.inject-local-vars we have currently) so if someone does > disable it in their ~/.lldbinit file, and we remove the setting in a year or > two, they won't get errors starting lldb, it will be silently ignored. Sure, but can you clarify please should it be named `plugin.dynamic-loader.darwin.experimental.enable-parallel-image-load` or `plugin.experimental.dynamic-loader.darwin.enable-parallel-image-load` as you wrote previously? Seems like the first option reads as "'enable-parallel-image-load' is an experimental property of 'plugin.experimental.dynamic-loader.darwin'" and the second one as "'dynamic-loader.darwin.enable-parallel-image-load' is an experimental property of 'plugin'". https://github.com/llvm/llvm-project/pull/110439 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] DynamicLoaderDarwin load images in parallel (PR #110439)
DmT021 wrote: > I was chatting with Jim Ingham and he was a little bummed that we're looking > at doing this in a single DynamicLoader plugin, instead of having the > DynamicLoader plugin create a list of ModuleSpec's and having a central > method in ModuleList or something, create Modules for each of them via a > thread pool, and then the DynamicLoader plugin would set the section load > addresses in the Target, run any scripting resources (python in .dSYMs), call > ModulesDidLoad etc. I'm looking at the implementation of `DynamicLoaderDarwin::FindTargetModuleForImageInfo` and to me it seems like moving the parallelization into a common place (let's say `Target::GetOrCreateImages`) would give us significant complexity. We can't just make a module_spec and call a method to load it for us, there should be either a complicated description of what to load or some kind of back-and-forth communication between `GetOrCreateImages` and a dynamic loader. For darwin: 1) create a module_spec 2) check if there's an image in the target images that matches it already 3) if it's not, and if we can use the shared cache, make a new module_spec and try to load a module with it 4) if it still fails, try to load it with the original module_spec 5) if it still fails, try to load it with `m_process->ReadModuleFromMemory`. https://github.com/llvm/llvm-project/pull/110439 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] DynamicLoaderDarwin load images in parallel (PR #110439)
DmT021 wrote: > Was the setting intended for testing purposes only, or did you intend to > include that in a final PR? The latter. IMO the risks involved by parallelization are a bit too high to do it without a flag. I'm even thinking about having it opt-in rather than opt-out for some time. https://github.com/llvm/llvm-project/pull/110439 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Remove redundant symbol lookups in IRExecutionUnit::FindInSymbols (PR #102835)
@@ -785,38 +785,50 @@ IRExecutionUnit::FindInSymbols(const
std::vector &names,
return LLDB_INVALID_ADDRESS;
}
+ ModuleList images = target->GetImages();
+ // We'll process module_sp separately, before the other modules.
+ images.Remove(sc.module_sp);
+
LoadAddressResolver resolver(target, symbol_was_missing_weak);
ModuleFunctionSearchOptions function_options;
function_options.include_symbols = true;
function_options.include_inlines = false;
for (const ConstString &name : names) {
+// The lookup order here is as follows:
+// 1) Functions in `sc.module_sp`
+// 2) Functions in the other modules
+// 3) Symbols in `sc.module_sp`
+// 4) Symbols in the other modules
+SymbolContextList sc_list;
if (sc.module_sp) {
- SymbolContextList sc_list;
sc.module_sp->FindFunctions(name, CompilerDeclContext(),
lldb::eFunctionNameTypeFull,
function_options,
sc_list);
if (auto load_addr = resolver.Resolve(sc_list))
return *load_addr;
+ sc_list.Clear();
DmT021 wrote:
I remembered why it was calling `Clear()` - to avoid repetitive allocations of
the internal vector in `SymbolContextList`.
https://github.com/llvm/llvm-project/pull/102835
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] DynamicLoaderDarwin load images in parallel with preload (PR #110646)
https://github.com/DmT021 ready_for_review https://github.com/llvm/llvm-project/pull/110646 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] DynamicLoaderDarwin load images in parallel with preload (PR #110646)
https://github.com/DmT021 updated
https://github.com/llvm/llvm-project/pull/110646
>From 64a4d69d1bb3f32c4b303d6f45b45cdf513183c2 Mon Sep 17 00:00:00 2001
From: Dmitrii Galimzianov
Date: Sun, 29 Sep 2024 22:24:19 +0200
Subject: [PATCH 1/2] DynamicLoaderDarwin load images in parallel
---
.../DynamicLoader/MacOSX-DYLD/CMakeLists.txt | 13
.../MacOSX-DYLD/DynamicLoaderDarwin.cpp | 71 +--
.../MacOSX-DYLD/DynamicLoaderDarwin.h | 4 +-
.../DynamicLoaderDarwinProperties.cpp | 53 ++
.../DynamicLoaderDarwinProperties.h | 34 +
.../DynamicLoaderDarwinProperties.td | 8 +++
.../MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp | 8 ++-
.../MacOSX-DYLD/DynamicLoaderMacOSXDYLD.h | 2 +
8 files changed, 185 insertions(+), 8 deletions(-)
create mode 100644
lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwinProperties.cpp
create mode 100644
lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwinProperties.h
create mode 100644
lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwinProperties.td
diff --git a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/CMakeLists.txt
b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/CMakeLists.txt
index 7308374c8bfba6..77a560541fcb1f 100644
--- a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/CMakeLists.txt
+++ b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/CMakeLists.txt
@@ -1,7 +1,16 @@
+lldb_tablegen(DynamicLoaderDarwinProperties.inc -gen-lldb-property-defs
+ SOURCE DynamicLoaderDarwinProperties.td
+ TARGET LLDBPluginDynamicLoaderDarwinPropertiesGen)
+
+lldb_tablegen(DynamicLoaderDarwinPropertiesEnum.inc
-gen-lldb-property-enum-defs
+ SOURCE DynamicLoaderDarwinProperties.td
+ TARGET LLDBPluginDynamicLoaderDarwinPropertiesEnumGen)
+
add_lldb_library(lldbPluginDynamicLoaderMacOSXDYLD PLUGIN
DynamicLoaderMacOSXDYLD.cpp
DynamicLoaderMacOS.cpp
DynamicLoaderDarwin.cpp
+ DynamicLoaderDarwinProperties.cpp
LINK_LIBS
lldbBreakpoint
@@ -16,3 +25,7 @@ add_lldb_library(lldbPluginDynamicLoaderMacOSXDYLD PLUGIN
Support
TargetParser
)
+
+add_dependencies(lldbPluginDynamicLoaderMacOSXDYLD
+ LLDBPluginDynamicLoaderDarwinPropertiesGen
+ LLDBPluginDynamicLoaderDarwinPropertiesEnumGen)
diff --git
a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
index 30242038a5f66f..3d57a10a196c11 100644
--- a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
+++ b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
@@ -8,6 +8,7 @@
#include "DynamicLoaderDarwin.h"
+#include "DynamicLoaderDarwinProperties.h"
#include "lldb/Breakpoint/StoppointCallbackContext.h"
#include "lldb/Core/Debugger.h"
#include "lldb/Core/Module.h"
@@ -31,6 +32,7 @@
#include "lldb/Utility/LLDBLog.h"
#include "lldb/Utility/Log.h"
#include "lldb/Utility/State.h"
+#include "llvm/Support/ThreadPool.h"
#include "Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h"
#include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
@@ -77,6 +79,17 @@ void DynamicLoaderDarwin::DidLaunch() {
SetNotificationBreakpoint();
}
+void DynamicLoaderDarwin::CreateSettings(lldb_private::Debugger &debugger) {
+ if (!PluginManager::GetSettingForDynamicLoaderPlugin(
+ debugger, DynamicLoaderDarwinProperties::GetSettingName())) {
+const bool is_global_setting = true;
+PluginManager::CreateSettingForDynamicLoaderPlugin(
+debugger,
+DynamicLoaderDarwinProperties::GetGlobal().GetValueProperties(),
+"Properties for the DynamicLoaderDarwin plug-in.", is_global_setting);
+ }
+}
+
// Clear out the state of this class.
void DynamicLoaderDarwin::Clear(bool clear_process) {
std::lock_guard guard(m_mutex);
@@ -88,7 +101,7 @@ void DynamicLoaderDarwin::Clear(bool clear_process) {
}
ModuleSP DynamicLoaderDarwin::FindTargetModuleForImageInfo(
-ImageInfo &image_info, bool can_create, bool *did_create_ptr) {
+const ImageInfo &image_info, bool can_create, bool *did_create_ptr) {
if (did_create_ptr)
*did_create_ptr = false;
@@ -642,6 +655,41 @@ ModuleSP DynamicLoaderDarwin::GetDYLDModule() {
void DynamicLoaderDarwin::ClearDYLDModule() { m_dyld_module_wp.reset(); }
+template
+static std::vector parallel_map(
+llvm::ThreadPoolInterface &threadPool, InputIterator first,
+InputIterator last,
+llvm::function_ref::value_type &)>
+transform) {
+ const auto size = std::distance(first, last);
+ std::vector results(size);
+ if (size > 0) {
+llvm::ThreadPoolTaskGroup taskGroup(threadPool);
+auto it = first;
+for (ssize_t i = 0; i < size; ++i, ++it) {
+ taskGroup.async([&, i, it]() { results[i] = transform(*it); });
+}
+taskGroup.wait();
+ }
+ return results;
+}
+
+template
+static std::vector
+map(InputIterator first, InputIterator last,
+llvm::function_ref::value_ty
[Lldb-commits] [lldb] DynamicLoaderDarwin load images in parallel (PR #110439)
DmT021 wrote: > I think that's where the real perf difference you were measuring kicked in > (you showed a 10.5 second versus 13.4 second time difference for preload > versus parallel in the beginning), do I have that right? That's right. > Do you prefer the non-preload approach? No, I think we should stick to a more general approach unless we have to perform the loading itself differently for the special modules. And currently, we don't have any difference between loading them and all the other - the calls to `FindTargetModuleForImageInfo` are all the same. So I'm in favor of the preload variant. https://github.com/llvm/llvm-project/pull/110439 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Create dependent modules in parallel (PR #114507)
@@ -1608,9 +1612,28 @@ void Target::SetExecutableModule(ModuleSP &executable_sp,
if (image_module_sp) {
added_modules.AppendIfNeeded(image_module_sp, false);
ObjectFile *objfile = image_module_sp->GetObjectFile();
- if (objfile)
+ if (objfile) {
+std::lock_guard guard(dependent_files_mutex);
objfile->GetDependentModules(dependent_files);
DmT021 wrote:
Just in case, did you check for a regression in performance after the latest
patch?
https://github.com/llvm/llvm-project/pull/114507
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Create dependent modules in parallel (PR #114507)
@@ -1608,9 +1612,28 @@ void Target::SetExecutableModule(ModuleSP &executable_sp,
if (image_module_sp) {
added_modules.AppendIfNeeded(image_module_sp, false);
ObjectFile *objfile = image_module_sp->GetObjectFile();
- if (objfile)
+ if (objfile) {
+std::lock_guard guard(dependent_files_mutex);
objfile->GetDependentModules(dependent_files);
DmT021 wrote:
I wonder if this operation is heavy in any of the `ObjectFile` implementations.
If it is we may want to lock the mutex only when an actual append to the
dependent_files happens.
https://github.com/llvm/llvm-project/pull/114507
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Remove redundant symbol lookups in IRExecutionUnit::FindInSymbols (PR #102835)
https://github.com/DmT021 updated
https://github.com/llvm/llvm-project/pull/102835
>From 424b3b74d54b10067155f0ceaeec80c78d2e6aab Mon Sep 17 00:00:00 2001
From: Dmitrii Galimzianov
Date: Tue, 24 Sep 2024 00:33:43 +0200
Subject: [PATCH] Remove redundant symbol lookups in
IRExecutionUnit::FindInSymbols
---
lldb/source/Expression/IRExecutionUnit.cpp | 36 ++
1 file changed, 24 insertions(+), 12 deletions(-)
diff --git a/lldb/source/Expression/IRExecutionUnit.cpp
b/lldb/source/Expression/IRExecutionUnit.cpp
index f220704423627d..d86f63edb9759e 100644
--- a/lldb/source/Expression/IRExecutionUnit.cpp
+++ b/lldb/source/Expression/IRExecutionUnit.cpp
@@ -785,6 +785,10 @@ IRExecutionUnit::FindInSymbols(const
std::vector &names,
return LLDB_INVALID_ADDRESS;
}
+ ModuleList images = target->GetImages();
+ // We'll process module_sp separately, before the other modules.
+ images.Remove(sc.module_sp);
+
LoadAddressResolver resolver(target, symbol_was_missing_weak);
ModuleFunctionSearchOptions function_options;
@@ -792,31 +796,39 @@ IRExecutionUnit::FindInSymbols(const
std::vector &names,
function_options.include_inlines = false;
for (const ConstString &name : names) {
+// The lookup order here is as follows:
+// 1) Functions in `sc.module_sp`
+// 2) Functions in the other modules
+// 3) Symbols in `sc.module_sp`
+// 4) Symbols in the other modules
+SymbolContextList sc_list;
if (sc.module_sp) {
- SymbolContextList sc_list;
sc.module_sp->FindFunctions(name, CompilerDeclContext(),
lldb::eFunctionNameTypeFull,
function_options,
sc_list);
if (auto load_addr = resolver.Resolve(sc_list))
return *load_addr;
+ sc_list.Clear();
}
-if (sc.target_sp) {
- SymbolContextList sc_list;
- sc.target_sp->GetImages().FindFunctions(name,
lldb::eFunctionNameTypeFull,
- function_options, sc_list);
- if (auto load_addr = resolver.Resolve(sc_list))
-return *load_addr;
-}
+images.FindFunctions(name, lldb::eFunctionNameTypeFull, function_options,
+ sc_list);
+if (auto load_addr = resolver.Resolve(sc_list))
+ return *load_addr;
+sc_list.Clear();
-if (sc.target_sp) {
- SymbolContextList sc_list;
- sc.target_sp->GetImages().FindSymbolsWithNameAndType(
- name, lldb::eSymbolTypeAny, sc_list);
+if (sc.module_sp) {
+ sc.module_sp->FindSymbolsWithNameAndType(name, lldb::eSymbolTypeAny,
+ sc_list);
if (auto load_addr = resolver.Resolve(sc_list))
return *load_addr;
+ sc_list.Clear();
}
+images.FindSymbolsWithNameAndType(name, lldb::eSymbolTypeAny, sc_list);
+if (auto load_addr = resolver.Resolve(sc_list))
+ return *load_addr;
+
lldb::addr_t best_internal_load_address =
resolver.GetBestInternalLoadAddress();
if (best_internal_load_address != LLDB_INVALID_ADDRESS)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Remove redundant symbol lookups in IRExecutionUnit::FindInSymbols (PR #102835)
DmT021 wrote: @clayborg @Michael137 I've reverted the changes here and rewrote this patch pretty much like it was originally presented with a slightly different order of operation. The only change is in the `IRExecutionUnit::FindInSymbols` function and the order of lookup is described in the comment [here](https://github.com/llvm/llvm-project/pull/102835/files#diff-717a850c4315286c025e2739ebe9dacbf27e626b7679c72479b05c996d721112R799). If you don't mind let's merge this variant. In my opinion, [the variant with lambda](https://github.com/llvm/llvm-project/commit/c647b26ad534bb998063722f930ddd07162bfee7) is too complex for a little win and I'm afraid future changes in this logic will be even more complicated. https://github.com/llvm/llvm-project/pull/102835 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Remove redundant symbol lookups in IRExecutionUnit::FindInSymbols (PR #102835)
DmT021 wrote: ping? https://github.com/llvm/llvm-project/pull/102835 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Remove redundant symbol lookups in IRExecutionUnit::FindInSymbols (PR #102835)
DmT021 wrote: > Why is it so expensive to perform consecutive lookups into the same module > for the same name? It depends on circumstances but basically, the worst-case scenario is when you: 1) lookup a symbol in the context of a huge module 2) the symbol is in another module 3) the huge module appears before the other module in the ModuleList For example, I have a project that actively links its dependencies statically when possible, so the total number of symbols in the main executable is enormous(order of 10^6). When I perform a lookup for a foreign symbol (let's say `swift_retain`) in the context of this module I'll always search through this huge module twice. And this is probably quite common in practice. One will likely evaluate an expression while standing on a breakpoint in the main executable, and `swift_retain` will likely be looked up for any expression evaluation, and `libswiftCore` will appear after the main executable in the module list. > Should that be perhaps where we should focus the performance investigation > on? So users don't need to care about how many times we're doing the lookup? I was thinking about it and these two optimizations in particular: 1) Swift symbols contain module names. So we probably can infer a pretty good guess about the module from the symbol itself. 2) We probably can build an index of all the symbols in all the modules. But keeping this index in sync with the target's module list doesn't seem to be an easy task. Both of these approaches are of the scope of this change though. https://github.com/llvm/llvm-project/pull/102835 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Remove redundant symbol lookups in IRExecutionUnit::FindInSymbols (PR #102835)
https://github.com/DmT021 updated
https://github.com/llvm/llvm-project/pull/102835
>From aad4d1c669f84db462454b96e42fd682ea818720 Mon Sep 17 00:00:00 2001
From: Dmitrii Galimzianov
Date: Tue, 24 Sep 2024 00:33:43 +0200
Subject: [PATCH] Remove redundant symbol lookups in
IRExecutionUnit::FindInSymbols
---
lldb/source/Expression/IRExecutionUnit.cpp | 29 +-
1 file changed, 23 insertions(+), 6 deletions(-)
diff --git a/lldb/source/Expression/IRExecutionUnit.cpp
b/lldb/source/Expression/IRExecutionUnit.cpp
index f220704423627d..e6aa8978fe8c34 100644
--- a/lldb/source/Expression/IRExecutionUnit.cpp
+++ b/lldb/source/Expression/IRExecutionUnit.cpp
@@ -785,6 +785,10 @@ IRExecutionUnit::FindInSymbols(const
std::vector &names,
return LLDB_INVALID_ADDRESS;
}
+ ModuleList non_local_images = target->GetImages();
+ // We'll process module_sp separately, before the other modules.
+ non_local_images.Remove(sc.module_sp);
+
LoadAddressResolver resolver(target, symbol_was_missing_weak);
ModuleFunctionSearchOptions function_options;
@@ -792,6 +796,11 @@ IRExecutionUnit::FindInSymbols(const
std::vector &names,
function_options.include_inlines = false;
for (const ConstString &name : names) {
+// The lookup order here is as follows:
+// 1) Functions in `sc.module_sp`
+// 2) Functions in the other modules
+// 3) Symbols in `sc.module_sp`
+// 4) Symbols in the other modules
if (sc.module_sp) {
SymbolContextList sc_list;
sc.module_sp->FindFunctions(name, CompilerDeclContext(),
@@ -801,18 +810,26 @@ IRExecutionUnit::FindInSymbols(const
std::vector &names,
return *load_addr;
}
-if (sc.target_sp) {
+{
+ SymbolContextList sc_list;
+ non_local_images.FindFunctions(name, lldb::eFunctionNameTypeFull,
+ function_options, sc_list);
+ if (auto load_addr = resolver.Resolve(sc_list))
+return *load_addr;
+}
+
+if (sc.module_sp) {
SymbolContextList sc_list;
- sc.target_sp->GetImages().FindFunctions(name,
lldb::eFunctionNameTypeFull,
- function_options, sc_list);
+ sc.module_sp->FindSymbolsWithNameAndType(name, lldb::eSymbolTypeAny,
+ sc_list);
if (auto load_addr = resolver.Resolve(sc_list))
return *load_addr;
}
-if (sc.target_sp) {
+{
SymbolContextList sc_list;
- sc.target_sp->GetImages().FindSymbolsWithNameAndType(
- name, lldb::eSymbolTypeAny, sc_list);
+ non_local_images.FindSymbolsWithNameAndType(name, lldb::eSymbolTypeAny,
+ sc_list);
if (auto load_addr = resolver.Resolve(sc_list))
return *load_addr;
}
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Remove redundant symbol lookups in IRExecutionUnit::FindInSymbols (PR #102835)
@@ -785,38 +785,50 @@ IRExecutionUnit::FindInSymbols(const
std::vector &names,
return LLDB_INVALID_ADDRESS;
}
+ ModuleList images = target->GetImages();
+ // We'll process module_sp separately, before the other modules.
+ images.Remove(sc.module_sp);
+
LoadAddressResolver resolver(target, symbol_was_missing_weak);
ModuleFunctionSearchOptions function_options;
function_options.include_symbols = true;
function_options.include_inlines = false;
for (const ConstString &name : names) {
+// The lookup order here is as follows:
+// 1) Functions in `sc.module_sp`
+// 2) Functions in the other modules
+// 3) Symbols in `sc.module_sp`
+// 4) Symbols in the other modules
+SymbolContextList sc_list;
if (sc.module_sp) {
- SymbolContextList sc_list;
sc.module_sp->FindFunctions(name, CompilerDeclContext(),
lldb::eFunctionNameTypeFull,
function_options,
sc_list);
if (auto load_addr = resolver.Resolve(sc_list))
return *load_addr;
+ sc_list.Clear();
DmT021 wrote:
done
https://github.com/llvm/llvm-project/pull/102835
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
