How to properly keep the UI updated while transfer

Question

I have this form that spawns a new thread and starts listening and waiting for UDP packets in a loop. What I need is to keep the UI updated with the number of bytes received.

For that, I have setup an event which I'll raise as soon as a packet is received and pass the number of bytes received as an argument. Since I'm not running on the UI thread, I cannot simply update the UI directly. Here's what I'm currently doing:

private void EVENTHANDLER_UpdateTransferProgress(long receivedBytes) {
    if(InvokeRequired) {
        Invoke(new MethodInvoker(() => {
            totalReceivedBytes += receivedBytes;
            Label.Text = totalReceivedBytes.ToString("##,0");
        }));
    }
}

But this is still running on the same thread as the packet reception loop and it will not return to that loop - and wait for another packet - until this EVENTHANDLER_UpdateTransferProgress method returns.

My question is basically about the following line in the method above:

Label.Text = totalReceivedBytes.ToString("##,0");

Updating the UI like this slows down the packet reception. If I take that line off (or comment it), the packet reception will be much faster.

How can I possibly solve this issue? I think more threads is the key, but I'm not sure how to properly implement them in this situation... I'm using Windows Forms with .NET 2.0.

EDIT:

On my previous testing, the above seem to be true and it may actually be to some extent. But after a little more testing I realized the problem was on the whole Invoke(new MethodInvoker(() => { ... })); thing. When I remove that (the UI won't be updated of course) and leave EVENTHANDLER_UpdateTransferProgress but keep raising the event, the packet reception is much faster.

I tested receiving some file which took in average about ~1.5sec without calling Invoke() at all on the event handler. When I did call Invoke() in the event handler, even without updating any control in the UI or doing any operation (in other words, the anonymous method body was empty), it took much longer, around ~5.5sec. You can see it's a big difference.

Is there anyway to improve this?

Answer 1

Have you tried using BeginInvoke instead of Invoke? BeginInvoke() is an asychronous call.

private void EVENTHANDLER_UpdateTransferProgress(long receivedBytes) {
    if(InvokeRequired) {
        BeginInvoke(new MethodInvoker(() => {
            totalReceivedBytes += receivedBytes;
            Label.Text = totalReceivedBytes.ToString("##,0");
        }));
    }
}

Answer 2

The problem with your approach is that it updates the UI on every single packet. If you received 1000 packets every second, you would update the UI 1000 times every second! The monitor probably doesn't refresh more than 100 times per second, and nobody is going to be able to read it if it updates more than 10 times per second.

A better way to approach this problem is to put the totalReceivedBytes += receivedBytes; in the thread that handles the I/O and put a timer on the UI thread that executes Label.Text = totalReceivedBytes.ToString("##,0"); only a few times per second at most. When the transfer starts, start the timer; when the transfer stops, stop the timer.

Answer 3

Yes, there is a way to improve this.

The first is to use BeginInvoke instead of Invoke which will not wait for the invoke to return. You should also consider using another form in your method

private void EVENTHANDLER_UpdateTransferProgress(long receivedBytes) {
    if(InvokeRequired) {
        BeginInvoke(new Action<long>(EVENTHANDLER_UpdateTransferProgress),
                    receivedBytes));
        return;
    }
    totalReceivedBytes += receivedBytes;
    Label.Text = totalReceivedBytes.ToString("##,0");
}

So if you call this method from a method that does not require invoking, the update on the GUI is still performed.

---

Another option that you can do is break of a thread in your download thread. Something in the likes of

public event EventHandler<MonitorEventArgs> ReportProgress;

public void startSendingUpdates(MonitorEventArgs args) {
  EventHandler<MonitorEventArgs> handler = ReportProgress;
  if (handler == null) {
      return;
  }
  ThreadPool.QueueUserWorkItem(delegate {
      while (!args.Complete) {
          handler(this, args);
          Thread.Sleep(800);
      }
  });
}

public void download() {
    MonitorEventArgs args = new MonitorEventArgs();
    startSendingUpdates(args);
    while (downloading) {
        int read = downloadData(bytes);
        args.BytesTransferred += read;
    }
    args.Complete = true;
}

public class MonitorEventArgs : EventArgs {
    public bool Complete { get; set; }
    public long BytesTransferred { get; set; }
}

The overhead of this is kind of small compared to the benefits. Your download thread is not affected by the updates to the GUI (at least not compared to waiting on the GUI to update). The downside is you are occupying a thread in the threadpool, but hey, that's what they're there for! And, the thread shuts down when it's done, since you set the complete flag. You don't need to lock when setting that either, since an extra run in the worker thread is unimportant in the context.