multi-thread c# application hitting high CPU usage

Question

I am working on an application that will connect to x number of hardware devices. I have designed the ReaderLayer such that there is a dedicated thread where the code runs to connect to a single device and continuously read the device data buffer. The code for the reader layer is as follows:

        while (true)
        {

                // read buffer from the reader
                IList<IRampActiveTag> rampTagList = ConnectedReader.ReadBuffer();
                if (rampTagList != null && rampTagList.Any())
                {
                    // trigger read event handler
                    if (ReadMessage != null)
                        ReadMessage(this, new RampTagReadEventArg(rampTagList));
                }


        }

There is a Logic Layer built on top of the Reader Layer that is responsible for processing the data received from the Readers and forwarding it via HTTP Post. It has y number of threads each running a separate Logic Block that has to process relevant information that gets written to its thread-safe queue. The logic layer subscribes to the ReadForwardevent that is exposed by the Reader Layer and forwards that data to the relevant logic block by writing to its ConcurrentQueue.

The code in each of the Logic Block is pretty straightforward:

        public void ProcessLogicBuffer()
        {

            while (true)
            {   
                // Dequeue the list
                IRampActiveTag tag;
                LogicBuffer.TryDequeue(out tag);
                if (tag != null)
                {

                    ProcessNewTagReceivedLogic(tag);
                    Console.WriteLine("Process Buffer #Tag {0}. Buffer Count #{1}", tag.NewLoopId, LogicBuffer.Count);
                }

            }
        }

The layout of the loop is mostly the same for both the Reader Layer and the Logic Layer while(true). However when I tested with 3 readers and 3 logic blocks, I found that my CPU utilization jumped up to 77%. I quickly narrowed down the CPU usage to Logic Threads as I received 50% usage for 2 and 25% usage for 1 block.

I am able to bring down the CPU usage to ~3% simply by adding in a Thread.Sleep(100) in the logic thread with 3 blocks, however, I am worried that my logic could be out of synch. By looking at the sample could anyone please suggest to me any improvements as the production code will need to work with about 30 logic blocks. Do I need to change my architecture?

Answer 1

You're doing a whole lot of needless polling in this loop:

    public void ProcessLogicBuffer()
    {

        while (true)
        {   
            // Dequeue the list
            IRampActiveTag tag;
            LogicBuffer.TryDequeue(out tag);
            if (tag != null)
            {

                ProcessNewTagReceivedLogic(tag);
                Console.WriteLine("Process Buffer #Tag {0}. Buffer Count #{1}", tag.NewLoopId, LogicBuffer.Count);
            }

        }
    }

Assuming that the queue is empty most of the time, most of what this code does is repeatedly check the queue. "Is there anything for me? How 'bout now? Now? ..."

You can get rid of all that useless polling by replacing your ConcurrentQueue with a BlockingCollection. Assuming that you change LogicBuffer to a BlockingCollection, the loop then looks like this:

    public void ProcessLogicBuffer()
    {
        foreach (var tag in LogicBuffer.GetConsumingEnumerable())
        {
            ProcessNewTagReceivedLogic(tag);
            Console.WriteLine("Process Buffer #Tag {0}. Buffer Count #{1}", tag.NewLoopId, LogicBuffer.Count);
        }
    }

GetConsumingEnumerable will dequeue items as they arrive, and will continue to do so until the collection is empty and has been marked as complete for adding. See BlockingCollection.CompleteAdding. The real beauty, though, is that GetConsumingEnumerable does a non-busy wait. If the collection is empty, it waits for a notification that an item is available. It doesn't do a lot of useless polling with TryDequeue.

Changing code that uses ConcurrentQueue so that it uses BlockingCollection instead is pretty easy. You can probably do it in under an hour. Doing so will make your code a lot simpler, and it will eliminate needless polling.

Update

If you need to do some periodic processing, you have two choices. If you want that done in the same loop that's reading the BlockingCollection, then you can change the loop that uses GetConsumingEnumerable to something like:

Stopwatch sw = Stopwatch.StartNew();
TimeSpan lastProcessTime = TimeSpan.Zero;
while (true)
{
    IRampActiveTag tag;
    // wait up to 200 ms to dequeue an item.
    if (LogicBuffer.TryTake(out tag, 200))
    {
        // process here
    }
    // see if it's been 200 ms or more
    if ((sw.ElapsedMilliseconds - lastProcessTime.TotalMilliseconds) > 200)
    {
        // do periodic processing
        lastProcessTime = sw.Elapsed;
    }
}

That will give you periodic processing rate in the range of 200 to 400 ms. It's kind of ugly, in my opinion, and might not be good enough for your purposes. You could reduce the timeout to 20 ms instead of 200, which will give you closer to your 200 ms polling rate, at the cost of 10 times as many calls to TryTake. Likely, you wouldn't notice a difference in CPU usage.

My preference would be to separate that periodic processing loop from the queue consumer. Create a timer that fires every 200 ms, and have it do the work. For example:

public void ProcessLogicBuffer()
{
    var timer = new System.Threading.Timer(MyTimerProc, null, 200, 200);

    // queue processing stuff here

    // Just to make sure that the timer isn't garbage collected. . .
    GC.KeepAlive(timer);
}

private void MyTimerProc(object state)
{
    // do periodic processing here
}

That will give you an update frequency of very close to 200 ms. The timer proc will be executed on a separate thread, true, but the only time that thread is active will be when the timer fires.

Answer 2

It's simple really, you never yield control, so your code is always executing, so each thread of your code is using 100% of 1 core (so 25% of your cpu if you have 4 cores).

There's no magic there, Windows doesn't "try to guess" you want to wait and throttle you, you need to explcitely wait, you'd need to show the implementation of ProcessNewTagReceivedLogic but i'm guessing "somewhere" you're making a network connection, instead of polling (just checking and again untill you get non null), you want to call something that actually holds and yields untill it gets an answer.