From 81c7975dcdf37ec96d308f800c2a40f6cb08907e Mon Sep 17 00:00:00 2001 From: Joona Kiiski Date: Wed, 11 Mar 2015 21:50:41 +0000 Subject: [PATCH] Use thread specific mutexes instead of a global one. This is necessary to improve the scalability with high number of cores. There is no functional change in a single thread mode. Resolves #281 --- src/search.cpp | 28 ++++++++++++++-------------- src/thread.cpp | 38 ++++++++++++++++++++------------------ src/thread.h | 1 - 3 files changed, 34 insertions(+), 33 deletions(-) diff --git a/src/search.cpp b/src/search.cpp index 14e95a84..501d6c60 100644 --- a/src/search.cpp +++ b/src/search.cpp @@ -1526,13 +1526,12 @@ void Thread::idle_loop() { // If this thread has been assigned work, launch a search while (searching) { - Threads.mutex.lock(); + mutex.lock(); assert(activeSplitPoint); - SplitPoint* sp = activeSplitPoint; - Threads.mutex.unlock(); + mutex.unlock(); Stack stack[MAX_PLY+4], *ss = stack+2; // To allow referencing (ss-2) and (ss+2) Position pos(*sp->pos, this); @@ -1618,20 +1617,24 @@ void Thread::idle_loop() { sp = bestSp; // Recheck the conditions under lock protection - Threads.mutex.lock(); sp->mutex.lock(); if ( sp->allSlavesSearching - && sp->slavesMask.count() < MAX_SLAVES_PER_SPLITPOINT - && can_join(sp)) + && sp->slavesMask.count() < MAX_SLAVES_PER_SPLITPOINT) { - sp->slavesMask.set(idx); - activeSplitPoint = sp; - searching = true; + mutex.lock(); + + if (can_join(sp)) + { + sp->slavesMask.set(idx); + activeSplitPoint = sp; + searching = true; + } + + mutex.unlock(); } sp->mutex.unlock(); - Threads.mutex.unlock(); } } @@ -1687,12 +1690,11 @@ void check_time() { else if (Limits.nodes) { - Threads.mutex.lock(); - int64_t nodes = RootPos.nodes_searched(); // Loop across all split points and sum accumulated SplitPoint nodes plus // all the currently active positions nodes. + // FIXME: Racy... for (Thread* th : Threads) for (size_t i = 0; i < th->splitPointsSize; ++i) { @@ -1709,8 +1711,6 @@ void check_time() { sp.mutex.unlock(); } - Threads.mutex.unlock(); - if (nodes >= Limits.nodes) Signals.stop = true; } diff --git a/src/thread.cpp b/src/thread.cpp index 3f901445..d8740db2 100644 --- a/src/thread.cpp +++ b/src/thread.cpp @@ -144,6 +144,8 @@ void Thread::split(Position& pos, Stack* ss, Value alpha, Value beta, Value* bes // Pick and init the next available split point SplitPoint& sp = splitPoints[splitPointsSize]; + sp.mutex.lock(); // No contention here until we don't increment splitPointsSize + sp.master = this; sp.parentSplitPoint = activeSplitPoint; sp.slavesMask = 0, sp.slavesMask.set(idx); @@ -160,27 +162,29 @@ void Thread::split(Position& pos, Stack* ss, Value alpha, Value beta, Value* bes sp.nodes = 0; sp.cutoff = false; sp.ss = ss; - - // Try to allocate available threads and ask them to start searching setting - // 'searching' flag. This must be done under lock protection to avoid concurrent - // allocation of the same slave by another master. - Threads.mutex.lock(); - sp.mutex.lock(); - sp.allSlavesSearching = true; // Must be set under lock protection + ++splitPointsSize; activeSplitPoint = &sp; activePosition = nullptr; + // Try to allocate available threads Thread* slave; while ( sp.slavesMask.count() < MAX_SLAVES_PER_SPLITPOINT - && (slave = Threads.available_slave(activeSplitPoint)) != nullptr) + && (slave = Threads.available_slave(&sp)) != nullptr) { - sp.slavesMask.set(slave->idx); - slave->activeSplitPoint = activeSplitPoint; - slave->searching = true; // Slave leaves idle_loop() - slave->notify_one(); // Could be sleeping + slave->mutex.lock(); + + if (slave->can_join(activeSplitPoint)) + { + activeSplitPoint->slavesMask.set(slave->idx); + slave->activeSplitPoint = activeSplitPoint; + slave->searching = true; + slave->sleepCondition.notify_one(); // Could be sleeping + } + + slave->mutex.unlock(); } // Everything is set up. The master thread enters the idle loop, from which @@ -188,7 +192,6 @@ void Thread::split(Position& pos, Stack* ss, Value alpha, Value beta, Value* bes // The thread will return from the idle loop when all slaves have finished // their work at this split point. sp.mutex.unlock(); - Threads.mutex.unlock(); Thread::idle_loop(); // Force a call to base class idle_loop() @@ -198,13 +201,13 @@ void Thread::split(Position& pos, Stack* ss, Value alpha, Value beta, Value* bes assert(!searching); assert(!activePosition); + searching = true; + // We have returned from the idle loop, which means that all threads are - // finished. Note that setting 'searching' and decreasing splitPointsSize must - // be done under lock protection to avoid a race with Thread::available_to(). - Threads.mutex.lock(); + // finished. Note that decreasing splitPointsSize must be done under lock + // protection to avoid a race with Thread::can_join(). sp.mutex.lock(); - searching = true; --splitPointsSize; activeSplitPoint = sp.parentSplitPoint; activePosition = &pos; @@ -213,7 +216,6 @@ void Thread::split(Position& pos, Stack* ss, Value alpha, Value beta, Value* bes *bestValue = sp.bestValue; sp.mutex.unlock(); - Threads.mutex.unlock(); } diff --git a/src/thread.h b/src/thread.h index 231443db..4d70bf2f 100644 --- a/src/thread.h +++ b/src/thread.h @@ -151,7 +151,6 @@ struct ThreadPool : public std::vector { void start_thinking(const Position&, const Search::LimitsType&, Search::StateStackPtr&); Depth minimumSplitDepth; - Mutex mutex; ConditionVariable sleepCondition; TimerThread* timer; }; -- 2.39.2