Skip to content

Commit

Permalink
Use thread specific mutexes instead of a global one.
Browse files Browse the repository at this point in the history
This is necessary to improve the scalability with high number of cores.

There is no functional change in a single thread mode.

Resolves #281
  • Loading branch information
Joona Kiiski authored and zamar committed Mar 11, 2015
1 parent 4b59347 commit 81c7975
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 33 deletions.
28 changes: 14 additions & 14 deletions src/search.cpp
Expand Up @@ -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);
Expand Down Expand Up @@ -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();
}
}

Expand Down Expand Up @@ -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)
{
Expand All @@ -1709,8 +1711,6 @@ void check_time() {
sp.mutex.unlock();
}

Threads.mutex.unlock();

if (nodes >= Limits.nodes)
Signals.stop = true;
}
Expand Down
38 changes: 20 additions & 18 deletions src/thread.cpp
Expand Up @@ -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);
Expand All @@ -160,35 +162,36 @@ 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
// it will instantly launch a search, because its 'searching' flag is set.
// 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()

Expand All @@ -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;
Expand All @@ -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();
}


Expand Down
1 change: 0 additions & 1 deletion src/thread.h
Expand Up @@ -151,7 +151,6 @@ struct ThreadPool : public std::vector<Thread*> {
void start_thinking(const Position&, const Search::LimitsType&, Search::StateStackPtr&);

Depth minimumSplitDepth;
Mutex mutex;
ConditionVariable sleepCondition;
TimerThread* timer;
};
Expand Down

0 comments on commit 81c7975

Please sign in to comment.