[Lldb-commits] [PATCH] D150805: Proof of concept for reducing progress-reporting frequency.

Fri May 19 08:33:40 PDT 2023

JDevlieghere added a comment.

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

> This update switches to a time-based approach as suggested by Jordan. However, the timing is about the same as the original. I believe because calling getCurrentTime every iteration is comparably slow as printing the progress report itself.
>
> It probably is still a win over very slow connections, where printing is even slower and time would remain the same.
>
> 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.

As I said in my previous comment, if we want to do anything timeout based it should be done in the the broadcaster/listener machinery so that other types of events can benefit from it too. I think the issue you're describing can be solved at that level, for example by blocking for a the desired time and flushing all but the last event received until that point.

> Shall we just switch to a percentage? Printing it every percent update?
>
> That has the issues Jordan described, where things appear to progress quickly, and then may grind to a halt due to some big DIE.
>
> But I think the perfect shouldn't be the enemy of the good here.

Yes, that's definitely a shortcoming of this approach. What makes this somewhat less bad in my mind is that the inaccuracy is bounded by the granularity: e.g. something if you report every percentage, the error can never be more than a percentage. It would be nice if the consumer could set this property though, but I don't think there's a straightforward way to do that.

Adding support for a generic, user-specified, rate limit to the listeners would be my preferred solution, but I don't know how much work that would be.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150805