how do i properly design a worker thread? (avoid for example Sleep(1))

i am still a beginner at multi-threading, so bear with me please:

i am currently writing an application that does some FVM calculation on a grid. it's a time-e开发者_运维技巧xplicit model, so at every timestep i need to calculate new values for the whole grid. my idea was to distribute this calculation to 4 worker-threads, which then deal with the cells of the grid (first thread calculating 0, 4, 8... second thread 1, 5, 9... and so forth).

i create those 4 threads at program start.

they look something like this:

void __fastcall TCalculationThread::Execute()
{
    bool alive = true;
    THREAD_SIGNAL ts;
    while (alive)
    {
        Sleep(1);
        if (TryEnterCriticalSection(&TMS))
        {
        ts = thread_signal;
        LeaveCriticalSection(&TMS);
        alive = !ts.kill;
        if (ts.go && !ts.done.at(this->index))
        {
            double delta_t = ts.dt;
            for (unsigned int i=this->index; i < cells.size(); i+= this->steps)
            {
                    calculate_one_cell();
            }
            EnterCriticalSection(&TMS);
                thread_signal.done.at(this->index)=true;
            LeaveCriticalSection(&TMS);
        }
    }
}

they use a global struct, to communicate with the main thread (main thread sets ts.go to true when the workers need to start.

now i am sure this is not the way to do it! not only does it feel wrong, it also doesn't perform very well...

i read for example here that a semaphore or an event would work better. the answer to this guy's question talks about a lockless queue.

i am not very familiar with these concepts would like some pointers how to continue. could you line out any of the ways to do this better?

thank you for your time. (and sorry for the formatting)

i am using borland c++ builder and its thread-object (TThread).

The definitely more effective algorithm would be to calculate yields for 0,1,2,3 on one thread, 4,5,6,7 on another, etc. Interleaving memory accesses like that is very bad, even if the variables are completely independent- you'll get false sharing problems. This is the equivalent of the CPU locking every write.

Calling Sleep(1) in a calculation thread can't be a good solution to any problem. You want your threads to be doing useful work rather than blocking for no good reason.

I think your basic problem can be expressed as a serial algorithm of this basic form:

for (int i=0; i<N; i++)
    cells[i]->Calculate();

You are in the happy position that calls to Calculate() are independent of each other—what you have here is a parallel for. This means that you can implement this without a mutex.

There are a variety of ways to achieve this. OpenMP would be one; a threadpool class another. If you are going to roll your own thread based solution then use InterlockedIncrement() on a shared variable to iterate through the array.

You may hit some false sharing problems, as @DeadMG suggests, but quite possibly not. If you do have false sharing then yet another approach is to stride across larger sub-arrays. Essentially the increment (i.e. stride) passed to InterlockedIncrement() would be greater than one.

The bottom line is that the way to make the code faster is to remove both the the critical section (and hence the contention on it) and the Sleep(1).