From: Marco Costalba Date: Thu, 26 Jan 2012 22:18:43 +0000 (+0100) Subject: Reformat threads code X-Git-Url: https://git.sesse.net/?p=stockfish;a=commitdiff_plain;h=7fb6fd2f558c9e2967fb393238d1dbe063f2d277 Reformat threads code Apart from some renaming the biggest change is the retire of split_point_finished() replaced by slavesMask flags. As a side effect we now take also split point lock when allocation available threads. No functional change. Signed-off-by: Marco Costalba --- diff --git a/src/search.cpp b/src/search.cpp index 2720edb3..f0bbf0fd 100644 --- a/src/search.cpp +++ b/src/search.cpp @@ -573,20 +573,26 @@ namespace { thread.maxPly = ss->ply; // Step 1. Initialize node - if (!SpNode) - { - ss->currentMove = ss->bestMove = threatMove = (ss+1)->excludedMove = MOVE_NONE; - (ss+1)->skipNullMove = false; (ss+1)->reduction = DEPTH_ZERO; - (ss+2)->killers[0] = (ss+2)->killers[1] = MOVE_NONE; - } - else + if (SpNode) { - sp = ss->sp; tte = NULL; ttMove = excludedMove = MOVE_NONE; + sp = ss->sp; threatMove = sp->threatMove; + bestValue = sp->bestValue; + moveCount = sp->moveCount; // Lock must be held here + + assert(bestValue > -VALUE_INFINITE && moveCount > 0); + goto split_point_start; } + else + { + ss->currentMove = ss->bestMove = threatMove = (ss+1)->excludedMove = MOVE_NONE; + (ss+1)->skipNullMove = false; (ss+1)->reduction = DEPTH_ZERO; + (ss+2)->killers[0] = (ss+2)->killers[1] = MOVE_NONE; + + } // Step 2. Check for aborted search and immediate draw // Enforce node limit here. FIXME: This only works with 1 search thread. @@ -820,14 +826,6 @@ split_point_start: // At split points actual search starts from here && !excludedMove // Recursive singular search is not allowed && (tte->type() & VALUE_TYPE_LOWER) && tte->depth() >= depth - 3 * ONE_PLY; - if (SpNode) - { - lock_grab(sp->lock); - bestValue = sp->bestValue; - moveCount = sp->moveCount; - - assert(bestValue > -VALUE_INFINITE && moveCount > 0); - } // Step 11. Loop through moves // Loop through all pseudo-legal moves until no moves remain or a beta cutoff occurs @@ -1129,14 +1127,6 @@ split_point_start: // At split points actual search starts from here } } - if (SpNode) - { - // Here we have the lock still grabbed - sp->is_slave[pos.thread()] = false; - sp->nodes += pos.nodes_searched(); - lock_release(sp->lock); - } - assert(bestValue > -VALUE_INFINITE && bestValue < VALUE_INFINITE); return bestValue; @@ -1836,24 +1826,24 @@ void RootMove::insert_pv_in_tt(Position& pos) { /// Thread::idle_loop() is where the thread is parked when it has no work to do. -/// The parameter 'sp', if non-NULL, is a pointer to an active SplitPoint object -/// for which the thread is the master. +/// The parameter 'master_sp', if non-NULL, is a pointer to an active SplitPoint +/// object for which the thread is the master. -void Thread::idle_loop(SplitPoint* sp) { +void Thread::idle_loop(SplitPoint* sp_master) { - while (true) + // If this thread is the master of a split point and all slaves have + // finished their work at this split point, return from the idle loop. + while (!sp_master || sp_master->slavesMask) { // If we are not searching, wait for a condition to be signaled // instead of wasting CPU time polling for work. while ( do_sleep - || do_terminate - || (Threads.use_sleeping_threads() && !is_searching)) + || do_exit + || (!is_searching && Threads.use_sleeping_threads())) { - assert((!sp && threadID) || Threads.use_sleeping_threads()); - - if (do_terminate) + if (do_exit) { - assert(!sp); + assert(!sp_master); return; } @@ -1861,7 +1851,7 @@ void Thread::idle_loop(SplitPoint* sp) { lock_grab(sleepLock); // If we are master and all slaves have finished don't go to sleep - if (sp && Threads.split_point_finished(sp)) + if (sp_master && !sp_master->slavesMask) { lock_release(sleepLock); break; @@ -1880,46 +1870,42 @@ void Thread::idle_loop(SplitPoint* sp) { // If this thread has been assigned work, launch a search if (is_searching) { - assert(!do_terminate); + assert(!do_sleep && !do_exit); // Copy split point position and search stack and call search() Stack ss[MAX_PLY_PLUS_2]; - SplitPoint* tsp = splitPoint; - Position pos(*tsp->pos, threadID); - - memcpy(ss, tsp->ss - 1, 4 * sizeof(Stack)); - (ss+1)->sp = tsp; - - if (tsp->nodeType == Root) - search(pos, ss+1, tsp->alpha, tsp->beta, tsp->depth); - else if (tsp->nodeType == PV) - search(pos, ss+1, tsp->alpha, tsp->beta, tsp->depth); - else if (tsp->nodeType == NonPV) - search(pos, ss+1, tsp->alpha, tsp->beta, tsp->depth); + SplitPoint* sp = splitPoint; + Position pos(*sp->pos, threadID); + + memcpy(ss, sp->ss - 1, 4 * sizeof(Stack)); + (ss+1)->sp = sp; + + lock_grab(sp->lock); + + if (sp->nodeType == Root) + search(pos, ss+1, sp->alpha, sp->beta, sp->depth); + else if (sp->nodeType == PV) + search(pos, ss+1, sp->alpha, sp->beta, sp->depth); + else if (sp->nodeType == NonPV) + search(pos, ss+1, sp->alpha, sp->beta, sp->depth); else assert(false); assert(is_searching); + // We return from search with lock held + sp->slavesMask &= ~(1ULL << threadID); + sp->nodes += pos.nodes_searched(); + lock_release(sp->lock); + is_searching = false; // Wake up master thread so to allow it to return from the idle loop in // case we are the last slave of the split point. if ( Threads.use_sleeping_threads() - && threadID != tsp->master - && !Threads[tsp->master].is_searching) - Threads[tsp->master].wake_up(); - } - - // If this thread is the master of a split point and all slaves have - // finished their work at this split point, return from the idle loop. - if (sp && Threads.split_point_finished(sp)) - { - // Because sp->is_slave[] is reset under lock protection, - // be sure sp->lock has been released before to return. - lock_grab(sp->lock); - lock_release(sp->lock); - return; + && threadID != sp->master + && !Threads[sp->master].is_searching) + Threads[sp->master].wake_up(); } } } diff --git a/src/thread.cpp b/src/thread.cpp index 81a718df..3b7a10ee 100644 --- a/src/thread.cpp +++ b/src/thread.cpp @@ -97,12 +97,11 @@ bool Thread::is_available_to(int master) const { // Make a local copy to be sure doesn't become zero under our feet while // testing next condition and so leading to an out of bound access. - int localActiveSplitPoints = activeSplitPoints; + int sp_count = activeSplitPoints; // No active split points means that the thread is available as a slave for any // other thread otherwise apply the "helpful master" concept if possible. - if ( !localActiveSplitPoints - || splitPoints[localActiveSplitPoints - 1].is_slave[master]) + if (!sp_count || (splitPoints[sp_count - 1].slavesMask & (1ULL << master))) return true; return false; @@ -190,9 +189,11 @@ void ThreadsManager::init() { void ThreadsManager::exit() { + assert(threads[0].is_searching == false); + for (int i = 0; i <= MAX_THREADS; i++) { - threads[i].do_terminate = true; // Search must be already finished + threads[i].do_exit = true; // Search must be already finished threads[i].wake_up(); thread_join(threads[i].handle); // Wait for thread termination @@ -225,19 +226,6 @@ bool ThreadsManager::available_slave_exists(int master) const { } -// split_point_finished() checks if all the slave threads of a given split -// point have finished searching. - -bool ThreadsManager::split_point_finished(SplitPoint* sp) const { - - for (int i = 0; i < activeThreads; i++) - if (sp->is_slave[i]) - return false; - - return true; -} - - // split() does the actual work of distributing the work at a node between // several available threads. If it does not succeed in splitting the node // (because no idle threads are available, or because we have no unused split @@ -274,6 +262,7 @@ Value ThreadsManager::split(Position& pos, Stack* ss, Value alpha, Value beta, sp->parent = masterThread.splitPoint; sp->master = master; sp->is_betaCutoff = false; + sp->slavesMask = (1ULL << master); sp->depth = depth; sp->threatMove = threatMove; sp->alpha = alpha; @@ -286,9 +275,6 @@ Value ThreadsManager::split(Position& pos, Stack* ss, Value alpha, Value beta, sp->nodes = 0; sp->ss = ss; - for (i = 0; i < activeThreads; i++) - sp->is_slave[i] = false; - // If we are here it means we are not available assert(masterThread.is_searching); @@ -298,21 +284,25 @@ Value ThreadsManager::split(Position& pos, Stack* ss, Value alpha, Value beta, // is_searching flag. This must be done under lock protection to avoid concurrent // allocation of the same slave by another master. lock_grab(threadsLock); + lock_grab(sp->lock); // To protect sp->slaves_mask - for (i = 0; !Fake && i < activeThreads && workersCnt < maxThreadsPerSplitPoint; i++) + for (i = 0; !Fake && i < activeThreads; i++) if (threads[i].is_available_to(master)) { - workersCnt++; - sp->is_slave[i] = true; + sp->slavesMask |= (1ULL << i); threads[i].splitPoint = sp; - // This makes the slave to exit from idle_loop() + // Allocate the slave and make it exit from idle_loop() threads[i].is_searching = true; if (useSleepingThreads) threads[i].wake_up(); + + if (++workersCnt >= maxThreadsPerSplitPoint) + break; } + lock_release(sp->lock); lock_release(threadsLock); // We failed to allocate even one slave, return @@ -337,15 +327,17 @@ Value ThreadsManager::split(Position& pos, Stack* ss, Value alpha, Value beta, // finished. Note that changing state and decreasing activeSplitPoints is done // under lock protection to avoid a race with Thread::is_available_to(). lock_grab(threadsLock); + lock_grab(sp->lock); // To protect sp->nodes + masterThread.is_searching = true; masterThread.activeSplitPoints--; - - lock_release(threadsLock); - masterThread.splitPoint = sp->parent; pos.set_nodes_searched(pos.nodes_searched() + sp->nodes); + lock_release(sp->lock); + lock_release(threadsLock); + return sp->bestValue; } @@ -360,7 +352,7 @@ extern void check_time(); void Thread::timer_loop() { - while (!do_terminate) + while (!do_exit) { lock_grab(sleepLock); timed_wait(sleepCond, sleepLock, maxPly ? maxPly : INT_MAX); @@ -396,7 +388,7 @@ void Thread::main_loop() { do_sleep = true; // Always return to sleep after a search is_searching = false; - while (do_sleep && !do_terminate) + while (do_sleep && !do_exit) { cond_signal(Threads.sleepCond); // Wake up UI thread if needed cond_wait(sleepCond, sleepLock); @@ -406,7 +398,7 @@ void Thread::main_loop() { lock_release(sleepLock); - if (do_terminate) + if (do_exit) return; Search::think(); diff --git a/src/thread.h b/src/thread.h index 80363fa7..beb09e4d 100644 --- a/src/thread.h +++ b/src/thread.h @@ -50,12 +50,12 @@ struct SplitPoint { // Shared data Lock lock; + volatile uint64_t slavesMask; volatile int64_t nodes; volatile Value alpha; volatile Value bestValue; volatile int moveCount; volatile bool is_betaCutoff; - volatile bool is_slave[MAX_THREADS]; }; @@ -69,7 +69,7 @@ struct Thread { void wake_up(); bool cutoff_occurred() const; bool is_available_to(int master) const; - void idle_loop(SplitPoint* sp); + void idle_loop(SplitPoint* sp_master); void main_loop(); void timer_loop(); @@ -85,7 +85,7 @@ struct Thread { volatile int activeSplitPoints; volatile bool is_searching; volatile bool do_sleep; - volatile bool do_terminate; + volatile bool do_exit; }; @@ -110,7 +110,6 @@ public: void set_size(int cnt); void read_uci_options(); bool available_slave_exists(int master) const; - bool split_point_finished(SplitPoint* sp) const; void set_timer(int msec); void wait_for_stop_or_ponderhit(); void stop_thinking();