2
\$\begingroup\$

I have seen many examples which the results of a Producer/Consumer queue implemented using BlockingCollection<T> are independent, e.g. each result can be consumed independently in parallel. For example, here the produces calculate a square root and consumers grab each result to update the UI.

I wonder whether explicit locking can be avoided if the consumers can NOT independently process each result calculated by producers. I created a demo example of this where each result has to be processed taking into account previous processed results in order to either insert a row to the data grid or update an existing data grid; it seems to work ok on some small data.

My questions are:

  1. Can explicit locking be avoided?

  2. How to encapsulate consumer and producer within a single object e.g. ProducerConsumerQueue which currently is really a producer queue.

  3. Currently I use a separate task to process each result especially given it ultimately has to post back to the UI thread; is this a waste?

Below is the code behind a simple WinForm which only has a DataGridView and a button.

    using System;
    using System.Collections.Concurrent;
    using System.Threading;
    using System.Threading.Tasks;
    using System.Windows.Forms;

    namespace PCQueueDemo
    {
        public partial class Form1 : Form
        {
            // to consume results that are depedent
            private ConcurrentDictionary<int, WorkItem> _dict = new ConcurrentDictionary<int, WorkItem>();

        private readonly object _sync = new object();
        private BindingSource _bindingSource = new BindingSource();
        private TaskScheduler _ui; 
        private ProducerConsumerQueue _producers;
        private BlockingCollection<WorkItem> _workQueue;
        public Form1()
        {
            InitializeComponent();
            _workQueue = new BlockingCollection<WorkItem>();

            // create 3 producers which will produce work item to the work queue.
            _producers = new ProducerConsumerQueue(3, _workQueue);

            // capture the UI context in order to update data grid.
            _ui = TaskScheduler.FromCurrentSynchronizationContext();

            // bind the data grid
            dataGridView.DataSource = _bindingSource;
            dataGridView.AutoSize = true;
            dataGridView.Dock = DockStyle.Fill;
        }

        // Run button click handler
        private void button1_Click(object sender, EventArgs e)
        {
            _producers.Produce(); // start producing on each producer thread.
            var ignore = Task.Run(() => updateGrid()); // consume the producer result to update the grid.
        }

        private void updateGrid()
        {
            foreach (var item in _workQueue.GetConsumingEnumerable())
            {
                WorkItem copy = item;

                //? Is it a waste to use a thread for each result?

                Task.Factory.StartNew(() =>
                {
                    WorkItem localItem;
                    if (_dict.TryGetValue(item.Number, out localItem))
                    {
                        //? how to avoid locking if the update here takes longer than simply incrementing the counter?
                        lock (_sync) localItem.Counter++;
                    }
                    else if (_dict.TryAdd(copy.Number, copy))
                    {
                        //? Is this negligible since the lock should be very short?
                        lock(_sync) _bindingSource.Add(copy);
                    }
                    dataGridView.Refresh();
                }, CancellationToken.None, TaskCreationOptions.None, _ui);
            }
        }
    }

    // a demo work item
    public class WorkItem
    {
        public WorkItem(int number)
        {
            Number = number;
            Counter = 1; // 1st appearance
        }
        public int Number { get; private set; }
        public int Counter { get; set; }
    }

    //? This is just a producer queue; how to encapsulate the consumer together with producer while allowing it to update the UI?
    public class ProducerConsumerQueue
    {
        private readonly int _producerCount;
        private Task[] _producers;
        private BlockingCollection<WorkItem> _workQueue;

        public ProducerConsumerQueue(int producerCount, BlockingCollection<WorkItem> workQueue)
        {
            _workQueue = workQueue;
            _producerCount = producerCount;
            _producers = new Task[_producerCount];
        }

        public void Produce()
        {
            for (int i = 0; i < _producerCount; i++)
                _producers[i] = Task.Run(() => runProducer());

            Task.WhenAll(_producers).ContinueWith(_ => _workQueue.CompleteAdding());
        }

        private void runProducer()
        {
            // to demo just run for 5 seconds on each producer
            var fiveSecondsLater = DateTime.Now.AddSeconds(15);
            var random = new Random();
            while (!_workQueue.IsAddingCompleted && DateTime.Now <= fiveSecondsLater)
            {
                _workQueue.Add(new WorkItem(random.Next(21)));
                Thread.Sleep(100);
            }
        }
    }
}
\$\endgroup\$
  • 2
    \$\begingroup\$ The real question is why? What are you trying to achieve? Does this have any particular purpose or is this just an experiment? \$\endgroup\$ – t3chb0t Apr 18 '17 at 16:43
  • \$\begingroup\$ @t3chb0t yes it does have a purpose as I need to implement this for a real app which of course has way more complex business logic. \$\endgroup\$ – stt106 Apr 18 '17 at 16:45
4
\$\begingroup\$

Can explicit locking be avoided?

Sure, you can use immutable data structures with interlocked replacements. It's important to note that locking isn't bad, though - even BlockingCollection<T> uses locks internally.

I'm not a huge fan of lockfree algorithms because they're incredibly complex, particularly with more relaxed memory models.

How to encapsulate consumer and producer within a single object e.g. ProducerConsumerQueue which currently is really a producer queue.

I would say it's a queue combined with producer logic. I'm not sure if you'd want to combine producer and consumer logic within the same class.

Currently I use a separate task to process each result; is this a waste?

For general producer/consumer systems, I would say no. In this particular case, I'd say yes, because every consumer task is run on the UI thread. This means they run one at a time. As a consequence of this, they do not need locks.

\$\endgroup\$
  • \$\begingroup\$ Your link is pure gold ! \$\endgroup\$ – Heslacher Apr 19 '17 at 8:27
  • \$\begingroup\$ Thanks Stephen. A few follow up questions about original question 3 based on your answer: 1) I agree that if each result ultimately has to be used to update UI, it may not be worth using a separate task/thread to consume each result since this would increase context switch a lot when many results are produced. But using UI thread to handle all results, doesn't it make the UI unresponsive sometimes, which is generally not ideal? \$\endgroup\$ – stt106 Apr 19 '17 at 18:37
  • \$\begingroup\$ 2) Assume there is NO UI now, but the assumption of there is dependency between results still holds. If now using a separate thread to handle each result, it will have to use locking as well, for instance, to update the Counter property of WorkItem. Effectively, this means many results leading to many threads and many locks; in such case, is it still worth using separate threads compared with using a single thread to handle all results without any lock? \$\endgroup\$ – stt106 Apr 19 '17 at 18:42
  • \$\begingroup\$ 1) UI updates have to be done on the UI thread. 2) If you have to lock for the entire processing, then you may as well just use one thread. Multiple threads are only useful if they can run independently. \$\endgroup\$ – Stephen Cleary Apr 19 '17 at 18:53
3
\$\begingroup\$

Can explicit locking be avoided?

The answer to that question is a question...if a shared resource at anytime could be accessed by one or more threads/processes, then yes an explicit lock(ing) should be used to keep state and more importantly data sanity consistent between operations.

How to encapsulate consumer and producer within a single object e.g. ProducerConsumerQueue which currently is really a producer queue.

Any design is viable, but if you can make each operation atomic and not dependent on the other it is easier to test and maintain. That doesn't mean you can't create a wrapper/manager class which can handle both atomic operations.

For example I am about to publish on GitHub/Nuget a named pipe wrapper class which has a server and a client consumer all one class.

Currently I use a separate task to process each result; is this a waste?

No, I created a General Ledger Migration code where each thread handled a specific set of data operation during migration from database to database. Each one of those tasks, were a separate thread which I had a generalized class which would report status and process operations once fed the target of the item.

The program remained responsive as up to 140 concurrent threads were created and run as the program created each 1000+ operations.


The key is separation of concerns and no one thread should block another and it should report status as well as handle/report all errors known or unknown. Keep all business logic in separate projects away from the main app to be unit testable if not able to run in other programs.

Doing the separation of concerns from the ground up will pay off when the program is tested and ultimately run.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.