Rework lock protecting

When changing 'search' and 'splitPointsSize' we have to
use thread locks, not split point ones, because can_join()
is called under the formers.

Verified succesfully with 24 hours toruture tests with 20
cores machine by Louis Zulli: it does not hangs.

Verifyed for no regressions with STC, 7 threads:
LLR: 2.94 (-2.94,2.94) [-3.00,1.00]
Total: 52804 W: 8159 L: 8087 D: 36558

No functional change.

diff --git a/src/search.cpp b/src/search.cpp
index 54a9ec878c44f5b617a30fbc85280de160f49628..9ff447beb6e0a873cc0b5252927175d0a476bb29 100644 (file)
--- a/src/search.cpp
+++ b/src/search.cpp
@@ -1622,11 +1622,15 @@ void Thread::idle_loop() {
           else
               assert(false);
 
           else
               assert(false);
 

-          spinlock.acquire();

           assert(searching);
 
           assert(searching);
 

+          spinlock.acquire();
+

           searching = false;
           activePosition = nullptr;
           searching = false;
           activePosition = nullptr;

+
+          spinlock.release();
+

           sp->slavesMask.reset(idx);
           sp->allSlavesSearching = false;
           sp->nodes += pos.nodes_searched();
           sp->slavesMask.reset(idx);
           sp->allSlavesSearching = false;
           sp->nodes += pos.nodes_searched();


@@ -1634,7 +1638,6 @@ void Thread::idle_loop() {
           // After releasing the lock we can't access any SplitPoint related data
           // in a safe way because it could have been released under our feet by
           // the sp master.
           // After releasing the lock we can't access any SplitPoint related data
           // in a safe way because it could have been released under our feet by
           // the sp master.

-          spinlock.release();

           sp->spinlock.release();
 
           // Try to late join to another split point if none of its slaves has
           sp->spinlock.release();
 
           // Try to late join to another split point if none of its slaves has

diff --git a/src/thread.cpp b/src/thread.cpp
index e2881c4ee7805621259f445ceab752618135e49a..cdb0d5412a501b781e4064815eacf255b358ef66 100644 (file)
--- a/src/thread.cpp
+++ b/src/thread.cpp
@@ -145,7 +145,6 @@ void Thread::split(Position& pos, Stack* ss, Value alpha, Value beta, Value* bes
   SplitPoint& sp = splitPoints[splitPointsSize];
 
   sp.spinlock.acquire(); // No contention here until we don't increment splitPointsSize
   SplitPoint& sp = splitPoints[splitPointsSize];
 
   sp.spinlock.acquire(); // No contention here until we don't increment splitPointsSize

-  spinlock.acquire();

 
   sp.master = this;
   sp.parentSplitPoint = activeSplitPoint;
 
   sp.master = this;
   sp.parentSplitPoint = activeSplitPoint;


@@ -168,7 +167,6 @@ void Thread::split(Position& pos, Stack* ss, Value alpha, Value beta, Value* bes
   ++splitPointsSize;
   activeSplitPoint = &sp;
   activePosition = nullptr;
   ++splitPointsSize;
   activeSplitPoint = &sp;
   activePosition = nullptr;

-  spinlock.release();

 
   // Try to allocate available threads
   Thread* slave;
 
   // Try to allocate available threads
   Thread* slave;


@@ -196,29 +194,28 @@ void Thread::split(Position& pos, Stack* ss, Value alpha, Value beta, Value* bes
 
   Thread::idle_loop(); // Force a call to base class idle_loop()
 
 
   Thread::idle_loop(); // Force a call to base class idle_loop()
 

-  sp.spinlock.acquire();
-  spinlock.acquire();
-

   // In the helpful master concept, a master can help only a sub-tree of its
   // split point and because everything is finished here, it's not possible
   // for the master to be booked.
   assert(!searching);
   assert(!activePosition);
 
   // In the helpful master concept, a master can help only a sub-tree of its
   // split point and because everything is finished here, it's not possible
   // for the master to be booked.
   assert(!searching);
   assert(!activePosition);
 

-  searching = true;
-

   // We have returned from the idle loop, which means that all threads are
   // finished. Note that decreasing splitPointsSize must be done under lock
   // protection to avoid a race with Thread::can_join().
   // We have returned from the idle loop, which means that all threads are
   // finished. Note that decreasing splitPointsSize must be done under lock
   // protection to avoid a race with Thread::can_join().

+  spinlock.acquire();
+
+  searching = true;

   --splitPointsSize;
   activeSplitPoint = sp.parentSplitPoint;
   activePosition = &pos;
   --splitPointsSize;
   activeSplitPoint = sp.parentSplitPoint;
   activePosition = &pos;

+
+  spinlock.release();
+
+  // Split point data cannot be changed now, so no need to lock protect

   pos.set_nodes_searched(pos.nodes_searched() + sp.nodes);
   *bestMove = sp.bestMove;
   *bestValue = sp.bestValue;
   pos.set_nodes_searched(pos.nodes_searched() + sp.nodes);
   *bestMove = sp.bestMove;
   *bestValue = sp.bestValue;

-
-  spinlock.release();
-  sp.spinlock.release();

 }
 
 
 }