labath wrote:

> Can we build this feature into the Progress class by calling an accessor? 
> Something like:
> 
> ```
> Progress progress("Manually indexing DWARF", module_desc.GetData(), 
> total_progress);
> progress.SetMinimumNotificationTime(std::chrono::milliseconds(10));
> ```
> 
> Then any busy progress dialogs can take advantage of this timing feature?

That's possible, but there are a couple of caveats:
- in the Progress class implementation, we'd only have a single "time of last 
progress report" member, which would mean that (in order for it to be 
lock-free) it would have to be atomic. As there's no `atomic<duration>`, this 
would have to be a raw "time since epoch") timestamp. It's fine, but slightly 
uglier, and it's also one more atomic. In this version I was able to get away 
with it by essentially making the timestamp thread-local, which is sort of 
nice, but also less correct than the hypothetical version with a single 
variable, so maybe that's okay..
- this implementation only works if the progress updates come at a fairly fast 
and steady rate. This is the case here, but may not be true for all usages. 
With this implementation, if you have something that tries to send e.g. 9 (out 
of 10) progress updates very quickly (within the 10ms interval), but then gets 
spends a lot of time (many seconds) on the last part, the progress bar will 
stay stuck at `1/10 ` because we've ignored the updates 2--9. Attempting to 
schedule some sort of to send the `9/10` event after a timeout would probably 
involve spinning up another thread, and all of this management would make the 
code significantly more complicated (and slower).

This is the reason I did not want to make this a general feature, but it's not 
strong opinion, so if you're find with these trade-offs, I can move the code 
into the Progress class. I definitely don't want get into the business of 
managing another thread though.

https://github.com/llvm/llvm-project/pull/118953
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to