[Lldb-commits] [PATCH] D150805: Rate limit progress reporting

[Pavel Labath via Phabricator via lldb-commits]

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