Move some stuff out of lock protection in split()

We shouldn't need lock protection to increment
splitPointsCnt and set curSplitPoint of masterThread.

Anyhow because this code is very tricky and prone to
races bound the change in a single patch.

No functional change.

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

diff --git a/src/search.cpp b/src/search.cpp
index 73050bdf12534065495807d016753debd2e8ca3b..16efe2db15cc5920fb0966bb64c4a493508bc88a 100644 (file)
--- a/src/search.cpp
+++ b/src/search.cpp
@@ -1906,9 +1906,6 @@ void Thread::idle_loop(SplitPoint* sp_master) {
               Threads[master].wake_up();
       }
   }
-  // In helpful master concept a master can help only a sub-tree of its split
-  // point, and because here is all finished is not possible master is booked.
-  assert(!is_searching);
 }
 
 

diff --git a/src/thread.cpp b/src/thread.cpp
index 7507ded119e5329f5df4cb9170ce5e79769c54d1..4fa081482ffab4144de201a4fd56014e25c1be19 100644 (file)
--- a/src/thread.cpp
+++ b/src/thread.cpp
@@ -317,7 +317,7 @@ Value ThreadsManager::split(Position& pos, Stack* ss, Value alpha, Value beta,
       return bestValue;
 
   // Pick the next available split point from the split point stack
-  SplitPoint* sp = &masterThread.splitPoints[masterThread.splitPointsCnt];
+  SplitPoint* sp = &masterThread.splitPoints[masterThread.splitPointsCnt++];
 
   sp->parent = masterThread.curSplitPoint;
   sp->master = master;
@@ -337,6 +337,7 @@ Value ThreadsManager::split(Position& pos, Stack* ss, Value alpha, Value beta,
 
   assert(masterThread.is_searching);
 
+  masterThread.curSplitPoint = sp;
   int slavesCnt = 0;
 
   // Try to allocate available threads and ask them to start searching setting
@@ -359,9 +360,6 @@ Value ThreadsManager::split(Position& pos, Stack* ss, Value alpha, Value beta,
               break;
       }
 
-  masterThread.curSplitPoint = sp;
-  masterThread.splitPointsCnt++;
-
   lock_release(splitLock);
   lock_release(sp->lock);
 
@@ -371,10 +369,16 @@ Value ThreadsManager::split(Position& pos, Stack* ss, Value alpha, Value beta,
   // the thread will return from the idle loop when all slaves have finished
   // their work at this split point.
   if (slavesCnt || Fake)
+  {
       masterThread.idle_loop(sp);
 
+      // In helpful master concept a master can help only a sub-tree of its split
+      // point, and because here is all finished is not possible master is booked.
+      assert(!masterThread.is_searching);
+  }
+
   // We have returned from the idle loop, which means that all threads are
-  // finished. Note that setting is_searching and decreasing activeSplitPoints is
+  // finished. Note that setting is_searching and decreasing splitPointsCnt is
   // done under lock protection to avoid a race with Thread::is_available_to().
   lock_grab(sp->lock); // To protect sp->nodes
   lock_grab(splitLock);