From: Wu Y. <ad...@us...> - 2003-11-18 10:52:03
Attachments:
gthr-win32.h.diff
gthr_test.cpp
|
Danny Smith wrote: > > It is known > > that Critical Sections are more efficient. Some benchmarks can be > > found on the Internet. Could we use Critical Sections, as in GCC 2? > > > I thought that gcc-2.95 only used Critical Sections for allocator > node locks and used mutexes everywhere else. Anyway, if you have a > patch against GCC HEAD, I would be keen to look at it. Please have a look whether this is in the right direction. I have modified only the case when __GTHREAD_HIDE_WIN32API is 0. If it is OK, gcc/config/i386/gthr-win32.c, and probably the ObjC part of gcc/gthr-win32.h, need to be patched too. The test case really shows that successful mutex/critical section lock/unlock operations has a performance ratio of about 600/9 (instructions), as Microsoft indicates. The ugliest part is the definition of __gthread_mutex_t. But I have not found a good way without including <windows.h>. Best regards, Wu Yongwei |
From: Wu Y. <ad...@us...> - 2003-11-19 13:41:09
|
>> static inline int >> @@ -560,7 +552,7 @@ >> >> if (__gthread_active_p ()) >> { >> - if (WaitForSingleObject (*mutex, 0) == WAIT_OBJECT_0) >> + if (TryEnterCriticalSection ((PCRITICAL_SECTION) mutex) != 0) > >WIN9x does not have TryEnterCriticalSection, therefore this will not run >on 9x. > >I have added a mutex implementation that works similar to a Critical >Section to both pthreads_win32 and cygwin. It is based on a counter >modified by Interlocked[Increment|Decrement|CompareExchange] and a >semaphore. When the Interlocked funktions are replaced by inline >assembler routines it runs on every Windows version on a 486 or above. > >If there is interest to add this to MINGW let me know. I do not think >that 9x Support should be dropped. > >Regards, > >Thomas Thomas's suggestion is one good idea: I did not notice that TryEnterCriticalSection was not available on Windows 9x. However, grepping the gcc source and not finding a reference to __gthread_mutex_trylock, I doubt the necessity of it: if no one is expected to use it, could we ignore it for the moment? Maybe something like: #ifdef __GTHREAD_WIN32_USE_CRITICAL_SECTION typedef ... #else typedef void* __gthread_mutex_t; #endif #ifdef __GTHREAD_WIN32_USE_CRITICAL_SECTION #else int __gthread_mutex_trylock (__gthread_mutex_t* mutex) { // Original Win32 mutex code } #endif Yet a third alternative is to write a TryEnterCriticalSection replacement. It should be possible, but I have not investigated to find out how difficult it is. Best regards, Wu Yongwei |
From: <ad...@sh...> - 2003-11-20 11:07:08
Attachments:
gthr-win32.h.diff
|
Danny and Thomas, Please have a look. Best regards, Wu Yongwei |
From: Thomas P. <tp...@gm...> - 2003-11-20 12:22:18
|
Your version of TryEnterCriticalSection : /* Our own version of TryEnterCriticalSection since it is not available * on Windows 9x. */ static inline BOOL __TryEnterCriticalSection (LPCRITICAL_SECTION lpCriticalSection) { HANDLE hThreadId = (HANDLE) GetCurrentThreadId(); if (InterlockedIncrement(&lpCriticalSection->LockCount) == 0) { lpCriticalSection->OwningThread = hThreadId; lpCriticalSection->RecursionCount = 1; } else { if (lpCriticalSection->OwningThread == hThreadId) lpCriticalSection->RecursionCount++; { InterlockedDecrement(&lpCriticalSection->LockCount); return FALSE; } return TRUE; } has a race condition. If the mutex is locked and a task switch occurs between InterlockedIncrement and InterlockedDecrement and the mutex is unlocked during that time then the counter contains an invalid (still incremented) value and the unlock function assumes that another thread is waiting on that mutex. And i do not think that critical section internals should be used here. The mutex logic which is used by pthreads-win32 and cygwin is similar to a critical section: struct mutex { LONG counter; HANDLE sema; }; mx_init (struct mutex *mx) { mx->counter = 0; mx->sema = CreateSemaphore (NULL, 0, LONG_MAX, NULL); } int mx_lock (struct mutex *mx) { if (InterlockedIncrement (&mx->counter) != 1) WaitForSingleObject (mx->sema, INFINITE); return 0; } int mx_trylock (struct mutex *mx) { if (InterlockedCompareExchange (&mx->counter, 1, 0 ) != 0) return 1; return 0; } int mx_unlock (struct mutex *mx) { if (InterlockedDecrement (&mx->counter)) ReleaseSemaphore (mx->sema, 1, NULL); return 0; } A return value of 0 signals success. This code will run on Win98 and NT4 or above. Win95 and NT3 are designed to run on a i386 and does not contain an InterlockedCompareExchange and the Interlocked[Increment|Decrement] are not thread safe. Cygwin avoids this by using inline assembler replacements for Interlocked[Increment|Decrement|CompareExchange] that are based on xadd and cmpxchg which requires at least on i486. The above implements a fast (deadlocking and not errorchecking) mutex. I think that this is enough for gcc since this is also the default on Linux. Regards, Thomas |
From: Wu Y. <ad...@sh...> - 2004-03-14 16:09:18
|
At last I cannot bear with it any more. Here is the patch based on Thomas's mutex implementation. And the test files and a test log. I think the speed test really shows (3605:70 or 3605:50) that the change is worth while. Best regards, Wu Yongwei On Thu, 2003-11-20 at 20:21, Thomas Pfaff wrote: > Your version of TryEnterCriticalSection : > > /* Our own version of TryEnterCriticalSection since it is not available > * on Windows 9x. */ > static inline BOOL > __TryEnterCriticalSection (LPCRITICAL_SECTION lpCriticalSection) > { > HANDLE hThreadId = (HANDLE) GetCurrentThreadId(); > if (InterlockedIncrement(&lpCriticalSection->LockCount) == 0) > { > lpCriticalSection->OwningThread = hThreadId; > lpCriticalSection->RecursionCount = 1; > } > else > { > if (lpCriticalSection->OwningThread == hThreadId) > lpCriticalSection->RecursionCount++; > { > InterlockedDecrement(&lpCriticalSection->LockCount); > return FALSE; > } > return TRUE; > } > > has a race condition. > > If the mutex is locked and a task switch occurs between > InterlockedIncrement and InterlockedDecrement and the mutex is unlocked > during that time then the counter contains an invalid (still > incremented) value and the unlock function assumes that another thread > is waiting on that mutex. > And i do not think that critical section internals should be used here. > > The mutex logic which is used by pthreads-win32 and cygwin is similar to > a critical section: > > struct mutex > { > LONG counter; > HANDLE sema; > }; > > mx_init (struct mutex *mx) > { > mx->counter = 0; > mx->sema = CreateSemaphore (NULL, 0, LONG_MAX, NULL); > } > > int mx_lock (struct mutex *mx) > { > if (InterlockedIncrement (&mx->counter) != 1) > WaitForSingleObject (mx->sema, INFINITE); > > return 0; > } > > int mx_trylock (struct mutex *mx) > { > if (InterlockedCompareExchange (&mx->counter, 1, 0 ) != 0) > return 1; > > return 0; > } > > int mx_unlock (struct mutex *mx) > { > if (InterlockedDecrement (&mx->counter)) > ReleaseSemaphore (mx->sema, 1, NULL); > > return 0; > } > > A return value of 0 signals success. > > This code will run on Win98 and NT4 or above. > > Win95 and NT3 are designed to run on a i386 and does not contain an > InterlockedCompareExchange and the Interlocked[Increment|Decrement] are > not thread safe. > > Cygwin avoids this by using inline assembler replacements for > Interlocked[Increment|Decrement|CompareExchange] that are based on xadd > and cmpxchg which requires at least on i486. > > The above implements a fast (deadlocking and not errorchecking) mutex. I > think that this is enough for gcc since this is also the default on Linux. > > Regards, > > Thomas |
From: Danny S. <dan...@cl...> - 2004-03-14 19:31:58
|
----- Original Message ----- From: "Wu Yongwei" > At last I cannot bear with it any more. Here is the patch based on > Thomas's mutex implementation. And the test files and a test log. I > think the speed test really shows (3605:70 or 3605:50) that the change > is worth while. > > Best regards, > > Wu Yongwei > Thanks, but.... 1) This needs to be submitted to GCC-patches, 2) You and/or Thomas will need FSF copyright assignment. 3) If part of this patch has been "borrowed" from cygwin/win32-pthreads there are other copyright issues. I haven't looked at the patch yet, and won't until at least 1) above is done. . Danny |
From: Christopher F. <cg...@re...> - 2004-03-14 22:47:51
|
On Sun, Mar 14, 2004 at 07:26:02PM +0000, Danny Smith wrote: >From: "Wu Yongwei" >>At last I cannot bear with it any more. Here is the patch based on >>Thomas's mutex implementation. And the test files and a test log. I >>think the speed test really shows (3605:70 or 3605:50) that the change >>is worth while. > >Thanks, but.... > >1) This needs to be submitted to GCC-patches, 2) You and/or Thomas will >need FSF copyright assignment. 3) If part of this patch has been >"borrowed" from cygwin/win32-pthreads there are other copyright issues. FWIW, I don't see anything like this code in cygwin. cgf |
From: Wu Y. <ad...@sh...> - 2004-03-18 13:05:53
|
Hi, Danny, 1) has been done. Would you please have a look at it? I am still not clear about 2). Please help. The Web pages talk about a directory `/gd/gnuorg', but I do not know where it is. I suppose you must know it. Would you please send me the template, or tell me where I can find it, if convenient? Forgive me if I am like an idiot. Best regards, Wu Yongwei On Mon, 2004-03-15 at 03:26, Danny Smith wrote: > > 1) This needs to be submitted to GCC-patches, > 2) You and/or Thomas will need FSF copyright assignment. > 3) If part of this patch has been "borrowed" from cygwin/win32-pthreads > there are other copyright issues. > > I haven't looked at the patch yet, and won't until at least 1) above is > done. > . > Danny |
From: Danny S. <dan...@cl...> - 2004-03-18 19:15:53
Attachments:
assign-request.txt
|
----- Original Message ----- From: "Wu Yongwei > Hi, Danny, > > 1) has been done. Would you please have a look at it? > Yes, I have. I see one small problem, but otherwise it looks good. > I am still not clear about 2). Please help. The Web pages talk about a > directory `/gd/gnuorg', but I do not know where it is. I suppose you > must know it. Would you please send me the template, or tell me where I > can find it, if convenient? > I see that Chris has made a request on gcc-patches. The way I proceeded to get my assignment form was to complete and email the attached the FSF assignemnet clerk. I found the form at http://gcc.gnu.org/ml/gcc/2002-09/msg00676.html Cheers Danny |
From: Christopher F. <cg...@re...> - 2004-03-19 00:44:46
|
On Thu, Mar 18, 2004 at 07:09:16PM +0000, Danny Smith wrote: >----- Original Message ----- >From: "Wu Yongwei >>1) has been done. Would you please have a look at it? > >Yes, I have. I see one small problem, but otherwise it looks good. > >>I am still not clear about 2). Please help. The Web pages talk about >>a directory `/gd/gnuorg', but I do not know where it is. I suppose you >>must know it. Would you please send me the template, or tell me where >>I can find it, if convenient? > >I see that Chris has made a request on gcc-patches. The way I >proceeded to get my assignment form was to complete and email the >attached the FSF assignemnet clerk. I found the form at >http://gcc.gnu.org/ml/gcc/2002-09/msg00676.html I thought that someone (not Diego) from gcc was supposed to send a form to anyone who asked. We shouldn't have to be working with old forms. cgf |
From: Wu Y. <ad...@sh...> - 2004-03-21 03:38:06
Attachments:
gthr-win32.diff
|
On Fri, 2004-03-19 at 03:09, Danny Smith wrote: > > > 1) has been done. Would you please have a look at it? > > > > Yes, I have. I see one small problem, but otherwise it looks good. You saw one small problem? Um, I have seen two now: one is the definition of __GTHREAD_MUTEX_INIT_DEFAULT, the other a no-return-value branch in __gthread_mutex_unlock. Hope what you found is one of them :-). The updated patch is attached. Need I post it to gcc-patches again? Or wait till the FSF paper that I should sign reaches me? Best regards, Wu Yongwei |
From: Danny S. <dan...@cl...> - 2004-03-21 05:12:26
|
----- Original Message ----- From: "Wu Yongwei" > On Fri, 2004-03-19 at 03:09, Danny Smith wrote: > > Yes, I have. I see one small problem, but otherwise it looks good. > > You saw one small problem? Um, I have seen two now: one is the > definition of __GTHREAD_MUTEX_INIT_DEFAULT, the other a no-return-value > branch in __gthread_mutex_unlock. Hope what you found is one of them > :-). Yes, I saw those two as warnings when I tried to bootstrap. There is one more I saw before: +#ifndef _WINDOWS_H +typedef long LONG; +typedef void *HANDLE; +#endif + Above will cause problems is windows.h is included after this file. Just use long and void* in definition of __gthread_mutex_t > > The updated patch is attached. Need I post it to gcc-patches again? Or > wait till the FSF paper that I should sign reaches me? > I think wait. Danny > Best regards, > > Wu Yongwei > |
From: Wu Y. <ad...@sh...> - 2004-03-21 08:48:02
|
On Sun, 2004-03-21 at 13:06, Danny Smith wrote: > > Yes, I saw those two as warnings when I tried to bootstrap. There is > one more I saw before: > > +#ifndef _WINDOWS_H > +typedef long LONG; > +typedef void *HANDLE; > +#endif > + > > Above will cause problems is windows.h is included after this file. > Just use long and void* in definition of __gthread_mutex_t I just wanted to ask you why, but now I see that this can cause problems in C, but no problem at all in C++. Maybe I should take less care of C++ for such kind of code :-). I'll revise accordingly. Best regards, Wu Yongwei |
From: Wu Y. <ad...@us...> - 2003-11-20 15:40:54
|
While my patch hours ago (in a hurry) has some problems, it has an even greater wrong assumption. To my surprise, InitializeCriticalSection in Windows 98 initialized the CRITICAL_SECTION differently than in Windows 2000! It really made my "third alternative" infeasible at all. If we want to use a light-weight mutex AND provide trylock, it seems that we have to write our own quasi-CRITICAL_SECTION. I suggest that Thomas provide one. Opinions? Best regards, Wu Yongwei |
From: Wu Y. <ad...@us...> - 2003-11-20 15:49:45
|
>[code snipped] > >has a race condition. > >If the mutex is locked and a task switch occurs between >InterlockedIncrement and InterlockedDecrement and the mutex is unlocked >during that time then the counter contains an invalid (still >incremented) value and the unlock function assumes that another thread >is waiting on that mutex. >And i do not think that critical section internals should be used here. Agreed. And I have realized other problems. It seems that I am not seasoned enough in the multi-threaded world. :-( >[snipped] > >The above implements a fast (deadlocking and not errorchecking) mutex. I >think that this is enough for gcc since this is also the default on Linux. Yes, I think so too. Best regards, Wu Yongwei |
From: Thomas P. <tp...@gm...> - 2003-11-18 11:14:02
|
Wu Yongwei wrote: > Danny Smith wrote: > >>>It is known >>>that Critical Sections are more efficient. Some benchmarks can be >>>found on the Internet. Could we use Critical Sections, as in GCC 2? >> >>I thought that gcc-2.95 only used Critical Sections for allocator >>node locks and used mutexes everywhere else. Anyway, if you have a >>patch against GCC HEAD, I would be keen to look at it. > > > Please have a look whether this is in the right direction. I have > modified only the case when __GTHREAD_HIDE_WIN32API is 0. If it is OK, > gcc/config/i386/gthr-win32.c, and probably the ObjC part of > gcc/gthr-win32.h, need to be patched too. The test case really shows > that successful mutex/critical section lock/unlock operations has a > performance ratio of about 600/9 (instructions), as Microsoft indicates. > > The ugliest part is the definition of __gthread_mutex_t. But I have not > found a good way without including <windows.h>. > > Best regards, > > Wu Yongwei > [snip] > static inline int > @@ -560,7 +552,7 @@ > > if (__gthread_active_p ()) > { > - if (WaitForSingleObject (*mutex, 0) == WAIT_OBJECT_0) > + if (TryEnterCriticalSection ((PCRITICAL_SECTION) mutex) != 0) WIN9x does not have TryEnterCriticalSection, therefore this will not run on 9x. I have added a mutex implementation that works similar to a Critical Section to both pthreads_win32 and cygwin. It is based on a counter modified by Interlocked[Increment|Decrement|CompareExchange] and a semaphore. When the Interlocked funktions are replaced by inline assembler routines it runs on every Windows version on a 486 or above. If there is interest to add this to MINGW let me know. I do not think that 9x Support should be dropped. Regards, Thomas |