views:

81

answers:

1

Briefly under a single producer - single consumer scenario, I used a mutable object for synchronization and passing data and messages between producer and consumer. Shared buffer is a ConcurrentQueue of byte arrays. To implement a circular buffer and prevent heap fragmentation and frequent object swapping by GC I used a ConcurrentBag of byte arrays as a recycle-bin for used byte arrays. ManualResetEventSlim is used for thread synchronization. Sometimes I lose reference to byte arrays in my code. Below is a simplified version of my code in case you need more details but I guess this is a routine mistake while working with threads.

MutableObject mutableObject = new MutableObject();

Producer producer = MutableObject.GetProducer();
Consumer consumer = MutableObject.GetConsumer();

Thread fork = new Thread(new ThreadStart(producer.Start));

// Forking execution path
fork.Start();
// Main thread goes here
consumer.Start();


class MutableObject()
{
    private Producer m_producer;
    private Consumer m_consumer;
    private ConcurrentBag<byte[]> m_recycleBin = new ConcurrentBag<byte[]>();
    private ConcurrentQueue<byte[]> m_sharedBuffer = new ConcurrentQueue<byte[]>();

    public Producer GetProducer()
    {
        // Keep a reference to the mutable object
        return new Producer(this);
    }

    // GetConsumer() method is just like GetProducer() method

    public void GetEmptyBuffer(out byte[] buffer)
    {
        if (!m_recycleBin.TryTake(out buffer))
            buffer = new byte[1024];
    }

    public bool Put(byte[] buffer)
    {
        m_sharedBuffer.Enqueue(buffer);
        // Set ManualResetEventSlim for consumer
    }

    public bool Get(byte[] buffer) // Consumer calls this method in a loop
    {
        m_sharedBuffer.TryDequeue(out buffer);
        // I save a reference to buffer here and pass it to recyclebin at next call like this: lastBuffer = buffer;
        // This is because buffers are passing by refrence for I should wait until it would be used by consumer.
        m_recycleBin.Add(lastBuffer);
        // Set ManualResetEventSlim for producer
    }
}

class Producer
{
    private MutableObject m_mutableObject;

    public Producer(MutableObject mutableObject)
    {
        m_mutableObject = mutableObject;
    }

    public void Start()
    {
        byte[] buffer;

        while (true)
        {
            m_mutableObject.GetEmptyBuffer(out buffer);
            m_mutableObject.Put(buffer);
        }
    }
}

Actually GetEmptyBuffer() method frequently creates new buffers and although used buffers are stored in recycle-bin, the recycle-bin count doesn't raise sometimes !

+2  A: 
public bool Get(byte[] buffer)

That would be one obvious place to lose a reference. This method cannot actually return the buffer that was retrieved. You would have to use the ref keyword to let it return the array. Hard to believe that the real code looks anything like this, it just wouldn't work at all. There are lots of other red flags, ConcurrentBag has thread associativity, stuff gets lost if you create consumer threads on the fly. You can't sync a consumer with a producer with a ManualResetEvent, it can only count to 1.

In general, this optimization is inappropriate unless the buffers are bigger than 85KB. Trust the garbage collector, it does an excellent job that is very hard to improve upon.

Hans Passant
Real code is much more complex. I made it brief on the fly and after checking again found that signature is "public bool Get(out byte[] buffer)" but its probabbly associated with out/ref usage. ConcurrentBag is thread safe, so used for holding refrence. Actually I use Monitor for synchronization. Buffers are 1 MB and under performance test I got red flag for frequent object swapping so changed the design. If you know good sample please share it with me. Thanks.
Xaqron
You did not do yourself a favor by posting junk code. And then not fixing it. Good luck with it.
Hans Passant