[Lldb-commits] [PATCH] D152364: [lldb] Rate limit progress reports -- different approach [WIP-ish]

[Jonas Devlieghere via Phabricator via lldb-commits]

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