Click here to Skip to main content
15,890,670 members
Please Sign up or sign in to vote.
2.00/5 (1 vote)
See more:
I have one producer and four consumers. I want four consumers process items from the producer concurrently. However, it is not. It is sequentially instead.
I put all code here. Thanks for help.
C#
using System;
using System.Collections.Concurrent;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading;
using System.Threading.Tasks;

namespace ConcurrentMakeTest
{
    class Program
    {
        public static BlockingCollection<string> m_Queue =
             new BlockingCollection<string>();
        public static Object obj = new Object();
        static void Main(string[] args)
        {
            Task producer = Task.Run(() => Producer());
            Task consumer1 = Consumer();
            Task consumer2 = Consumer();
            Task consumer3 = Consumer();
            Task consumer4 = Consumer();

            Task.WaitAll(producer, consumer1, consumer2, consumer3, consumer4);
            Console.WriteLine("-----------------------------------------");
            Console.ReadLine();
        }
        static void Producer()
        {
            try
            {
                for (int i = 0; i <= 10; i++)
                {
                    m_Queue.TryAdd(i.ToString());
                }
            }
            finally
            {
                m_Queue.CompleteAdding();
                Console.WriteLine("There are totally {0} items in the queue.\n", m_Queue.Count);
                Thread.Sleep(500);
            }
        }
        static async Task Consumer()
        {
            try
            {
                while (m_Queue.Count > 0)
                {
                    string x = "";
                    m_Queue.TryTake(out x);
                    await Task.Factory.StartNew(() =>
                    {
                        lock (obj)
                        {
                            PrintItem m = new PrintItem(DateTime.Now);
                            m.RunScript(x);
                        }
                    });
                }

            }
            catch (Exception ex)
            {
                Console.Write("Exception: " + ex.Message + "\r\n" + ex.StackTrace);
            }
        }
    }
}

And:
C#
public class PrintItem
    {
        public PrintItem(DateTime t)
        {
            Console.WriteLine(t);
        }
        internal void RunScript(string item) 
        {
            Console.WriteLine(item);
        }
    }
Posted
Comments
Sergey Alexandrovich Kryukov 14-Oct-14 16:14pm    
One note: don't use "", use string.Empty. This is the same, only string.Empty is better for maintenance. Avoid hard-coding of immediate constants, except for some simplest cases. Better declare explicit constants (if not variables or read-write members).
—SA

1 solution

Because, most of the time, you they are waiting for each other at the queue at the lock on the same lock object, obj. The code is just not designed too leverage parallelism well enough. Look at what RunScript(x) does? Do the different calls to this method really need to be interlocked and called one at a time? Why? If they don't have to be interlocked, remove the lock. Be careful.

What else to advise? Consider redesign of the code. How? Probably, I would be able to advise some design if I knew it all: your scope, goals, etc.

One idea is: maybe you need to learn threading, thread synchronization, tasks (TPL) and parallelism on some simpler problems first. Even when you perfectly understand the behavior of code, reasonable efficient design is not trivial at all. I cannot say for sure though; maybe your present problem is reasonable enough to learn it all.

—SA
 
Share this answer
 
v2
Comments
[no name] 14-Oct-14 16:26pm    
@SA, I used to remove lock. But the output liked <a href="https://www.flickr.com/photos/67801243@N06/15528088831/">this image</a>. I want to print one datetime then one string, it is the stack's structure. If you remove lock, the three datatime stacked together.
Sergey Alexandrovich Kryukov 14-Oct-14 16:56pm    
I did not mean you should remove the lock using a trial-and-error approach. You should have analyzed the consequences. Maybe your RunScript should be different (thread-safe).

I must tell you: parallelism is such a field when trial-and-error can lead you to real disasters. The code may work for an unpredictably long time and eventually fail. You really need to analyze everything theoretically. Sometime, such analysis is easy. In other cases, it may require advanced mathematics. On such formal analysis is based on the formalism of Petri nets, especially in 1-concervative Petri nets:

Please see: Petri net.

—SA
[no name] 14-Oct-14 17:10pm    
Do I have to make a big change to RunScript()? I run out of idea.
Sergey Alexandrovich Kryukov 14-Oct-14 17:33pm    
You should embrace a different idea: instead of guessing and trying, try to solve the problem is a holistic manner. Chances are, you would need to redesign it all. But first, you should clearly realize, what are you doing in parallel, why. What are bottlenecks and what not. And so on...
You never explained the whole thing from scratch, so, based on your information, I cannot help you to design the code at this moment. At this moment, your design leads you (already existing code or something else), but you should let your problem lead to right design.

I really don't want to discourage you and say that you took a bigger piece than you can swallow, but, if you takes it, you should take it seriously enough.

—SA
[no name] 14-Oct-14 17:47pm    
Originally it was a telephony application. Say we have many customers are trying to make a call through our telephony server. However the server only have N ports/channels, which means at any time we can only take N calls. So it is a concurrent with bottleneck problem. Actually I already had a TPL solution, however the other people in different team will debug the program as well. To them, the learning curve is not a little. So my boss asked me try to use async/await pattern to solve it instead of too modern skills such as .net 4.5 task scheduler. Right now I try to create a simple demo printing project as the entire structure is very similar to the demo. I stuck there. The telephony server will accept a parameter say dialing number(string type). I want to add all customer's dialing number to the queue then parallelism to N tasks.

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)



CodeProject, 20 Bay Street, 11th Floor Toronto, Ontario, Canada M5J 2N8 +1 (416) 849-8900