From 3441e0075decb1acd8d3b4020b7822f743613124 Mon Sep 17 00:00:00 2001 From: Marco Costalba Date: Sun, 19 Feb 2012 10:32:06 +0100 Subject: [PATCH] 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 --- src/search.cpp | 3 --- src/thread.cpp | 14 +++++++++----- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/search.cpp b/src/search.cpp index 73050bdf..16efe2db 100644 --- 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 7507ded1..4fa08148 100644 --- 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); -- 2.39.2