@@ -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)
+ i
@@ -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)
+ i
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
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-com
@@ -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 las
@@ -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 las
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
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/
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
---
.../D
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
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 thin
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 ll
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
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
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.
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
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 con
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_
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
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
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 tr
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 spen
@@ -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);
+
LoadAddress
@@ -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);
+
LoadAddress
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::FindIn
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
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
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
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
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 d6183d6b0e1c755d6adf3f5246374
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 c
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::FindIn
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
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
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,
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-comm
@@ -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, th
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
---
.../SymbolFil
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
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
---
.../SymbolFil
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
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
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-edef3a65d5d569bbb75a4158d35b827aa5d42ee03ccd3b0c1d4f354afa1
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`
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 res
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
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::FindIn
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::Fi
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::Fi
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::Fi
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::Fi
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::Fi
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::Fi
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::Fi
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::Fi
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::FindIn
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_shado
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::FindIn
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 Mo
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
> >
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
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
@@ -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;
+
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:
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
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::FindIn
@@ -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;
-
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@list
@@ -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 doin
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
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 checke
71 matches
Mail list logo
