[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kdevelop-devel
Subject:    Patch: wake up duchainlock writers
From:       Hamish Rodda <rodda () kde ! org>
Date:       2009-12-14 16:58:17
Message-ID: 200912150358.17168.rodda () kde ! org
[Download RAW message or body]

Hi,

I've made a patch to DUChainLock to enable it to wake up writers which have 
had to contend for a lock, rather than waiting for their usleep(10000) to end.  
This has the effect of making multiple parser threads more efficient than just 
one thread for the duchainify tool. (4 threads - 34.3s, 1 thread - 38.0s)  
This is pretty cool, because before, I found that using 4 threads actually 
added 10s.  mutrace shows that with this patch, lock contention becomes a much 
bigger issue, which is to be expected.

I'm posting here as I have not had sufficient time to test it with kdevelop, 
and also it seems slower (although not measured)... which is pretty weird.  
Any ideas? Post your experience with it...

Cheers,
Hamish.

["kdev-duchainlock-wakeupwriters.patch" (text/x-patch)]

Index: duchain/duchainlock.cpp
===================================================================
--- duchain/duchainlock.cpp	(revision 1062371)
+++ duchain/duchainlock.cpp	(working copy)
@@ -84,6 +84,8 @@
     m_totalReaderRecursion = 0;
     m_readers.set_empty_key(0); //Assuming that no thread can ever have handle zero
     m_readersEnd = m_readers.end(); //This is used to speedup \
currentThreadHasReadLock() +    m_waitingReaders = 0;
+    m_waitingWriters = 0;
   }
 
   /**
@@ -128,7 +130,10 @@
   int m_writerRecursion; ///How often is the chain write-locked by the writer?
   int m_totalReaderRecursion; ///How often is the chain read-locked recursively by \
all readers? Should be sum of all m_reader values  
-  QWaitCondition m_waitForWriter;
+  int m_waitingReaders;
+  QWaitCondition m_readerWait;
+  int m_waitingWriters;
+  QWaitCondition m_writerWait;
 
   ///Map readers to the count of recursive read-locks they hold(0 if they do not \
hold a lock)  typedef google::dense_hash_map<Qt::HANDLE, int> ReaderMap;
@@ -193,11 +198,14 @@
     timeval waited;
     timersub(&currentTime, &startTime, &waited);
     
-    if(toMilliSeconds(waited) < timeout)
-      d->m_waitForWriter.wait(&d->m_mutex, timeout);
-    else
+    if(toMilliSeconds(waited) < timeout) {
+      ++d->m_waitingReaders;
+      d->m_readerWait.wait(&d->m_mutex, timeout);
+      --d->m_waitingReaders;
+    } else {
       break;
   }
+  }
   
   if (d->m_writer == 0 || d->m_writer == QThread::currentThreadId()) {
     DUChainLockPrivate::ReaderMap::iterator it = d->m_readers.find( \
QThread::currentThreadId() ); @@ -258,6 +266,10 @@
 #ifdef DEBUG_LOCK_TIMING
   d->auditTime();
 #endif
+
+  if ((d->m_totalReaderRecursion == 0) && !d->m_writer)
+    if (d->m_waitingWriters)
+      d->m_writerWait.wakeOne();
 }
 
 bool DUChainLock::currentThreadHasReadLock()
@@ -294,18 +306,24 @@
   Q_ASSERT(d->ownReaderRecursion() == 0);
 
   bool locked = false;
-  uint currentTime = 0;
 
-  //We cannot use m_waitForWriterForWriter here, because we also have to wait for \
                other readers to finish
-  while ( ( (d->m_writer && d->m_writer != QThread::currentThreadId()) || \
                d->haveOtherReaders()) && currentTime < timeout) {
-    lock.unlock();
-    usleep(10000);
-    currentTime++;
-    lock.relock();
-#ifdef DEBUG_LOG_BACKTRACE
-    d->auditTime(); //Search for deadlocks
-#endif
+  timeval startTime;
+  gettimeofday(&startTime, 0);
+
+  while((d->m_writer && d->m_writer != QThread::currentThreadId()) || \
d->haveOtherReaders()) { +    timeval currentTime;
+    gettimeofday(&currentTime, 0);
+    timeval waited;
+    timersub(&currentTime, &startTime, &waited);
+
+    if(toMilliSeconds(waited) < timeout) {
+      ++d->m_waitingWriters;
+      d->m_writerWait.wait(&d->m_mutex, timeout);
+      --d->m_waitingWriters;
+    } else {
+      break;
   }
+  }
 
   if ( (d->m_writer == 0 || d->m_writer == QThread::currentThreadId()) && \
!d->haveOtherReaders()) {  d->m_writer = QThread::currentThreadId();
@@ -330,10 +348,15 @@
   
   --d->m_writerRecursion;
   
-  if( !d->m_writerRecursion )
+  if( !d->m_writerRecursion ) {
     d->m_writer = 0;
 
-  d->m_waitForWriter.wakeAll();
+    if (d->m_waitingReaders)
+      d->m_readerWait.wakeAll();
+    else if (d->m_waitingWriters)
+      d->m_writerWait.wakeOne();
+  }
+
 #ifdef DEBUG_LOCK_TIMING
   d->auditTime();
 #endif



-- 
KDevelop-devel mailing list
KDevelop-devel@kdevelop.org
https://barney.cs.uni-potsdam.de/mailman/listinfo/kdevelop-devel


[prev in list] [next in list] [prev in thread] [next in thread]