views:

72

answers:

5

Why does the code sample below cause one thread to execute way more than another but a mutex does not?

#include <windows.h>
#include <conio.h>
#include <process.h>
#include <iostream>
using namespace std;

typedef struct _THREAD_INFO_ {

    COORD coord;        // a structure containing x and y coordinates
    INT threadNumber;   // each thread has it's own number
    INT count; 

}THREAD_INFO, * PTHREAD_INFO;

void gotoxy(int x, int y);

BOOL g_bRun; 
CRITICAL_SECTION g_cs; 

unsigned __stdcall ThreadFunc( void* pArguments )
{
    PTHREAD_INFO info = (PTHREAD_INFO)pArguments;

    while(g_bRun)
    {

        EnterCriticalSection(&g_cs); 

        //if(TryEnterCriticalSection(&g_cs))
        //{
            gotoxy(info->coord.X, info->coord.Y);
            cout << "T" << info->threadNumber << ": " << info->count;

            info->count++; 

            LeaveCriticalSection(&g_cs); 

        //}
    }

    ExitThread(0);
    return 0;
}

int main(void)
{
    // OR unsigned int
    unsigned int id0, id1; // a place to store the thread ID returned from CreateThread
    HANDLE h0, h1;  // handles to theads

    THREAD_INFO tInfo[2]; // only one of these - not optimal!

    g_bRun = TRUE;

    ZeroMemory(&tInfo, sizeof(tInfo)); // win32 function - memset(&buffer, 0, sizeof(buffer))

    InitializeCriticalSection(&g_cs); 

    // setup data for the first thread
    tInfo[0].threadNumber = 1;
    tInfo[0].coord.X = 0;
    tInfo[0].coord.Y = 0;

    h0 = (HANDLE)_beginthreadex( 
            NULL,        // no security attributes
            0,           // defaut stack size
            &ThreadFunc, // pointer to function
            &tInfo[0],   // each thread gets its own data to output
            0,           // 0 for running or CREATE_SUSPENDED 
            &id0 ); // return thread id - reused here

    // setup data for the second thread
    tInfo[1].threadNumber = 2;
    tInfo[1].coord.X = 15;
    tInfo[1].coord.Y = 0;

    h1 = (HANDLE)_beginthreadex( 
            NULL,        // no security attributes
            0,           // defaut stack size
            &ThreadFunc, // pointer to function
            &tInfo[1],   // each thread gets its own data to output
            0,           // 0 for running or CREATE_SUSPENDED 
            &id1 ); // return thread id - reused here

    _getch(); 

    g_bRun = FALSE; 

    return 0;
}

void gotoxy(int x, int y)   // x=column position and y=row position
{
   HANDLE hdl;
   COORD coords;
   hdl = GetStdHandle(STD_OUTPUT_HANDLE);
   coords.X = x;
   coords.Y = y;      
   SetConsoleCursorPosition(hdl, coords);
}
A: 

In hand wavey terms:

CriticalSection is saying the thread wants control to do some things together.

Mutex is making a marker to show 'being busy' so others can wait and notifying of completion so somebody else can start. Somebody else already waiting for the mutex will grab it before you can start the loop again and get it back.

So what you are getting with CriticalSection is a failure to yield between loops. You might see a difference if you had Sleep(0); after LeaveCriticalSection

Greg Domjan
A: 

I can't say why you're observing this particular behavior, but it's probably to do with the specifics of the implementation of each mechanism. What I CAN say is that unlocking then immediately locking a mutex is a bad thing. You will observe odd behavior eventually.

Brett
+1  A: 

That may not answer your question but the behavior of critical sections changed on Windows Server 2003 SP1 and later.

If you have bugs related to critical sections on Windows 7 that you can't reproduce on an XP machine you may be affected by that change.

My understanding is that on Windows XP critical sections used a FIFO based strategy that was fair for all threads while later versions use a new strategy aimed at reducing context switching between threads.

There's a short note about this on the MSDN page about critical sections

You may also want to check this forum post

Alexandre Jasmin
Thanks. The forum post seems to answer the question. The conclusion seems to be that a critical section is not a good way to share a resource like a log file, etc.
That's not a correct conclusion. A critical section should be perfectly fine. As long as the disk bandwidth is higher than the rate of log file writes, all threads will eventually be serviced. When the total rate of writes exceeds the disk bandwidth, you have bigger problems than unfair scheduling.
MSalters
+1  A: 

From some MSDN docs (http://msdn.microsoft.com/en-us/library/ms682530.aspx):

Starting with Windows Server 2003 with Service Pack 1 (SP1), threads waiting on a critical section do not acquire the critical section on a first-come, first-serve basis. This change increases performance significantly for most code

Michael Burr
The code works well on XP but not 7 on the same hardware. In the code shown, one thread usually dominates and the other thread runs every few seconds. It is also worth noting that SRW behaves the same way.
A: 

Critical sections, like mutexes are designed to protect a shared resource against conflicting access (such as concurrent modification). Critical sections are not meant to replace thread priority.

You have artificially introduced a shared resource (the screen) and made it into a bottleneck. As a result, the critical section is highly contended. Since both threads have equal priority, that is no reason for Windows to prefer one thread over another. Reduction of context switches is a reason to pick one thread over another. As a result of that reduction, the utilization of the shared resource goes up. That is a good thing; it means that one thread will be finished a lot earlier and the other thread will finish a bit earlier.

To see the effect graphically, compare

A B A B A B A B A B

to

AAAAA BBBBB

The second sequence is shorter because there's only one switch from A to B.

MSalters