views:

459

answers:

4

My form receives asynchronous callbacks from another object on random worker threads. I have been passing the data to the main thread (where it can be used to update onscreen controls) using delegates as shown below. Performance is dreadful -- once I reach 500 updates per second, the program completely locks up. My GUI processing itself is not the problem, as I can simulate this level of updating within the form and have no problems. Is there a more efficient mechanism I should be using to hand off the data from thread to thread?

delegate void DStatus( MyStatus obj );
DStatus _status; // set to MainThreadOnStatus during construction

// this function only called on form's owner thread
void MainThreadOnStatus( MyStatus obj )
{
   // screen updates here as needed
}

// this function called by arbitrary worker threads in external facility
void OnStatus( MyStatus obj )
{
   this.BeginInvoke( _status, obj );
}
+1  A: 

You probably don't need to update UI on every event, but rather "not as often as X times per second". You may utilize StopWatch or other timing system to collect events during a period of time, and then update UI when appropriate.

If you need to capture all events, collect them in the Queue and fire event every so often, and that event handler will process the Queue and update UI once for all queued events.

Ilya Ryzhenkov
A: 

It is difficult to tell the exact problem, but some possibilities...

Is your MyStatus object that you are passing to OnStatus derived from MarshalByRefObject (and every object in it)? If not it will be serailized on every call that is marshaled and that can cause a huge performance hit.

Also, you should really call this.InvokeRequired before invoking the delegate using the control, but really that is just a best practice.

Brian ONeil
Does MarshalByRefObject have any effect across thread boundaries within a single appdomain? I thought it only applied when you had multiple appdomains.
Eric
+1  A: 

I've been doing what Ilya has been suggesting. For UIs that don't have to respond "real time" I have a stopwatch that goes twice a second or so. For faster updates I use a queue or other datastructure that stores the event data and then use "lock (queue) { }" to avoid contention. If you don't want to slow down the worker threads you have to make sure that the UI thread doesn't block the workers too long.

Andrew Queisser
+1  A: 

I’m not a big fan of timers, if you want a more event driven approach, try something like this:

public class Foo
{
 private AsyncOperation _asyncOperation = null;
 private SendOrPostCallback _notifyNewItem = null;

 //Make sure you call this on your UI thread.
 //Alternatively you can call something like the AttachUI() below later on and catch-up with
 //your workers later.
 public Foo()
 {
  this._notifyNewItem = new SendOrPostCallback(this.NewDataInTempList);
  this._asyncOperation = AsyncOperationManager.CreateOperation(this);
 }

 public void AttachUI()
 {
  if (this._asyncOperation != null)
  {
   this._asyncOperation.OperationCompleted();
   this._asyncOperation = null;
  }

  this._asyncOperation = AsyncOperationManager.CreateOperation(this);
  //This is for catching up with the workers if they’ve been busy already
  if (this._asyncOperation != null)
  {
   this._asyncOperation.Post(this._notifyNewItem, null);
  }
 }


 private int _tempCapacity = 500;
 private object _tempListLock = new object();
 private List<MyStatus> _tempList = null;

 //This gets called on the worker threads..
 //Keeps adding to the same list until UI grabs it, then create a new one.
 public void Add(MyStatus status)
 {
  bool notify = false;
  lock (_tempListLock)
  {
   if (this._tempList == null)
   {
    this._tempList = new List<MyStatus>(this._tempCapacity);
    notify = true;
   }

   this._tempList.Add(status);
  }
  if (notify)
  {
   if (this._asyncOperation != null)
   {
    this._asyncOperation.Post(this._notifyNewItem, null);
   }
  }
 }

 //This gets called on your UI thread.
 private void NewDataInTempList(object o)
 {
  List<MyStatus> statusList = null;
  lock (this._tempListLock)
  {
   //Grab the list, and release the lock as soon as possible.
   statusList = this._tempList;
   this._tempList = null;
  }
  if (statusList != null)
  {
   //Deal with it here at your leasure
  }
 }
}

I’ve used this in a custom Log4Net logger collecting log entries and adding them to a circular array that’s bound to a grid. The performance ended up being pretty good for what it does.