patterncsharpModerate
Producer/Consumer programs
Viewed 0 times
producerconsumerprograms
Problem
I am studying mutual exclusion in college, and we just covered the producer/consumer problem. The class does not involve writing code, but I decided to implement a bounded buffer version of this problem. I have never written a multi-threaded program before, nor have I written a program with mutual exclusion before, so I decided to request a review here.
I implemented three variations, a busy-waiting variation, a
This is the part of the code that never changes:
This is the busy-waiting producer/consumer and their related global variable:
This is the
```
private static Semaphore isFull = new Semaphore(buffSize, buffSize);
private static Semaphore isEmpty = new Semaphore(0, buffSize);
static void produce()
{
for (int i = 0; i < valuesToProduce; i++)
{
isFull.WaitOne();
buffer[i % buffSize] = (char)(32 + i % 95);
Console.WriteLine("Produced: {0}", buffer[i % buf
I implemented three variations, a busy-waiting variation, a
Semaphore variation, and a Monitor variation. All of these reside in a class named Program, which is needed for the threading. The Monitor variation looks as if there should be a simpler solution with fewer variables. Is this so?This is the part of the code that never changes:
const int buffSize = 10;
static char[] buffer = new char[buffSize];
static int valuesToProduce = 95;
static void Main(string[] args)
{
Thread p = new Thread(new ThreadStart(Program.produce));
Thread c = new Thread(new ThreadStart(Program.consume));
p.Start();
c.Start();
}This is the busy-waiting producer/consumer and their related global variable:
static int avail = 0;
static void produce()
{
for(int i=0; i<valuesToProduce; i++)
{
while (avail == buffSize) { };
buffer[i % buffSize] = (char)(32 + i % 95);
Console.WriteLine("Produced: {0}", buffer[i % buffSize]);
avail++;
}
}
static void consume()
{
for (int i = 0; i < valuesToProduce; i++)
{
while (avail < 1) { };
char c = buffer[i % buffSize];
Console.WriteLine("Consumed: {0}", buffer[i % buffSize]);
avail--;
}
}This is the
Semaphore implementation:```
private static Semaphore isFull = new Semaphore(buffSize, buffSize);
private static Semaphore isEmpty = new Semaphore(0, buffSize);
static void produce()
{
for (int i = 0; i < valuesToProduce; i++)
{
isFull.WaitOne();
buffer[i % buffSize] = (char)(32 + i % 95);
Console.WriteLine("Produced: {0}", buffer[i % buf
Solution
Style
Standard C# naming convention for methods is
Structure
It is all
The easiest way to simplify this is to provide an abstract base class which the various implementations can derive from an implement. You should also pass the buffer size and values to produce/consume as parameters to break dependencies on global variables.
Something along these lines:
this can then be derived like this:
The main method of your program then would be:
And you can easily swap out the concrete implementation you want to test with without having to change or comment out a lot of code.
Bugs
Your busy-waiting implementation is broken in that you will need to at least call
Standard C# naming convention for methods is
PascalCase. For static and instance members there are more variants around but often they are prefixed with _ and/or area also PascalCase so they can be easily distinguished from local variables and parameters. Following standard naming conventions makes your code look more familiar to other C# developers.Structure
It is all
static methods and global variables which seriously hurts maintenance and makes testing the code real painful as you probably have already discovered (I guess you have managed with a lot of commenting in and out code).The easiest way to simplify this is to provide an abstract base class which the various implementations can derive from an implement. You should also pass the buffer size and values to produce/consume as parameters to break dependencies on global variables.
Something along these lines:
public class ProducerConsumerBase
{
protected char[] _Buffer;
protected int _TotalNumberOfValues;
protected ProducerConsumerBase(int bufferSize, int totalNumberOfValues)
{
_Buffer = new char[buffSize];
_TotalNumberOfValues = totalNumberOfValues;
}
public abstract void ProduceAll();
public abstract void ConsumeAll();
}this can then be derived like this:
public class BusyWaitingProducerConsumer : ProducerConsumerBase
{
public BusyWaitingProducerConsumer(int bufferSize, int totalNumberOfValues)
: base(bufferSize, totalNumberOfValues)
{
}
public void ProduceAll()
{
...
}
public void ConsumeAll()
{
...
}
}
public class SemaphoreProducerConsumer : ProducerConsumerBase
{
public SemaphoreProducerConsumer (int bufferSize, int totalNumberOfValues)
: base(bufferSize, totalNumberOfValues)
{
}
public void ProduceAll()
{
...
}
public void ConsumeAll()
{
...
}
}The main method of your program then would be:
static const int BuffSize = 10;
static const int ValuesToProduce = 95;
static void Main(string[] args)
{
var producerConsumer = new BusyWaitingProducerConsumer(BuffSize, ValuesToProduce);
Thread p = new Thread(new ThreadStart(producerConsumer.Produce));
Thread c = new Thread(new ThreadStart(producerConsumer.Consume));
p.Start();
c.Start();
}And you can easily swap out the concrete implementation you want to test with without having to change or comment out a lot of code.
Bugs
Your busy-waiting implementation is broken in that you will need to at least call
Interlocked.Increment and Interlocked.Decrement on avail because ++ and -- are not atomic operations - they consist of a read, an increment/decrement and a store. For example if avail is 1 and avail++ and avail-- are executed on two different threads simultaneously then the end result could be 0, 1 or 2.Code Snippets
public class ProducerConsumerBase
{
protected char[] _Buffer;
protected int _TotalNumberOfValues;
protected ProducerConsumerBase(int bufferSize, int totalNumberOfValues)
{
_Buffer = new char[buffSize];
_TotalNumberOfValues = totalNumberOfValues;
}
public abstract void ProduceAll();
public abstract void ConsumeAll();
}public class BusyWaitingProducerConsumer : ProducerConsumerBase
{
public BusyWaitingProducerConsumer(int bufferSize, int totalNumberOfValues)
: base(bufferSize, totalNumberOfValues)
{
}
public void ProduceAll()
{
...
}
public void ConsumeAll()
{
...
}
}
public class SemaphoreProducerConsumer : ProducerConsumerBase
{
public SemaphoreProducerConsumer (int bufferSize, int totalNumberOfValues)
: base(bufferSize, totalNumberOfValues)
{
}
public void ProduceAll()
{
...
}
public void ConsumeAll()
{
...
}
}static const int BuffSize = 10;
static const int ValuesToProduce = 95;
static void Main(string[] args)
{
var producerConsumer = new BusyWaitingProducerConsumer(BuffSize, ValuesToProduce);
Thread p = new Thread(new ThreadStart(producerConsumer.Produce));
Thread c = new Thread(new ThreadStart(producerConsumer.Consume));
p.Start();
c.Start();
}Context
StackExchange Code Review Q#69451, answer score: 11
Revisions (0)
No revisions yet.