jingham added a comment.

In D97739#2610521 <https://reviews.llvm.org/D97739#2610521>, @clayborg wrote:

> In D97739#2608510 <https://reviews.llvm.org/D97739#2608510>, @friss wrote:
>
>> In D97739#2607961 <https://reviews.llvm.org/D97739#2607961>, @clayborg wrote:
>>
>>> In D97739#2607869 <https://reviews.llvm.org/D97739#2607869>, @jingham wrote:
>>>
>>>> This way of doing progress is going to look odd in anything that uses 
>>>> multiple debuggers.  If I'm reading the code aright, if I have two 
>>>> debuggers, and a target in Debugger A starts doing something that would 
>>>> cause progress traffic, both debuggers will show activity.
>>>
>>> that is true, but it is a global module repository that benefits both 
>>> debuggers. And I very rarely debug two things at the same time, so most of 
>>> the time for most people this will be beneficial and shouldn't cause too 
>>> much confusion.
>>
>> Just one tidbit here. Most users are actually routinely running tens of 
>> debuggers at the same time, because tests run in parallel and they have a 
>> debugger attached by default. Now if you have a long running operation kick 
>> in in your unit tests, you might already have a different kind of issue, but 
>> I’d like to avoid a world where the IDE displays spurious and wrong 
>> information because of this.
>
> But you wouldn't actually hookup any progress callbacks on any of these 
> debuggers right? You don't make a window for each test, IIRC you are only 
> running it under the debugger so you can report issues by using the debugger. 
> Am I remember this correctly? What happens when a test crashes? If you are 
> running 100 tests in parallel and 10 of them crash, do you open a window for 
> each one that does crash? Or do you manually have to debug the test in order 
> to stop at breakpoints or at a crash?
>
> If we truly need debugger specific task progress, that is a lot more work and 
> we have no great solution. One idea is we could end up having progress items 
> take a SymbolContextScope pointer as an optional parameter that would allow 
> us to grab the module pointer from it and then ask the debugger if any 
> targets contain this module prior to reporting progress. This would be a bit 
> expensive code for a lot of quick progress updates as we would need to 
> iterate over each target and each target's module list and we would still 
> have many system libraries reporting progress to all debuggers when targets 
> contain the same system libraries.
>
> The threading overhead and expense of delivering SBEvents also seems like 
> overkill as threading itself and waiting using a mutex + condition will slow 
> down progress delivery especially if we have a progress that does a bunch of 
> updates. And once we receive the event we will need to make a static function 
> call to extract all progress variables (total, completed, progress_id, baton, 
> etc).

I don't want to push this too hard, either way will work.  But note that one of 
the big advantages of the event version is that we aren't running arbitrary 
code fairly deep in the Symbol side of lldb.  At the cost of taking a lock to 
deliver the event, we get all this code out of the depth of the symbol reader 
and into a separate reporter thread.

I think the important thing here is that the code that detects the progress 
event not stall the Symbol processing.  But I don't see the need to be 
super-performant in reporting the event.  If the progress events are going by 
so fast that taking a lock to get the event and report on it matters, then the 
progress was going fast enough that the user probably won't care about it.

Another nice thing about the event version is that it allows you to coalesce 
reports if they come in too quickly, since you can peek at the event queue when 
you get the first event, and pull any other pending ones off.  So for instance 
if you get a "started event" and peeking at the queue find that you also 
already have the completed event, you could choose not to report progress, or 
just completed...

> In D97739#2608510 <https://reviews.llvm.org/D97739#2608510>, @friss wrote:
>
>> In D97739#2607961 <https://reviews.llvm.org/D97739#2607961>, @clayborg wrote:
>>
>>> In D97739#2607869 <https://reviews.llvm.org/D97739#2607869>, @jingham wrote:
>>>
>>>> This way of doing progress is going to look odd in anything that uses 
>>>> multiple debuggers.  If I'm reading the code aright, if I have two 
>>>> debuggers, and a target in Debugger A starts doing something that would 
>>>> cause progress traffic, both debuggers will show activity.
>>>
>>> that is true, but it is a global module repository that benefits both 
>>> debuggers. And I very rarely debug two things at the same time, so most of 
>>> the time for most people this will be beneficial and shouldn't cause too 
>>> much confusion.
>>
>> Just one tidbit here. Most users are actually routinely running tens of 
>> debuggers at the same time, because tests run in parallel and they have a 
>> debugger attached by default. Now if you have a long running operation kick 
>> in in your unit tests, you might already have a different kind of issue, but 
>> I’d like to avoid a world where the IDE displays spurious and wrong 
>> information because of this.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97739

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

Reply via email to