labath added a comment.

I agree that we should have a rate limiting mechanism very close to the source, 
to avoid wasting work for events that aren't going to be used. This is 
particularly important for debug info parsing, where we have multiple threads 
working in parallel and taking a lock even just to check whether we should 
report something could be a choke point.

In D150805#4351277 <https://reviews.llvm.org/D150805#4351277>, @saugustine 
wrote:

> What would be ideal is a timing thread that wakes up every X seconds and 
> prints the results, but there isn't a good mechanism for that, and doing that 
> portably is way out of scope for this.

I've implemented something like that in D152364 
<https://reviews.llvm.org/D152364>.  Let me know what you think. I like it 
because the act of reporting progress does block the reporting thread in any 
way. (At least for update-string-free updates that is, but I expect that we 
won't be sending update strings for extremely high frequency events.) However, 
I'm not entirely sure whether it meets everyone's use cases, mainly because I 
don't know what those use cases are (e.g. this implementation can "lose" an 
update string if they are coming too fast -- is that acceptable ?)

> In my opinion, the problem is that a single-die is too small a unit of work 
> to be worth reporting on.

I don't think this is correct. The unit of reporting is single DWARF Unit 
<https://github.com/llvm/llvm-project/blob/main/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp#L88>,
 which feels OK, assuming we don't do anything egregious for each update. What 
might have confused you is this code here 
<https://github.com/llvm/llvm-project/blob/main/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp#L93>,
 which tries to parse DIE trees for all units (and updates progress after each 
**unit**. However, in my test at least, the DWARF units had all their DIEs 
extracted by the time we got to this point, which meant that code was 
essentially doing nothing else except generating progress reports. I'd be 
tempted to just remove progress reporting from this step altogether, though if 
we go with something like that patch above (where a single update just 
increments an atomic var), then I guess keeping it in would not be such a 
problem either.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150805

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

Reply via email to