Mailing-List: contact cygwin-developers-help AT sourceware DOT cygnus DOT com; run by ezmlm List-Subscribe: List-Archive: List-Post: List-Help: , Sender: cygwin-developers-owner AT sources DOT redhat DOT com Delivered-To: mailing list cygwin-developers AT sources DOT redhat DOT com Subject: Quick testfeedback... From: Robert Collins To: cygwin-developers AT cygwin DOT com Content-Type: multipart/mixed; boundary="=-ejJV5VH5bwQC9pkeHgul" X-Mailer: Evolution/0.13 (Preview Release) Date: 11 Sep 2001 22:00:11 +1000 Message-Id: <1000209619.7293.196.camel@lifelesswks> Mime-Version: 1.0 X-OriginalArrivalTime: 11 Sep 2001 11:47:10.0069 (UTC) FILETIME=[7D6B3A50:01C13AB7] --=-ejJV5VH5bwQC9pkeHgul Content-Type: text/plain Content-Transfer-Encoding: 7bit The attached patch changes pthread_mutexs on windows NT to use critical sections, instead of win32 mutex's. This should make a noticeable speed difference - (possibly even faster than the win32_pthreads library - we were neck and neck before this change :]). There is a minor issue with pthread_cond_timedwait that I need to resolve on NT, but it's no worse than the current win95 implementation in terms of potentially losing a signal so I'm not concerned about this today. I have tested this out on win95 for regressions, but not on NT unfortunately... If some kind NT/2k user could test this I would be very appreciative. A current test suite I use for this is available at: http://lifeless.home.dhs.org/PthreadTest-0.1.tar.gz - just configure and make check. It's fully automated except for the last test, which checks scheduler changes. Thanks, Rob --=-ejJV5VH5bwQC9pkeHgul Content-Type: text/plain Content-Disposition: attachment; filename=mutexsviacriticalsections.patch Content-ID: <1000209465 DOT 7231 DOT 192 DOT camel AT lifelesswks> Content-Transfer-Encoding: 7bit ? fhandler_ums.cc ? mutexsviacriticalsections.patch ? pthchange ? umsdos_gen.h ? virtualquery.patch Index: autoload.cc =================================================================== RCS file: /cvs/src/src/winsup/cygwin/autoload.cc,v retrieving revision 1.30 diff -u -p -r1.30 autoload.cc --- autoload.cc 2001/09/07 21:32:04 1.30 +++ autoload.cc 2001/09/11 11:52:10 @@ -478,6 +478,7 @@ LoadDLLfuncEx (CancelIo, 4, kernel32, 1) LoadDLLfuncEx (Process32First, 8, kernel32, 1) LoadDLLfuncEx (Process32Next, 8, kernel32, 1) LoadDLLfuncEx (CreateToolhelp32Snapshot, 8, kernel32, 1) +LoadDLLfunc (TryEnterCriticalSection, 4, kernel32) LoadDLLfuncEx (waveOutGetNumDevs, 0, winmm, 1) LoadDLLfuncEx (waveOutOpen, 24, winmm, 1) Index: thread.cc =================================================================== RCS file: /cvs/src/src/winsup/cygwin/thread.cc,v retrieving revision 1.46 diff -u -p -r1.46 thread.cc --- thread.cc 2001/09/11 11:23:41 1.46 +++ thread.cc 2001/09/11 11:52:14 @@ -506,8 +506,19 @@ pthread_cond::TimedWait (DWORD dwMillise rv = WaitForSingleObject (win32_obj_id, dwMilliseconds); } else + { + LeaveCriticalSection (&mutex->criticalsection); + rv = WaitForSingleObject (win32_obj_id, dwMilliseconds); +#if 0 + /* we need to use native win32 mutex's here, because the cygwin ones now use + * critical sections, which are faster, but introduce a race _here_. Until then + * The NT variant of the code is redundant. + */ + rv = SignalObjectAndWait (mutex->win32_obj_id, win32_obj_id, dwMilliseconds, false); +#endif + } switch (rv) { case WAIT_FAILED: @@ -604,11 +615,14 @@ pthread_mutex::pthread_mutex (pthread_mu magic = 0; return; } - - this->win32_obj_id =::CreateMutex (&sec_none_nih, false, NULL); - - if (!win32_obj_id) - magic = 0; + if (iswinnt) + InitializeCriticalSection (&criticalsection); + else + { + this->win32_obj_id =::CreateMutex (&sec_none_nih, false, NULL); + if (!win32_obj_id) + magic = 0; + } condwaits = 0; pshared = PTHREAD_PROCESS_PRIVATE; /* threadsafe addition is easy */ @@ -617,18 +631,25 @@ pthread_mutex::pthread_mutex (pthread_mu pthread_mutex::~pthread_mutex () { - if (win32_obj_id) - CloseHandle (win32_obj_id); - win32_obj_id = NULL; + if (iswinnt) + DeleteCriticalSection (&criticalsection); + else + { + if (win32_obj_id) + CloseHandle (win32_obj_id); + win32_obj_id = NULL; + } /* I'm not 100% sure the next bit is threadsafe. I think it is... */ if (MT_INTERFACE->mutexs == this) - InterlockedExchangePointer (&MT_INTERFACE->mutexs, this->next); + /* TODO: printf an error if the return value != this */ + InterlockedExchangePointer (&MT_INTERFACE->mutexs, next); else { pthread_mutex *tempmutex = MT_INTERFACE->mutexs; while (tempmutex->next && tempmutex->next != this) tempmutex = tempmutex->next; /* but there may be a race between the loop above and this statement */ + /* TODO: printf an error if the return value != this */ InterlockedExchangePointer (&tempmutex->next, this->next); } } @@ -636,19 +657,33 @@ pthread_mutex::~pthread_mutex () int pthread_mutex::Lock () { + if (iswinnt) + { + EnterCriticalSection (&criticalsection); + return 0; + } + /* FIXME: Return 0 on success */ return WaitForSingleObject (win32_obj_id, INFINITE); } +/* returns non-zero on failure */ int pthread_mutex::TryLock () { - return WaitForSingleObject (win32_obj_id, 0); + if (iswinnt) + return (!TryEnterCriticalSection (&criticalsection)); + return (WaitForSingleObject (win32_obj_id, 0) == WAIT_TIMEOUT); } int pthread_mutex::UnLock () { - return ReleaseMutex (win32_obj_id); + if (iswinnt) + { + LeaveCriticalSection (&criticalsection); + return 0; + } + return (!ReleaseMutex (win32_obj_id)); } void @@ -658,10 +693,14 @@ pthread_mutex::fixup_after_fork () if (pshared != PTHREAD_PROCESS_PRIVATE) api_fatal("pthread_mutex::fixup_after_fork () doesn'tunderstand PROCESS_SHARED mutex's\n"); /* FIXME: duplicate code here and in the constructor. */ - this->win32_obj_id =::CreateMutex (&sec_none_nih, false, NULL); - - if (!win32_obj_id) - api_fatal("pthread_mutex::fixup_after_fork() failed to create new win32 mutex\n"); + if (iswinnt) + InitializeCriticalSection(&criticalsection); + else + { + win32_obj_id =::CreateMutex (&sec_none_nih, false, NULL); + if (!win32_obj_id) + api_fatal("pthread_mutex::fixup_after_fork() failed to create new win32 mutex\n"); + } if (condwaits) api_fatal("Forked() while a mutex has condition variables waiting on it.\nReport to cygwin AT cygwin DOT com\n"); } @@ -1908,7 +1947,7 @@ __pthread_mutex_trylock (pthread_mutex_t __pthread_mutex_init (mutex, NULL); if (!verifyable_object_isvalid (*themutex, PTHREAD_MUTEX_MAGIC)) return EINVAL; - if ((*themutex)->TryLock () == WAIT_TIMEOUT) + if ((*themutex)->TryLock ()) return EBUSY; return 0; } @@ -1927,7 +1966,7 @@ __pthread_mutex_unlock (pthread_mutex_t int __pthread_mutex_destroy (pthread_mutex_t *mutex) { - if (*mutex == PTHREAD_MUTEX_INITIALIZER) + if (check_valid_pointer (mutex) && (*mutex == PTHREAD_MUTEX_INITIALIZER)) return 0; if (!verifyable_object_isvalid (*mutex, PTHREAD_MUTEX_MAGIC)) return EINVAL; Index: thread.h =================================================================== RCS file: /cvs/src/src/winsup/cygwin/thread.h,v retrieving revision 1.25 diff -u -p -r1.25 thread.h --- thread.h 2001/09/11 08:15:39 1.25 +++ thread.h 2001/09/11 11:52:15 @@ -267,6 +267,7 @@ public: class pthread_mutex:public verifyable_object { public: + CRITICAL_SECTION criticalsection; HANDLE win32_obj_id; LONG condwaits; int pshared; --=-ejJV5VH5bwQC9pkeHgul--