rupprecht added a subscriber: labath.
rupprecht added a comment.

The other tricky part I missed before is this bit in between pulling the event 
off the listener queue and deciding to show it:

  auto *data = ProgressEventData::GetEventDataFromEvent(event_sp.get());
  if (!data)
    return;
  
  // Do some bookkeeping for the current event, regardless of whether we're
  // going to show the progress.
  const uint64_t id = data->GetID();
  if (m_current_event_id) {
    ...
    if (id != *m_current_event_id)
      return;
    if (data->GetCompleted() == data->GetTotal())
      m_current_event_id.reset();
  } else {
    m_current_event_id = id;
  }

When we pull the event off listener off the queue, we need to do 
post-processing ourselves to decide if it's even a progress event at all, and 
if it's meant for us. If we put the listener in charge of rate limiting and 
returning only the most recent event, it needs to know that the event it's 
returning is interesting. Otherwise the rate limiting might hide all the 
interesting events. On top of that, there are events that are *interesting* 
even if we don't want to show them.

Another option is to provide `Listener::GetRateLimitedEvents` (name TBD) 
instead. It (potentially) blocks for the rate limiting period and then returns 
a *list* of all the events that have happened within that time frame. Then we 
let the caller process it as it pleases and display only the most recent 
relevant one. It feels a little weird at first but might actually make sense 
compared to alternatives?

I think it would be nice to abstract the rate limiting somewhere, although I'm 
not sure anymore if having it directly in the broadcast/listen classes makes 
sense.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150805

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

Reply via email to