Avoid locking/unlocking in a tight loop

After previous patch if split point master is
waiting for job and "Use Sleeping Threads" is
false (our condition for official releases) then
it will lock/unlock splitPoint mutex in a super
tight loop badly affecting performance.

Rewrite the code to lock only when we are about
to finish. Note that race condition on slavesMask
is anyhow fixed.

No functional change.

diff --git a/src/search.cpp b/src/search.cpp
index c5c5b94470fed9365831fea28427db4d4070d0e5..9a373dec456223cfbac60cd4ce8729adbf3d7a13 100644 (file)
--- a/src/search.cpp
+++ b/src/search.cpp
@@ -1626,13 +1626,8 @@ void Thread::idle_loop() {
 
   assert(!this_sp || (this_sp->masterThread == this && searching));
 
 
   assert(!this_sp || (this_sp->masterThread == this && searching));
 

-  // If this thread is the master of a split point and all slaves have finished
-  // their work at this split point, return from the idle loop.
-  while (!this_sp || this_sp->slavesMask)
+  while (true)

   {
   {

-      if (this_sp)
-          this_sp->mutex.unlock();
-

       // If we are not searching, wait for a condition to be signaled instead of
       // wasting CPU time polling for work.
       while ((!searching && Threads.sleepWhileIdle) || exit)
       // If we are not searching, wait for a condition to be signaled instead of
       // wasting CPU time polling for work.
       while ((!searching && Threads.sleepWhileIdle) || exit)


@@ -1725,8 +1720,16 @@ void Thread::idle_loop() {
           sp->mutex.unlock();
       }
 
           sp->mutex.unlock();
       }
 

-      if(this_sp)
+      // If this thread is the master of a split point and all slaves have finished
+      // their work at this split point, return from the idle loop.
+      if (this_sp && !this_sp->slavesMask)
+      {

           this_sp->mutex.lock();
           this_sp->mutex.lock();

+          bool finished = !this_sp->slavesMask; // Retest under lock protection
+          this_sp->mutex.unlock();
+          if (finished)
+              return;
+      }

   }
 }
 
   }
 }
 

diff --git a/src/thread.cpp b/src/thread.cpp
index df746a4a825d2befdc77fa5397e0b5be952b3a65..0d8070f202b0774d5077b4ec26962d54f438f7c4 100644 (file)
--- a/src/thread.cpp
+++ b/src/thread.cpp
@@ -312,7 +312,6 @@ void Thread::split(Position& pos, Stack* ss, Value alpha, Value beta, Value* bes
       sp.mutex.unlock();
       Threads.mutex.unlock();
 
       sp.mutex.unlock();
       Threads.mutex.unlock();
 

-      // Calling idle_loop with sp.mutex locked

       Thread::idle_loop(); // Force a call to base class idle_loop()
 
       // In helpful master concept a master can help only a sub-tree of its split
       Thread::idle_loop(); // Force a call to base class idle_loop()
 
       // In helpful master concept a master can help only a sub-tree of its split


@@ -323,10 +322,6 @@ void Thread::split(Position& pos, Stack* ss, Value alpha, Value beta, Value* bes
       // We have returned from the idle loop, which means that all threads are
       // finished. Note that setting 'searching' and decreasing splitPointsSize is
       // done under lock protection to avoid a race with Thread::is_available_to().
       // We have returned from the idle loop, which means that all threads are
       // finished. Note that setting 'searching' and decreasing splitPointsSize is
       // done under lock protection to avoid a race with Thread::is_available_to().

-      // idle_loop returns with sp.mutex locked but we must unlock it inorder to
-      // lock Threads.mutex without conflicting with check_time() (threads holding 
-      // multiple locks must always acquired them in the same order to avoid deadlocks)
-      sp.mutex.unlock(); 

       Threads.mutex.lock();
       sp.mutex.lock();
   }
       Threads.mutex.lock();
       sp.mutex.lock();
   }