This is the mail archive of the cygwin-developers@cygwin.com mailing list for the Cygwin project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]

Quick testfeedback...


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
? 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@cygwin.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;

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]