JDevlieghere added a comment.

Continuing some of the discussion from D150805 
<https://reviews.llvm.org/D150805> here as it mostly relates to this patch:

In D150805#4402906 <https://reviews.llvm.org/D150805#4402906>, @labath wrote:

> 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.

I'm not sure I (fully) agree with this statement. I may have misunderstood the 
motivation for D150805 <https://reviews.llvm.org/D150805>, but my understanding 
was that it was the consumption of the progress events that was too costly, not 
the generation. Of course this will indirectly make that problem better because 
of the rate limiting, but as I said in the original review, that still seems 
like something the consumer should have control over.

I would have assumed that reporting the progress wouldn't be that expensive. 
Based on the description of this patch, it seems like it's non-trivial, but 
also not a dominating factor by any means.

> 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.

Fair enough, but this could be achieved without the rate limiting: the 
reporting could be an async operation with the thread reporting every event.

> I like it because the act of reporting progress does block the reporting 
> thread in any way.

+1 but as I said above this could be orthogonal from rate limiting.

TL;DR I like the async reporting part, not (yet) convinced of the rate limiting 
"at the source".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152364

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

Reply via email to