[Lldb-commits] Performing ManualDWARFIndex::Index() async during ManualDWARFIndex::Preload()

[Lasse Folger via lldb-commits]

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 <
teemperor at 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 at 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 at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20210129/d07d2b60/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-lldb-Create-ManualDWARFIndex-asynchronously-during-P.patch
Type: application/x-patch
Size: 1791 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20210129/d07d2b60/attachment.bin>