Jump to content
  • Advertisement
Sign in to follow this  
maya18222

Threadpool design

This topic is 2628 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

First off, Im not trying to reinvent the wheel, I'm just doing this as an exercise.

Below is my ThreadPool class and first of all I was looking for some feedback on it. Secondly, you'll notice that queued tasks can only be dispatched to available threads in 2 places, when they are queued and then in the wait call. How would I go about implementing it so that tasks get dispatched as soon as a thread is free? Im thinking I'll need to have the ThreadPool operating on a thread of its own.

Thread Pool Code


#pragma once

#include <windows.h>
#include <vector>

template< int N >
class ThreadPool
{
public:
ThreadPool()
{
for( int i = 0; i < N; ++i )
{
m_Threads = CreateThread( 0, 0, Loop, &m_ThreadStates, 0, 0 );
}
}

~ThreadPool()
{
for( int i = 0; i < N; ++i )
{
TerminateThread( m_Threads, 0 );
CloseHandle( m_Threads );
}
}

void Queue( LPTHREAD_START_ROUTINE func, void* pParams )
{
Task t = { func, pParams };
m_Tasks.push_back(t);

for( int i = 0; i < N; ++i )
{
if( !m_ThreadStates.Working )
{
const Task& t = m_Tasks.back();
m_ThreadStates.SetTask( t );
m_Tasks.pop_back();
return;
}
}
}

void WaitForAll()
{
while(true)
{
bool WorkStillinProcess = false;
for( int i = 0; i < N; ++i )
{
if( m_ThreadStates.Working )
{
WorkStillinProcess = true;
}
else if( !m_Tasks.empty() )
{
const Task& t = m_Tasks.back();
m_ThreadStates.SetTask( t );
m_Tasks.pop_back();
WorkStillinProcess = true;
}
}

Sleep(0);

if( !WorkStillinProcess )
return;
}
return;
}

private:

static DWORD WINAPI Loop( void* pParams )
{
ThreadState* pts = (ThreadState*)pParams;
while(true)
{
if( pts->HasTask() )
{
pts->DoTask();
pts->SetAvaliable();
}

Sleep(0);
}
}

struct Task
{
LPTHREAD_START_ROUTINE Workfunc;
void* pWorkParams;
};

struct ThreadState
{
ThreadState() : Working(0), Workfunc(0), pWorkParams(0) {}

bool HasTask()
{
return Workfunc != 0;
}

void SetTask( const Task& t )
{
Working = 1;
Workfunc = t.Workfunc;
pWorkParams = t.pWorkParams;
}

void DoTask()
{
Workfunc( pWorkParams );
}

void SetAvaliable()
{
Working = 0;
Workfunc = 0;
pWorkParams = 0;
}

int Working;
LPTHREAD_START_ROUTINE Workfunc;
void* pWorkParams;
};

ThreadState m_ThreadStates[N];
HANDLE m_Threads[N];
std::vector<Task> m_Tasks;
};


Usage


#include <iostream>
using namespace std;

#include "ThreadPool.h"

const unsigned int COUNT = 100000000;
const unsigned int THREADS = 4;

struct WorkParams
{
float begin;
float end;
unsigned int count;
float result;
float pad[16];
};

DWORD WINAPI ComputeAreaUnderCurve( void* pParams )
{
WorkParams* plParams = (WorkParams*)pParams;
double Area = 0.0f;
double dx = ( plParams->end - plParams->begin ) / plParams->count;
for( unsigned int i = 0; i < plParams->count; ++i )
{
double x = plParams->begin + (i * dx);
Area += dx * x * x;
}
plParams->result = (float)Area;
return 0;
}

int main()
{
ThreadPool<THREADS> threadpool;

while(true)
{
WorkParams wp[THREADS];
float Result = 0.0f;

for( int i = 0; i < THREADS; ++i )
{
wp.begin = i * (1.0f / THREADS);
wp.end = (i+1) * (1.0f / THREADS);
wp.count = (COUNT / THREADS);
threadpool.Queue( ComputeAreaUnderCurve, &wp );
}

threadpool.WaitForAll();

for( int i = 0; i < THREADS; ++i )
{
Result += wp.result;
}

cout << "Result : " << Result << "\n";
}

cin.get();
}




Share this post


Link to post
Share on other sites
Advertisement
Leaving everything else aside, its seems folly to use template parameters to determine the number of threads in the pool. Remember that templates are instantiated at compile time, one copy for each combination of template arguments. You don't really need separate copies for 1,2,4,8... capacity pools, and since you're kicking off (presumably) large, or at least non-trivial, jobs there's no argument against "overhead" involved in managing a dynamic number of threads.

From an API perspective, how does one set job priority? Thread affinity?

As for processing jobs as soon as a thread is able, you could use a job complete callback which would call on the job manager to kick off the next job as one way -- its non-deterministic though, which might be a problem in environments that expect less fluctuation (games, especially on consoles). The second approach is to just accept that the job manager operates on a fixed time step -- either along with some other fixed timestep (eg, framerate or physics time-step) or as a result of a timer-based callback or thread.

Share this post


Link to post
Share on other sites
You should check out http://software.intel.com/en-us/articles/do-it-yourself-game-task-scheduling/

Share this post


Link to post
Share on other sites
The ThreadPool class needs to be non-copyable (e.g. add a private, non-implemented copy-constructor and assignment-operator).
The destructor should wait for the threads to finish instead of calling TerminateThread.

The Queue function is not thread-safe -- only one thread can call this safely, but this fact is not documented with a comment.

The ThreadState logic contains dangerous race-conditions and is currently not safe to use:

Main thread calls:
m_ThreadStates.SetTask( t );
Which expands to:
Workfunc = t.Workfunc;//Has Task now begins returning true
pWorkParams = t.pWorkParams;//But DoTask is not safe to call until this line is finished

Worker Thread:
...
if( pts->HasTask() ) // returns Workfunc != 0
{
pts->DoTask(); // calls Workfunc( pWorkParams )

Share this post


Link to post
Share on other sites
Thanks for the comments.

Ravyne - Yeah, the template is unnecessary, that was actually being used at an earlier point for something else. I hadn't really thought about job priority as I was thinking more along the lines of important jobs will need to be added first.

_swx_ - Thanks for the link

Hodgeman - Sorry, I should have mentioned how the ThreadPool is to be used. The threads never exit as they contain an infinite loop, by calling "wait" in the "main thread" before the end of the ThreadPool objects scope the threads can be terminated safely as all work is complete. If the threads exit when no more tasks are *currently* available then the threadpool cant be used again . And yes, the Queue function is designed to only be called on the thread the ThreadPool is created in. And thanks for pointing out the race condition.

phantom - I'm curious now as to what was said.

Share this post


Link to post
Share on other sites
So... You create a bunch of threads (jobs) that never die until the entire pool does? And a pool dies when it runs out of work, and presumably you create new pools over time as needed? If that's all you're doing, what's the advantage of this over individual thread wrappers? Otherwise, what are we missing in your explanation?

Maybe I'm not well-read on the subject but my understanding is that most threadpools just manage a list of jobs, and that the jobs end. When one job is finished, another gets to take the slot -- also, jobs are typically managed in a priority-queue (usually determined by priority assignment, deadline, or both). Generally, a thread-pool is a persistent thing -- Otherwise, what if you create 2+ threadpools that overlap in time? Now you've got 2 times or more the "optimal" number of threads operating. The whole point of a thread-pool, as I understand it, is to have global knowledge about how many threads are allocated to job-processing, synchronize between them (dependency graph) and to prioritize the work.

Share this post


Link to post
Share on other sites
No. The ThreadPool dies when it goes out of scope, which could have the lifetime of the application. The threads stay alive until the ThreadPool is destructed. Tasks get queued. If there are tasks they get consumed by the threads. A thread gets a new task if its finished its current task and other tasks exist, else the threads just sleep.

Share this post


Link to post
Share on other sites
One issue I have with your design is that while you 'sleep(0)' you are still effectively busy waiting as all that will happen is the OS will give up the quantum and then, if you are the only active app, give it right on back again.

A better solution might be to use events of some sort to signal to a thread 'wake up and work'; you could even use three events;

- 'wake up and work'
- 'time to die'
- 'exiting'

So your thread pool class posts some work to a thread and then raises the 'wake up and work' event which the thread is waiting on, the thread then goes off and does the work as before.

On shut down your thread pool posts 'time to die' and the thread wakes up, optionally clears any work in the queue (depending on design), then exits it's loop and signals 'exiting'. In the mean time, having signalled all the threads to shut down your main thread is waiting on all their 'exiting' events and when they are all signalled it knows they have shut down and it can continue to shut down as well.

And by 'events' I mean OS primatives or some abstraction there of, not checking for bools etc :)

Share this post


Link to post
Share on other sites
What would you gain from using events though? If theres no work to be done, then you'd still just be iterating over some message loop, waiting for the WAKE_UP message.

[color=#1C2837][size=2]And by 'events' I mean OS primatives or some abstraction there of, not checking for bools etc [/quote]

Can you give an example? And whats wrong with checking bools?

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.

GameDev.net is your game development community. Create an account for your GameDev Portfolio and participate in the largest developer community in the games industry.

Sign me up!