0

Specifically, I'm wondering:

Will the ManualResetEvent consume resources while it is in a wait state? Does the performance degradation of context switching apply to threads that are in a wait state?

If I have a choice to use multiple BackgroundThreadQueues that do less work each, or one BackgroundThreadQueue that does more work, and I choose to use multiple...will the waiting thread queues affect process performance while they are not doing anything?

Is there a better FIFO thread queue I should be using in C#, or a different locking strategy?

Any suggestions are appreciated.

/// <summary>
/// This class is responsible for peforming actions in a FIFO order on a 
/// background thread. When it is constructed, a background thread is created 
/// and a manual reset event is used to trigger actions to be performed when 
/// a new action is enqueued, or when one finishes. There is a ShuttingDown 
/// flag that is set by calling code when it is time to destroy the thread, 
/// and a QueueIsEmpty event is fired whenever the queue finishes executing 
/// the last action.
/// </summary>
public class BackgroundThreadQueue : IBackgroundThreadQueue
{
    #region Fields

    private readonly Queue<Action> queueOfActions = new Queue<Action>();
    readonly ManualResetEvent resetEvent;
    private bool shuttingDown;
    private bool readyToShutdown;
    private readonly object lockObject = new object();
    private string queuName;

    #endregion Fields

    #region Events

    /// <summary>
    /// Occurs when the BackgroundThreadQueue is empty, and ready to shut down.
    /// </summary>
    public event EventHandler IsReadyToShutdown;

    #endregion Events

    #region Constructor

    public BackgroundThreadQueue(string threadName)
    {
        this.resetEvent = new ManualResetEvent(false);
        queuName = threadName;
        StartThread();
    }

    #endregion Constructor

    #region Public Methods

    public void ClearQueue()
    {
        lock (lockObject)
        {
            queueOfActions.Clear();
        }
        resetEvent.Set();
    }

    /// <summary>
    /// Enqueues an action, and calls set on the manual reset event to trigger 
    /// the action to be performed (if no action is currently being performed, 
    /// the one just enqueued will be done immediately, if an action is already 
    /// being performed, then the one just enqueued will have to wait its turn).
    /// </summary>
    public void EnqueueAction(Action actionToEnqueue)
    {
        if (actionToEnqueue == null)
        {
            throw new ArgumentNullException("actionToEnqueue");
        }

        bool localReadyToShutDown = false;
        lock (lockObject)
        {
            queueOfActions.Enqueue(actionToEnqueue);

            if(this.readyToShutdown)
            {
                localReadyToShutDown = true;
                this.readyToShutdown = false;
            }
        }

        //if this instance is ready to shut down...and we just enqueued a 
        //new action...we can't shut down now...
        if (localReadyToShutDown)
        {
            StartThread();
        }
        resetEvent.Set();
    }

    #endregion Public Methods

    #region Public Properties

    public bool ReadyToShutdown
    {
        get
        {
            lock (lockObject)
            {
                return this.shuttingDown && this.readyToShutdown;
            }
        }
        private set
        {
            this.readyToShutdown = value;
            if (this.readyToShutdown)
            {
                //let interested parties know that the queue is now empty 
                //and ready to shutdown
                IsReadyToShutdown.Raise(this);
            }
        }
    }

    /// <summary>
    /// Gets or sets a value indicating whether or not the queue should shut down 
    /// when it is finished with the last action it has enqueued to process.
    /// If the queues owner is shutting down, it needs to notify the queue,
    /// and wait for a QueueIsEmpty event to be fired, at which point the reset 
    /// event will exit ... the owner shouldn't actually destroy the queue 
    /// until all actions have been performed.
    /// </summary>
    public bool ShuttingDown
    {
        get
        {
            lock (lockObject)
            {
                return this.shuttingDown;
            }
        }
        set
        {
            lock (lockObject)
            {
                bool startThread = false;
                if (value == false)
                {
                    readyToShutdown = false;
                    //if we were shutting down...but, now are not
                    startThread = this.shuttingDown;
                }

                this.shuttingDown = value;

                //if we were shutting down, but now are not...
                //we need to restart the processing actions thread
                if (startThread)
                {
                    StartThread();
                }
            }

            this.resetEvent.Set();
        }
    }

    #endregion Public Properties

    #region Private Methods

    private void StartThread()
    {
        var processActionsThread = new Thread(this.ProcessActions);
        processActionsThread.Name = queuName;
        processActionsThread.IsBackground = true;
        processActionsThread.Start();            
    }

    /// <summary>
    /// Processes the actions in a while loop, resetting a ManualResetEvent that 
    /// is triggered in the EnqueueAction method and ShuttingDown property.
    /// </summary>
    private void ProcessActions()
    {
        while (true)
        {
            Action action = null;
            lock (lockObject)
            {
                //if there are any actions, then get the first one out of the queue
                if (queueOfActions.Count > 0)
                {
                    action = queueOfActions.Dequeue();
                }
            }
            if (action != null)
            {
                action();
            }
            lock (lockObject)
            {
                //if any actions were added since the last one was processed, go 
                //back around the loop and do the next one
                if (this.queueOfActions.Count > 0)
                {
                    continue;
                }

                if (this.shuttingDown)
                {
                    //ReadyToShutdown setter will raise IsReadyToShutdown
                    ReadyToShutdown = true;
                    //get out of the method if the user has chosen to shutdown, 
                    //and there are no more actions to process
                    return;
                }                    
                this.resetEvent.Reset();
            }

            this.resetEvent.WaitOne();
        }
    }

    #endregion Private Methods
}
Eric Dahlvang
  • 8,252
  • 4
  • 29
  • 50
  • 2
    Do you have a specific perf problem or is this just a "here's a blob of code, review it" request? The codereview.stackexchange.com site is best for that. – Hans Passant Feb 18 '12 at 17:48
  • Have to agree with Hans, seems like something for codereview. – Lloyd Feb 18 '12 at 17:52
  • Well, I don't ask a lot of questions on Stack Overflow, and I had reviewed the FAQ before posting this...which states one of the requirements as: " if your question generally covers...a software algorithm" which is what this is. And, I have specific questions about the functionality of a .net object: the ManualResetEvent. – Eric Dahlvang Feb 18 '12 at 18:09
  • @HansPassant After your suggestion of posting this on codereview.stackexchange.com, I posted an entirely different question: http://stackoverflow.com/questions/9344028/will-the-manualresetevent-consume-cpu-while-it-is-in-a-wait-state only to be suggested that I add a code sample. Please, SOMEONE SHOOT ME NOW!!! – Eric Dahlvang Feb 18 '12 at 18:58

2 Answers2

1

Threads that are blocked in a call to ManualResetEvent.WaitOne() are taken off the CPU and not considered again for scheduling by the OS until the event that is supposed to wake them up (i.e. a call to Set()) occurs. Therefore, while waiting to be signaled they are inactive and do not consume CPU cycles.

Tudor
  • 61,523
  • 12
  • 102
  • 142
0

The mere existence of waiting threads should have no ongoing performance impact beyond the memory reserved to house the thread's actual stack, and whatever accounting information the runtime or kernel keeps regarding them. So no, truly waiting threads aren't going to do anything except consume RAM.

That said, I'm not sure why you'd write this code, as .Net has a threadpool built in, and concurrency tools that you should probably prefer over your own.

Jerome Haltom
  • 1,670
  • 2
  • 17
  • 23
  • I realize that .Net has a built in ThreadPool, however, creating a thread takes half a second if there isn't one in the pool available. The consumers of the thread cannot take the chance of having to wait. What concurrency tools are you referring to exactly? I need separate threads to perform actions in order, and I need to know when those actions are completed. I would definitely prefer to use something pre-built, if you have a specific suggestion. – Eric Dahlvang Feb 18 '12 at 17:33
  • 3
    I'd suggest looking at the Task Parallel Library. Your actions are represented by a Task instance, and they can be strung together to represent dependencies between Tasks. So, when on finishes, it's dependents will run. – Jerome Haltom Feb 18 '12 at 17:46
  • I use the task parallel library for other things. It wouldn't work for this due to the things mentioned in the comment above, as well as the fact that continuations are not guaranteed to be on the same thread. (Correct me if i'm wrong). – Eric Dahlvang Feb 18 '12 at 18:23
  • 1
    Well, I don't want to get into an argument over this. If you know your implementation is what's needed, that's fine. The TPL does however let you define custom Task factories, custom Schedulers, and the like. It lets you implement your specifics (threading, etc) within a defined interface. I find value in that, so I try to use it. – Jerome Haltom Feb 18 '12 at 19:03
  • Thanks for your help. I will look more into other aspects of the TPL. – Eric Dahlvang Feb 19 '12 at 17:37