On 29/01/2021 16:25, Lasse Folger via lldb-commits 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.
Before I go further, let me ask this: You are aware of the
'target.preload-symbols' setting which can disable (delay) the indexing
at starting, right? I am asking because it may result in a similar
behavior than what you get with your patch. At the very least, it would
be interesting to see a comparison of the two scenarios, to get a feel
for the benefits it offers.
On 29/01/2021 18:23, Lasse Folger via lldb-commits wrote:
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 <mailto: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.
I also feel obligated to point out that this code has been optimized and
re-optimized by a number of people already. So all of the quick wins are
likely gone already.
I am not saying that its not possible to improve the performance of this
(in fact, I'm certain it can) -- however, such an effort will probably
require making fundamental changes to the algorithm, rather than just
using fancier data structures.
I don't want to sound too discouraging, but I think you ought to know
what you're getting yourself into.
Good luck! (I mean it)
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?
While it's very unlikely, I don't think we can guarantee that the Module
object holding the index is not deleted by the time the async thread
runs. Such a thing might happen due to events totally beyond our
control, such as the user calling SBTarget::RemoveModule.
This should be fixable by simply stashing the Module shared pointer.
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?
I think it should work, but a separate synchronization primitive might
be cleaner. I might even go for a once_flag/call_once to also get the
"runs once" behavior for free.
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.
The lack of locking is not surprising because these functions already
expect that the caller has taken the Module lock.
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).
This is an interesting question for me, but I don't really have a good
answer to it. On one hand, you could say that this is a situation that
could occur already (though it wasn't always the case -- back when we
had our own thread pool, it was process-wide and shared between
different module instances). But OTOH, this does make it much easier for
this situation to occur...
pl
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
