Re: thread communication

Tech-Archive recommends: Repair Windows Errors & Optimize Windows Performance

On Wed, 15 Oct 2008 00:12:52 -0700, Peter Morris <mrpmorrisNO@xxxxxxxxxxxxx> wrote:

Hi Pete

This morning I changed some code. I previously had a Queue<IFileProcessor> which I checked in a thread and called Execute() on each instance as I removed it. I changed this into a simple class which has a SyncRoot which I can Wait and Pulse, and then some code to process the queue.

[...]

Much happier now :-)

Cool...glad the info helped. The new version looks good.

One thing I noticed in your code though, in your desire to move the actual processing outside of the lock (a good idea), you have introduced an inefficiency in the synchronization (not so good).

Specifically, in the scenario in which your thread is blocked waiting, if something is enqueued, the very first thing that the thread does after being woken up again is to release the lock and then try to acquire it again. That's inefficient in and of itself, but it gets really bad if there's another thread also trying to get the lock at the same time. The thread you just woke up then has to go right back to the waiting state until the lock gets released again.

On a single-CPU system this winds up being not _quite_ so bad, because there's a good chance your thread will get back to the reacquisition of the lock before it uses up its quantum (in fact, in the code you posted, it's practically guaranteed). But on a multi-core system, it's entirely possible that even in that short period of time, some other thread can grab the lock.

If these tasks don't come up very often, that's probably okay. The inefficiency won't matter. But in a high-throughput system, lock contention and context switching can be expensive.

If you think the inefficiency might actually be an issue, there are surely even higher-performance implementations a true threading expert could come up with, but at the very least, you might want to consider something more like this:

private void ProcessQueue()
{
lock (SyncRoot)
{
while (true)
{
if (Queue.Count > 0)
{
IFileProcessor processor = Queue.Dequeue();

Monitor.Leave(SyncRoot);
processor.Execute();
Monitor.Enter(SyncRoot);

if (processor.State == FileProcessState.Ready)
{
Enqueue(processor);
}
}
else
{
Monitor.Wait(SyncRoot);
}
}
}
}

This allows the thread to retain the lock all the way from being woken up until it goes to check the queue again.

You may be able to gain even more efficiency by emptying the queue all at once before releasing the lock, putting each dequeued IFileProcessor instance into a temporary array. That will tend to minimize the number of times you actually have to release and then reacquire the lock. (Though, again...this would only matter in a high-throughput situation). For example, changing the above if() and true clause to:

if (Queue.Count > 0)
{
IFileProcessor[] processors = Queue.ToArray();
List<IFileProcessor> readyList = new List<IFileProcessor>(processors.Length);

Queue.Clear();

Monitor.Leave(SyncRoot);
foreach (IFileProcessor processor in processors)
{
processor.Execute();
if (processor.State == FileProcessState.Ready)
{
readyList.Add(processor);
}
}
Monitor.Enter(SyncRoot);

foreach (IFileProcessor processor in readyList)
{
Enqueue(processor);
}
}

That way, the thread grabs as much of the current state as it can before letting go of the lock. It also queues all the new "ready" things it knows about all at once.

Finally, if you want to get really picky, at least from the code you posted it appears that pushing the new "ready" items back through the queue is wasteful. Especially with the above alternative, you could easily maintain a local Queue to which all of the public Queue elements are copied, and then just loop while unlocked until the local Queue is empty, adding the "ready" items back into the local Queue as you find them. (Of course, you'd only want to do that if a) you can guarantee that doing so won't starve new items being added to the public queue during the processing of the local queue, or b) that starving those items won't matter...it would essentially boost the priority of the items that remain ready after processing above any newly added items that haven't been processed yet).

Thanks for sharing...it's these practical "code in the wild" examples that I think really help illustrate the different approaches available in C# for specific tasks. There's nothing quite like a real-world case study. :)

Pete
.

Relevant Pages

  • Re: Seeing lock order reversal
    ... 21 vm page queue free mutex -- ... 18 UMA zone -- ... 18 sleep mtxpool -- ... 15 process lock -- ...
    (freebsd-current)
  • Re: Latency issues with buf_ring
    ... been to make a compile option to the driver, but a REAL fix would be much ... result in really high packet latencies in some cases. ... The race, of course, is that the thread holding the lock ... for that queue. ...
    (freebsd-net)
  • Re: location of bioq lock
    ... queue is owned by the driver, and the locking scheme remains the same. ... from a different scheduler than the default, it can be easily plugged in. ... process you have to lock each queue before playing with it. ...
    (freebsd-current)
  • Re: thread-safety
    ... but that would depend on the implementation of your Queue class. ... lock { ... It's odd for you to be passing a WaitHandle of any sort to the Monitor class; if you've got a WaitHandle, you would normally just wait on _that_, rather than using the WaitHandle simply as a synchronization object for Monitor. ... It would be a problem to call Monitor.Pulseif no other thread is waiting at a call to Monitor.Wait, because then the waiting thread would miss the Pulse. ...
    (microsoft.public.dotnet.languages.csharp)
  • Re: Monitor.Wait(object, timeout) confusion
    ... It is then moved to the ready queue, ... >> and blocks until it can reaquire the lock. ... The method has to block until it reacquires the ... The first part of the documentation implies that if you call ...
    (microsoft.public.dotnet.framework)