high cpu usage on windows service application

Question

hi folks

I recently started on a new job and there is a windows service here that consumes messages from a private windows queue. This service consumes the messages only from 9am to 6pm. So, during 7pm to 8:59am it accumulates a lot of messages on the queue. When it starts processing at 9pm, the cpu usage of the service goes to high(98, 99 percent), screwing with the server's performance.

This service use threads to process the messages of the queue, but as I had never worked with threads before I am a little lost.

Here's the part of code that I am sure this is happening:

private Thread[] th;

//in the constructor of the class, the variable th is initialized like this:
this.th = new Thread[4];

//the interval of this method calling is 1sec, it only goes high cpu usage when there is a lot of messages in the queue
public void Exec()
{
    try
    {
        AutoResetEvent autoEvent = new AutoResetEvent(false);
        int vQtd = queue.GetAllMessages().Length;

        while (vQtd > 0)
        {
            for (int y = 0; y < th.Length; y++)
            {
                if (this.th[y] == null || !this.th[y].IsAlive)
                {
                    this.th[y] = new Thread(new ParameterizedThreadStart(ProcessMessage));
                    this.th[y].Name = string.Format("Thread_{0}", y);
                    this.th[y].Start(new Controller(queue.Receive(), autoEvent));
                    vQtd--;
                }
            }
        }
    }
    catch (Exception ex)
    {
        ExceptionPolicy.HandleException(ex, "RECOVERABLE");
    }
}

EDIT: I am trying the second approach posted by Brian Gideon. But I'll by honest: I'm deeply confused with the code and I don't have a clue about what it's doing.

I haven't changed the way the 4 threads are created and the other code I showed, just changed my Exec(exec is the method called every second when it's 9am to 6pm) method to this:

public void Exec()
    {
        try
        {
            AutoResetEvent autoEvent = new AutoResetEvent(false);
            int vQtd = queue.GetAllMessages().Length;

            while (vQtd > 0)
            {

                for (int i = 0; i < 4; i++)
                        {
                            var thread = new Thread(
                                (ProcessMessage) =>
                            {
                                while (true)
                                {
                                    Message message = queue.Receive();
                                    Controller controller = new Controller(message, autoEvent);
                                    //what am I supposed to do with the controller?
                                }
                            });
                        thread.IsBackground = true;
                        thread.Start();
                        }
        vQtd--;

            }
        }
        catch (Exception ex)
        {
            ExceptionPolicy.HandleException(ex, "RECOVERABLE");
        }
    }

Answer 1

You have two options. Either you insert delays after each message with Thread.Sleep() or lower the thread priority of the polling threads. If you lower the thread priority the CPU usage will still be high, but should not affect performance that much.

Edit: or you can lower the number of threads from 4 to 3 to leave one core for other processing (assuming you have a quad core). This of course reduces your dequeuing throughput.

Edit2: or you could rewrite the whole think with task parallel library if you are running .NET 4. Look for Parallel.ForEach(). That should save you from some of the footwork if you are not familiar with threads.

Answer 2

The method you show runs in a tight loop when all threads are busy. Try something like this:

    while (vQtd > 0)
    {
        bool full = true;

        for (int y = 0; y < th.Length; y++)
        {
            if (this.th[y] == null || !this.th[y].IsAlive)
            {
                this.th[y] = new Thread(new ParameterizedThreadStart(ProcessMessage));
                this.th[y].Name = string.Format("Thread_{0}", y);
                this.th[y].Start(new Controller(queue.Receive(), autoEvent));
                vQtd--;
                full = false;
            }
        }

        if (full)
        {
            Thread.Sleep(500); // Or whatever it may take for a thread to become free.
        }
    }

Answer 3

Ouch. I have to be honest. That is not a very good design. It could very well be spinning around that while loop waiting for previous threads to finish processing. Here is a much better way of doing it. Notice that the 4 threads are only created once and hang around forever. The code below uses the BlockingCollection from the .NET 4.0 BCL. If you are using an earlier version you can replace it with Stephen Toub's BlockingQueue.

Note: Further refactoring may be warranted in your case. This code tries to preserve some common elements from the original.

public class Example
{
    private BlockingCollection<Controller> m_Queue = new BlockingCollection<Controller>();

    public Example()
    {
        for (int i = 0; i < 4; i++)
        {
            var thread = new Thread(
                () =>
                {
                    while (true)
                    {
                        Controller controller = m_Queue.Take();
                        // Do whatever you need to with Contoller here.
                    }
                });
            thread.IsBackground = true;
            thread.Start();
        }
    }

    public void Exec()
    {
        try
        {
            AutoResetEvent autoEvent = new AutoResetEvent(false);
            int vQtd = Queue.GetAllMessages().Length
            while (vQtd > 0)
            {
                m_Queue.Add(new Controller(Queue.Receive(), autoEvent));
            }
        }
        catch (Exception ex)
        {
            ExceptionPolicy.HandleException(ex, "RECOVERABLE");
        }
    }
}

Edit:

Or better yet since MessageQueue is thread-safe:

public class Example
{
    public Example()
    {
        for (int i = 0; i < 4; i++)
        {
            var thread = new Thread(
                () =>
                {
                    while (true)
                    {
                      if (/* between 9am and 6pm */)
                      {
                        Message message = queue.Receive();
                        Controller controller = new Controller(message, /* AutoResetEvent? */);
                        // Do whatever you need to with Contoller here.
                        // Is the AutoResetEvent really needed?
                      }
                    }
                });
            thread.IsBackground = true;
            thread.Start();
        }
    }
}