Thanks Raphael for the pointers, I will take a look.
I just noticed I forgot to attach the actual patch I prepared ;)
Here it is.
On Fri, Jan 29, 2021, 5:02 PM Raphael “Teemperor” Isemann <
teempe...@gmail.com> wrote:
> I can't help with any of the requested feedback, but the last time I
> looked at a benchmark involving ManualDWARFIndex, the
> ManualDWARFIndex::IndexUnit spent about 40% of its runtime just
> constructing ConstString. Half of that 40% (so, 20% total time) was just
> spend contesting/locking the locks for ConstString's global string pools.
> Refactoring the code to not having to reaquire a contested global lock for
> every DIE might help with improving startup time.
>
> Cheers,
> - Raphael
>
> On 29 Jan 2021, at 16:25, Lasse Folger via lldb-commits <
> lldb-commits@lists.llvm.org> wrote:
>
> Hi lldb devs,
>
> We experienced some issues when debugging large binaries.
> The startup time of lldb for large binaries is long (tens of seconds).
> One of the reasons is that the *ManualDWARFIndex::Index()* function is
> executed synchronously and thus blocks during startup.
> We would like to change this to an async task. Let me know if you have any
> concerns.
>
> I prepared a small patch to schedule the index creation async.
>
> I would like to have feedback on three parts:
>
> 1. In *ManualDWARFIndex::Preload() *I capture *this* of the
> *ManualDWARFIndex*. As far as I could tell this is not a problem since
> the index is not relocated or deleted. However, I'm not that familiar with
> the architecture. Are there cases when the object is relocated or deleted
> before lldb stops?
> 2. I use the *Module *mutex for synchronization because I didn't want
> to introduce a mutex in *ManualDWARFIndex *or *DWARFIndex*. Do you
> think this locking granularity is fine or should I create a dedicated mutex
> for it? Furthermore, I looked through the implementation
> of ManualDWARFIndex::*Index()* and couldn't find any locking function
> in there. However, I might have missed something (or some of the underlying
> implementations change in the future) and there is a danger for a deadlock.
> 3. *ManualDWARFIndex::Index()* creates a thread pool internally. This
> means if multiple calls to* ManualDWARFIndex::Index() *are executed at
> the same time, there might be more threads than cores. This might cause
> contention and decrease efficiency. (Thanks to Andy for pointing this out).
>
> Please let me know what you think and if I should change something.
>
>
> kind regards,
> Lasse
> _______________________________________________
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>
>
>
From 90ee8ce572af5a73c5216f4a6864e570b7f169ab Mon Sep 17 00:00:00 2001
From: Lasse Folger <lassefolger@google.com>
Date: Mon, 25 Jan 2021 11:09:43 +0100
Subject: [PATCH] [lldb] Create ManualDWARFIndex asynchronously during Preload
---
lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp | 8 ++++++++
lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h | 2 +-
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp b/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
index dda599baffeb..e1f3f98e7993 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
@@ -17,11 +17,19 @@
#include "lldb/Utility/Stream.h"
#include "lldb/Utility/Timer.h"
#include "llvm/Support/ThreadPool.h"
+#include "llvm/Support/Threading.h"
using namespace lldb_private;
using namespace lldb;
+void ManualDWARFIndex::Preload() {
+ auto Task = [this]() { Index(); };
+ llvm::llvm_execute_on_thread_async(std::move(Task));
+}
+
void ManualDWARFIndex::Index() {
+ std::lock_guard<std::recursive_mutex> guard(m_module.GetMutex());
+
if (!m_dwarf)
return;
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h b/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h
index baff989eecca..cae511935e62 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h
@@ -24,7 +24,7 @@ public:
: DWARFIndex(module), m_dwarf(&dwarf),
m_units_to_avoid(std::move(units_to_avoid)) {}
- void Preload() override { Index(); }
+ void Preload() override;
void
GetGlobalVariables(ConstString basename,
--
2.30.0.280.ga3ce27912f-goog
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
