JDevlieghere added a comment.

In D131328#3706814 <https://reviews.llvm.org/D131328#3706814>, @labath wrote:

> In D131328#3706749 <https://reviews.llvm.org/D131328#3706749>, @JDevlieghere 
> wrote:
>
>> There was another approach I considered initially which might alleviate some 
>> of the issues Pavel raised. Instead of using the global thread pool, that 
>> plan consisted of a dedicated thread to fetch symbols in the background. 
>> That would allow us to add the symbol info synchronously. For example on 
>> every stop we could ask if there are any new symbol files to add instead of 
>> trying to add the symbol info as soon as it becomes available.
>
> I kinda like that.

I tried prototyping this. I wasn't super happy with the results:

- This one's minor, but compared to the current patch, there's a bunch of 
synchronization boilerplate: input/output mutex, condition variable, etc
- Who should own this class? Inherently it's tied to the shared module list, 
but that never gets destroyed, so that has the flushing issue. The only viable 
alternative is to have an instance controlled by 
Debugger::{Initialize/Terminate}, which is exactly what we do for the global 
thread pool in D131407 <https://reviews.llvm.org/D131407>.
- There's really no good place to poll for this in the target. You'd need to do 
it from the process.
- Once you poll for new symbol files, you can't flush the newly added modules 
from the download (unless you store them in the target) because other targets 
will need to be notified later.

All these things can be overcome, but that was the point where I decided that 
maybe it wasn't worth it.

Instead I focussed on avoiding doing all this work from an arbitrary thread and 
using a broadcast event to do it from the event handler thread.

> Bonus points if you make that single thread download multiple files at the 
> same time (using threads to paralelize downloading is not the best approach, 
> since the task is not cpu-bound). :)

If LLDB was doing the downloading itself we might be able to do better, but by 
shelling out to dsymForUUID all we see in an opaque blocking call.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131328/new/

https://reviews.llvm.org/D131328

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to