1

I've modified Producer/Consumer example http://msdn.microsoft.com/en-us/library/yy12yx1f(v=vs.80).aspx. I don't want Consumer to process queue "on event". Instead i'm using infinity loop (the same to one used in Producer) and try to process all elements asap. Are there any problems with such approach? Why we need "events" between Consumer and Producer if we can use infinity loop?

    // Consumer.ThreadRun
    public void ThreadRun()
    {
        int count = 0;
        while (!_syncEvents.ExitThreadEvent.WaitOne(0, false))
        {
            lock (((ICollection)_queue).SyncRoot)
            {
                while (_queue.Count > 0)
                {
                    int item = _queue.Dequeue();
                    count++;
                }
            }
        }
        Console.WriteLine("Consumer Thread: consumed {0} items", count);
    }
Oleg Vazhnev
  • 23,239
  • 54
  • 171
  • 305

3 Answers3

1

I see two potential problems with what you have

  1. When the queue is empty your version will sit in a busy loop burning precious CPU, using a event puts the thread to sleep until there is actual work to be done.
  2. By locking the queue and processing all the elements in the queue in a single loop like you are doing, you negate the potential benefit of having multiple consumer threads processing the queue. Now because you only increment a count in your example this might not seem like a big deal, but if you start doing real work with the items that you dequeue you could benefit from having multple threads handling that work.

If you are using .NET 4 you might want to take a look at using BlockingCollection(T) Class which would give an even cleaner solution to all of this with less locking to boot.

Chris Taylor
  • 52,623
  • 10
  • 78
  • 89
  • 1. Producer in the original code also use "busy loop". Two "busy loop" or one "busy loop" - is it bid differense? 2. I have just one consumer and multiply producers. Thanks for BlockingCollection reference, will try :) – Oleg Vazhnev Apr 30 '11 at 15:28
  • @javapowered, 1 busy loop burns one CPU/Core while the rest can continue working, 2 busy loops burn 2 CPU/Core. If you want multiple producers then your producers should only be doing work while there is work to be done, a busy loop that does nothing is normally not good, and will reduce the scalability of your system. If you want multiple producers and they can produce 100% work 100% of the time then yes let them loop away. The problem with your consumer version is that it will loop while there is nothing to do. – Chris Taylor Apr 30 '11 at 15:33
0

A potential problem could occur if your setting of the ExitThreadEvent gets into a race condition (since you don't show that part of the code it's hard to tell if that could happen).

Lucero
  • 59,176
  • 9
  • 122
  • 152
0

If you are able to use .NET 4.0 you can use the built in BlockingCollection class to solve this problem simply and efficiently.

Drew Marsh
  • 33,111
  • 3
  • 82
  • 100