views:

163

answers:

4

Lets say I have a class that is supposed to generate some ID (for example GUID) for me. Now unfortunately the ID generation is a somewhat long process and if I need a hundred of those I run into a problem of significant slowdowns. In order to avoid those, I keep a queue of pre-generated ID, and when this queue starts to run down on them I use the BackgroundWorker to generate new ones and place them in the queue. But there are some problems I've run into. The biggest one at the moment is how to make sure that in case the queue compleatelly runs out on IDs the main thread waits for the BackroundWorker to generate and place them in the queue. Heres the code that I have at the moment.

public class IdGenerator
{
    private Queue<string> mIds = new Queue<string>();
    private BackgroundWorker mWorker = new BackgroundWorker();
    private static EventWaitHandle mWaitHandle = new AutoResetEvent(false);

    public IdGenerator()
    {
        GenerateIds();

        this.mWorker.DoWork += new DoWorkEventHandler(FillQueueWithIds);
    }

    private void GenerateIds()
    {
        List<string> ids = new List<string>();

        for (int i = 0; i < 100; i++ )
        {
            ids.Add(Guid.NewGuid().ToString());
        }

        lock (this.mIds)
        {

            foreach (string id in ids)
            {
                this.mIds.Enqueue(id);
            } 
        }            
    }

    public string GetId()
    {
        string id = string.Empty;

        lock (this.mIds)
        {
            if (this.mIds.Count > 0)
            {
                id = this.mIds.Dequeue();
            }

            if (this.mIds.Count < 100)
            {
                if (!this.mWorker.IsBusy)
                {
                    this.mWorker.RunWorkerAsync();
                }
            }
        }

        if (this.mIds.Count < 1)
        {
            mWaitHandle.WaitOne();
        }

        return id;
    }

    void FillQueueWithIds(object sender, DoWorkEventArgs e)
    {
        GenerateIds();
        mWaitHandle.Set();   
    }
}

Obviously it doesn't work correctly. It seems that I have a problem with proper timing for calling WaitOne and Set methods. And sometimes the IsBusy property returns true even though the worker has already completed his work.


EDIT:

Its a WinForm and I'm required to use .NET 2.0

+1  A: 

The problem you have is the classic Producer-Consumer problem. Take a look at http://en.wikipedia.org/wiki/Producer-consumer_problem

A simple explanation is that you will have two threads. One will be the producer (the GUID generator) and the other will be the consumer.

You will keep these threads in synch through the use of semaphores. The semaphore will be the responsible to stop the producer when the queue is full and to stop the consumer when it is empty.

The process is all very well explained at the Wikipedia article and I bet you can find a basic implementation of Producer-Consumer in c# on the internet.

Edu
Thanks for the tip. I'll look into it.
L.E.O
+1  A: 

There are some bugs related to thread sync, see in changed code below. When you apply lock sync to queue pay attention to put under lock all uses of queue. I've changed GetId method to probe for new ids if there are none.

public class IdGenerator
{
    private Queue<string> mIds = new Queue<string>();
    private BackgroundWorker mWorker = new BackgroundWorker();
    private static EventWaitHandle mWaitHandle = new AutoResetEvent(false);

    public IdGenerator()
    {
        GenerateIds();

        this.mWorker.DoWork += new DoWorkEventHandler(FillQueueWithIds);
    }

    private void GenerateIds()
    {
        List<string> ids = new List<string>();

        for (int i = 0; i < 100; i++ )
        {
            ids.Add(Guid.NewGuid().ToString());
        }

        lock (this.mIds)
        {

            foreach (string id in ids)
            {
                this.mIds.Enqueue(id);
            } 
        }            
    }

    public string GetId()
    {
        string id = string.Empty;
        //Indicates if we need to wait
        bool needWait = false;

        do
        {
            lock (this.mIds)
             {
                if (this.mIds.Count > 0)
                {
                    id = this.mIds.Dequeue();
                    return id;
                }

                if (this.mIds.Count < 100 && this.mIds.Count > 0)
                {
                    if (!this.mWorker.IsBusy)
                    {
                        this.mWorker.RunWorkerAsync();
                    }
                } 
                else 
                {
                    needWait = true;
                }
            }

            if (needWait)
            {
                mWaitHandle.WaitOne();
                needWait = false;
            }
        } while(true);

        return id;
    }

    void FillQueueWithIds(object sender, DoWorkEventArgs e)
    {
        GenerateIds();
        mWaitHandle.Set();   
    }
}
Vadmyst
OK, noted, thanks. Thought it still doesn't work...
L.E.O
+2  A: 

In .NET 4 you can use the BlockingCollection<T> and more generically IProducerConsumerCollection<T>

Here's an example of 2 tasks, one adding and the other taking, using it.

http://msdn.microsoft.com/en-us/library/dd997306.aspx

James Manning
Sounds like something I could use... if I wasn't required to stick to .NET 2.0
L.E.O
A: 

OK, heres the final solution I went with. This one doesn't use the BackgroundWorker, but it works. Thanks to Edu who pointed to the Producer-Consumer problem. I used the example provided by MSDN located here.

L.E.O