Jump to content
  • Advertisement
Sign in to follow this  
Anfaenger

Work queue with condition variable - design issue

This topic is 762 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

I've written a basic work queue for background loading:

void ThreadFunction()
{
	while( isRunningFlag ) // 'volatile bool'
	{
		ASlowTask* taskTaken = 0;
		{
			SpinLock	scopedLock( pendingTasksCS );
			// Wait on the condition variable until there are items in the queue.
			while( pendingTasks.IsEmpty() ) {
				taskQueueNonEmptyCV.Wait( pendingTasksCS );
			}
			// Pop a work item off of the queue.
			taskTaken = pendingTasks.PopLastValue();
		}
		// Handle the work item.
		taskTaken->Execute();
		// Add it to the 'finished-items' queue
		SpinLock	scopedLock( completedTasksCS );
		completedTasks.Add( taskTaken );
	}
}
void Shutdown()
{
    // wake up and exit the background thread
    isRunningFlag = false;    //!<= must be done before signalling!
    taskQueueNonEmptyCV.NotifyOne();    // notify the thread ...
    backgroundThread.Shutdown();    // ... and wait for it to finish
}

When I call Shutdown(), the application hangs waiting for the background thread to finish,

because there are no items in the 'pendingTasks' queue and the thread won't be able to exit the inner loop (when 'pendingTasks.IsEmpty()').

 

Then I fixed the problem by adding two checks for isRunningFlag:

// in ThreadFunction():
// added special cases just to be able to exit the loop
while( pendingTasks.IsEmpty() && isRunningFlag ) {
	taskQueueNonEmptyCV.Wait( pendingTasksCS );
}
if( !isRunningFlag ) {
	break;	// the queue could be empty
}

Another solution would be to add a dummy task so that the thread could break on 'pendingTasks.IsEmpty()' and gracefully exit.

 

Both of them are ugly.

 

1) Is there any elegant/clean/'canonical' way of exiting the thread?

Is using a 'volatile bool' a good practice?

 

2) How can I implement priority-based task execution?

 

I'm thinking of organizing tasks in linked lists by their priorities, e.g. list heads are stored in a small fixed-size array [NumPriorities],

where enum { LoadPriorityHigh = 0, LoadPriorityLow = 15, NumPriorities }, and the thread will dequeue tasks from array slots starting with zero (the highest priority).

Edited by Anfaenger

Share this post


Link to post
Share on other sites
Advertisement

I've done something similar recently (not the priority side).

 

I actually made an atomic enum that tracked the state of the worker thread.

enum class WorkerThreadState : int
{
	CheckingForJob,
	RunningJob,
	Sleeping,
	Terminating,
	ReadyToJoin,
	Joined
};

std::atomic<WorkerThreadState> State{WorkerThreadState::CheckingForJob};

I could then use the atomic compare_exchange commands to test the state to see if it was safe to change or if it had changed to a terminating state, leave it alone.

And since I had a reference to the state outside of the thread, I could query what the thread was doing at the moment.

 

[edit]

Thread would start as CheckingForJob.  It would atomically check if the State was still CheckingForJob before checking the queue.  If it wasn't, then it was likely triggered to shutdown (Terminating).  This could have happened while processing a previous job.  Once a job was retrieved, it was set to RunningJob.  If no jobs were available, it would be set to Sleeping and wait for a condition variable to be triggered to check again.  The controlling thread would set the variable to Terminating when it was ready for it to shut down.  When set to Terminating, once whatever job it was working on was complete, it would stop the loop and set itself as ReadyToJoin.  A function outside the WorkerThread would check the vector of worker threads for threads that were ReadyToJoin and call join on them, then remove them from the vector.

Edited by Rattrap

Share this post


Link to post
Share on other sites

To implement queue and tasks priorities I didn't use queue but ring buffer. But in my case I had single producer and single consumer ( for more producers external synchronization is needed ). So each task written to the buffer contains priority. When consumer thread wakes up it moves atomically the "read pointer" forward and collects all new tasks. Then it does sorting ( well, not exactly sorting ) and executes them based on priority. Good thing that it doesn't require locking and single atomic read and write pointer do the job ( of course as long as ring buffer is large enough not to start overwriting items which haven't been processed yet ;) ). Exiting the thread loop is actually done by the task. High priority termination task breaks the loop. In my case I needed something really simple and fast, so there's nothing like thread state to be read from outside etc.

Share this post


Link to post
Share on other sites

Generally speaking, I've fiddled with variations of the shutdown problem and come to the conclusion that the best solution of the various ugly ones is to treat the shutdown as nothing more than another task.  So, the loop uses the first variation of "while (running)" without any other special code checks.  The owner of the thread simply implements a special "exit" task which changes the value of running.  With this, 'running' does not need to be volatile since it is modified within thread so it cleans up that little annoyance.  So, shutdown becomes:

 

post(exitTask);

thread.Join();

 

That's about as clean as you are going to get with threads in this area.

 

 

As to the priorities item.  I will warn you that priority systems are a massive pain to get correct and you might be better off not doing that and reconsidering the issue you are trying to solve.  I gave up on prioritized tasking for a number of reasons, the first reason is simple; doing it correctly is expensive and I wanted the performance back.  Generally speaking, for game work, I look at priorities as a fix for something that breaks my preferred separation of responsibilities approach.  Let me try and explain this a bit better.

 

So, the basic reasons I ended up wanting priorities turned out that I had some tasks that I didn't care if they finished this frame or a frame or two in the future, but I had a consistent need of many tasks executing in the frame and I could not complete the frame until those finished.  So, after considerable trial and error, I ended up with a frame work system and thread pool working in conjunction.  Making the two items work together is a bit of a trick but generally speaking, much easier than getting priorities correct in a single solution.  Sorry I can't suggest a solution to your actual problem and only suggest a different solution all together..

Share this post


Link to post
Share on other sites

edit: never mind I think there was a mistake in my post (it was older code after all)...

Edited by Ryan_001

Share this post


Link to post
Share on other sites

Thanks for all the answers!

I'll go with the 'dummy-task-just-to-exit-the-loop' approach.

My actual problem was creating/generating/loading/meshing voxel chunks as the player moves through the world (or growing the loaded area when a new world is created).

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

We are the game development community.

Whether you are an indie, hobbyist, AAA developer, or just trying to learn, GameDev.net is the place for you to learn, share, and connect with the games industry. Learn more About Us or sign up!

Sign me up!