views:

300

answers:

4

In my current C#/NET 3.5 application, I have a task queue (thread safe) and I have 5 worker threads that has to constantly look for tasks in the queue. If a task is available, any one worker will dequeue the task and take required action.

My worker thread class is as follows:

public class WorkerThread
{
    //ConcurrentQueue is my implementation of thread safe queue
    //Essentially just a wrapper around Queue<T> with synchronization locks
    readonly ConcurrentQueue<CheckPrimeTask> mQ; 
    readonly Thread mWorker;
    bool mStop;

    public WorkerThread (ConcurrentQueue<CheckPrimeTask> aQ) {
        mQ = aQ;
        mWorker = new Thread (Work) {IsBackground = true};
        mStop = false;
    }

    private void Work () {
        while (!mStop) {
            if (mQ.Count == 0) {
                Thread.Sleep (0);
                continue;
            }

            var task = mQ.Dequeue ();
            //Someone else might have been lucky in stealing
            //the task by the time we dequeued it!!
            if (task == null) 
                continue;

            task.IsPrime = IsPrime (task.Number);
            task.ExecutedBy = Thread.CurrentThread.ManagedThreadId;
            //Ask the threadpool to execute the task callback to 
            //notify completion
            ThreadPool.QueueUserWorkItem (task.CallBack, task);
        }
    }

    private bool IsPrime (int number) {
        int limit = Convert.ToInt32 (Math.Sqrt (number));
        for (int i = 2; i <= limit; i++) {
            if (number % i == 0)
                return false;
        }

        return true;
    }

    public void Start () {
        mStop = false;
        mWorker.Start ();
    }

    public void Stop () {
        mStop = true;
    }
}

Problem is that when queue is empty, it consumes too much CPU (nearly 98%). I tried AutoResetEvent to notify the workers that queue has been changed. So they effectively wait for that signal to set. It has braught down the CPU to nearly 0% but I am not entirely sure whether this is the best method. Can you suggest a better method to keep the threads idle without hurting CPU usage?

+7  A: 

Check out this implementation of a BlockingQueue. If the queue is empty, it uses Monitor.Wait() to put the thread to sleep. When an item is added, it uses Monitor.Pulse() to wake up a thread that is sleeping on the empty queue.

Another technique is to use a semaphore. Each time you add an item to a queue, call Release(). When you want an item from a queue, call WaitOne().

R Samuel Klatchko
I like the idea of Semaphore since it ensures that if one item is added in queue, only one thread is awakened. Using the reset event awakes all worker threads. Thanks.
Hemant
But it has a downsite, I **have to** specify the maximum allowed items in the queue when I create semaphore! Are semaphores efficient enough to handle a very large count?
Hemant
In response to first comment. Monitor.Pulse only wakes one thread (Monitor.PulseAll) would awake every thread so there is no difference there.
R Samuel Klatchko
Just set the largest possible value. `new Semaphore(0, Int32.MaxValue)`
R Samuel Klatchko
For the record: I have used the semaphore technique and it works pretty well! Thanks everyone.
Hemant
A: 

You have a couple of options that I can think of.

One way is to place a small thread sleep during your loop. This will basically drop your CPU usage to 0 and is fairly standard way of doing this.

Another way is to use a reset (either auto or manual) as suggested by Mitch Wheat in the comments.

You could also devise some kind of IdleTask that has a thread sleep for a certain amount of time and if your queue is empty, just process the IdleTask (which will thread sleep).

Alastair Pitts
+1  A: 

You currently have Thread.Sleep(0) in your Work method for where there are no queue items. Change it to anything greater than 0 and your CPU use will go down. Try 10 to start with...

ck
A: 

If your Queue is thread safe then you would not need to do this...

    //Someone else might have been lucky in stealing 
    //the task by the time we dequeued it!! 
    if (task == null)  
        continue; 
Jag