Reputation: 34509
I've recently tried to implement an asynchronous callback model, so that when developers consume my library, they don't need to worry about the following:
if(control.InvokeRequried) { // invoke }
That part of the code seems to be working quite nicely, but I discovered something a bit strange today, that I think I've diagnosed but would like to see what others with more experience think, and ways I might be able to resolve it.
The problem I've got, is it seems that my posts are very slow for some reason, and consequently they appear to get a little clogged up, and keep running after I've completed my work. A brief code example:
// called by user
WorkerEventHandler workerDelegate = new WorkerEventHandler(CalcAsync);
workerDelegate.BeginInvoke(p, asyncOp, null, null);
CalcAsync(P p, AsyncOperation asyncOp)
{
Calculate();
asyncOp.PostOperationCompleted(new AsyncCompletedEventArgs(null, false,
asyncOp.UserSuppliedState);
}
Calculate()
{
DoStuff() // updates the progress as it goes
this.UpdateStatus("Done"); // marks us as done
this.UpdateProgress(pId, 100); // increase progress to max
}
When stepping through I get repeated calls to an UpdateProgress being called from the posted events however, so it looks like they've been unable to keep up, and are still being posted. These are handled like so:
// initalized with
// onProgressUpdatedDelegate = new SendOrPostCallback(UpdateProgress);
private SendOrPostCallback onProgressUpdatedDelegate;
public void UpdateProgress(Guid pId, ProgressEventArgs e)
{
AsyncOperation asyncOp = GetAsyncOperation(pId);
if (asyncOp != null)
{
asyncOp.Post(this.onProgressUpdatedDelegate, e);
}
else
{
UpdateProgress(this, e);
}
}
private void UpdateProgress(object state)
{
ProgressEventArgs e = state as ProgressEventArgs;
UpdateProgress(this, e); // does the actual triggering of the EventHandler
}
If it's relevant these events are all writing to a console. But I seem to only see my progress go down on longer operations, which seems to be the same results I see through my WPF test app.
Does it suggest that the Post is just slow? I suppose what I'd really like is every time I call UpdateProgress via the Post I want to purge any existing posts within the queue if this is the case. But I'm not sure this is possible? If it is, is it possible to purge just for these events, as I don't want to then purge my UpdateStatus event accidentally...
Edit
Some of the missing methods if that makes it easier.
public void UpdateProgress(Guid pId, ProgressEventArgs e)
{
AsyncOperation asyncOp = GetAsyncOperation(pId);
if (asyncOp != null)
{
asyncOp.Post(this.onProgressUpdatedDelegate, e);
}
else
{
UpdateProgress(this, e);
}
}
private void UpdateProgress(object sender, ProgressEventArgs e)
{
ProgressChangedEventHandler handler;
lock (progressUpdateEventLock)
{
handler = progressUpdateEvent;
}
if (handler != null)
handler(sender, e);
}
Upvotes: 1
Views: 738
Reputation: 942000
It isn't that clear what's going on here. Getting multiple UpdateProgress calls on the UI thread for one UpdateProgress call on the worker thread is definitely bad. Your code as posted should not do this.
But, yes, the Post() call is not particularly fast. It takes roughly a millisecond for the message loop on the UI thread to pick up the notification, assuming it is not busy doing something else. This is not one millisecond of hard CPU work, it is a delay induced by the Windows plumbing that handles GetMessage(). The thread context switch is part of it, but just a small part.
This can have notable nasty side-effects. Calling Post() too often, or having the posted delegate doing too much hard work, can seriously affect the UI thread. To the point that it doesn't get around its regular duties anymore. Because of the delay, you can get there quickly; it only takes 1000 posts per second.
Of course, it is never necessary to post that often, the human eye cannot perceive updates at a rate any faster than about 25 per second. Doing it more frequently is just wasted effort. But obtaining this ideal update rate is not particularly easy to achieve, unless the worker thread pays specific attention to elapsed time.
Anyhoo, it is better to leave it up to the client code to tell you how it wants its notifications marshaled. The FileSystemWatcher.SynchronizingObject is a good example of that. If Post() falls apart, that's one way for the client code to fix the problem. Also take a look at the BackgroundWorker class.
Upvotes: 1
Reputation: 134035
Is there some code missing from your example? You have a call to UpdateProgress(this, e)
, but no corresponding method with that signature. Unless this
is a Guid
, but I doubt that, as then you'd end up with an infinitely recursive method. You also have a call to UpdateProgress(100)
, and no corresponding method.
In any event, it would probably be helpful if you used a profiler or, if you don't have one, a Stopwatch
, to see where time is being spent in your methods.
It could be that the thread context switches are killing you because you have to do InvokeRequired
and then Invoke
for every update. If that's the case, it'd probably be helpful to put the Post
s into a queue and have a timer that empties the queue periodically. Something like:
Timer queueTimer = new Timer(QueueTimerProc, null, 20, 20);
queueTimer.Start();
void QueueTimerProc(object state)
{
if (queue.Count > 0)
{
if (this.InvokeRequired)
// Invoke the EmptyQueue method
else
// Call the EmptyQueue method
}
}
void EmptyQueue()
{
lock (queue)
{
while (queue.Count > 0)
{
// dequeue an item and post the update
}
}
}
Upvotes: 0