Thread code reformat

Simplify out low level sync stuff (mutex
and friends) and avoid to use them directly
in many functions.

Also some renaming and better comment while
there.

No functional change.

src/main.cpp

@@ -45,7 +45,7 @@ int main(int argc, char* argv[]) {
   Pawns::init();
   Tablebases::init(Options["SyzygyPath"]);
   TT.resize(Options["Hash"]);
-  Threads.init();
+  Threads.init(Options["Threads"]);
   Search::clear(); // After threads are up
 
   UCI::loop(argc, argv);

src/search.cpp

@@ -341,7 +341,6 @@ void Thread::search() {
 
   bestValue = delta = alpha = -VALUE_INFINITE;
   beta = VALUE_INFINITE;
-  completedDepth = DEPTH_ZERO;
 
   if (mainThread)
   {

src/thread.cpp

@@ -24,47 +24,30 @@
 #include "movegen.h"
 #include "search.h"
 #include "thread.h"
-#include "uci.h"
 #include "syzygy/tbprobe.h"
 
 ThreadPool Threads; // Global object
 
-/// Thread constructor launches the thread and then waits until it goes to sleep
-/// in idle_loop().
 
-Thread::Thread() {
+/// Thread constructor launches the thread and waits until it goes to sleep
+/// in idle_loop(). Note that 'searching' and 'exit' should be alredy set.
 
-  exit = false;
-  selDepth = 0;
-  nodes = tbHits = 0;
-  idx = Threads.size(); // Start from 0
+Thread::Thread(size_t n) : idx(n), stdThread(&Thread::idle_loop, this) {
 
-  std::unique_lock<Mutex> lk(mutex);
-  searching = true;
-  nativeThread = std::thread(&Thread::idle_loop, this);
-  sleepCondition.wait(lk, [&]{ return !searching; });
+  wait_for_search_finished();
 }
 
 
-/// Thread destructor waits for thread termination before returning
+/// Thread destructor wakes up the thread in idle_loop() and waits
+/// for its termination. Thread should be already waiting.
 
 Thread::~Thread() {
 
-  mutex.lock();
+  assert(!searching);
+
   exit = true;
-  sleepCondition.notify_one();
-  mutex.unlock();
-  nativeThread.join();
-}
-
-
-/// Thread::wait_for_search_finished() waits on sleep condition
-/// until not searching
-
-void Thread::wait_for_search_finished() {
-
-  std::unique_lock<Mutex> lk(mutex);
-  sleepCondition.wait(lk, [&]{ return !searching; });
+  start_searching();
+  stdThread.join();
 }
 
 
@@ -72,103 +55,83 @@ void Thread::wait_for_search_finished() {
 
 void Thread::start_searching() {
 
-  std::unique_lock<Mutex> lk(mutex);
+  std::lock_guard<Mutex> lk(mutex);
   searching = true;
-  sleepCondition.notify_one();
+  cv.notify_one(); // Wake up the thread in idle_loop()
 }
 
 
-/// Thread::idle_loop() is where the thread is parked when it has no work to do
+/// Thread::wait_for_search_finished() blocks on the condition variable
+/// until the thread has finished searching.
+
+void Thread::wait_for_search_finished() {
+
+  std::unique_lock<Mutex> lk(mutex);
+  cv.wait(lk, [&]{ return !searching; });
+}
+
+
+/// Thread::idle_loop() is where the thread is parked, blocked on the
+/// condition variable, when it has no work to do.
 
 void Thread::idle_loop() {
 
   WinProcGroup::bindThisThread(idx);
 
-  while (!exit)
+  while (true)
   {
       std::unique_lock<Mutex> lk(mutex);
-
       searching = false;
+      cv.notify_one(); // Wake up anyone waiting for search finished
+      cv.wait(lk, [&]{ return searching; });
 
-      while (!searching && !exit)
-      {
-          sleepCondition.notify_one(); // Wake up any waiting thread
-          sleepCondition.wait(lk);
-      }
+      if (exit)
+          return;
 
       lk.unlock();
 
-      if (!exit)
-          search();
+      search();
   }
 }
 
 
-/// ThreadPool::init() creates and launches requested threads that will go
-/// immediately to sleep. We cannot use a constructor because Threads is a
-/// static object and we need a fully initialized engine at this point due to
-/// allocation of Endgames in the Thread constructor.
+/// ThreadPool::init() creates and launches the threads that will go
+/// immediately to sleep in idle_loop. We cannot use the c'tor because
+/// Threads is a static object and we need a fully initialized engine at
+/// this point due to allocation of Endgames in the Thread constructor.
 
-void ThreadPool::init() {
+void ThreadPool::init(size_t requested) {
 
-  push_back(new MainThread());
-  read_uci_options();
+  push_back(new MainThread(0));
+  set(requested);
 }
 
 
 /// ThreadPool::exit() terminates threads before the program exits. Cannot be
-/// done in destructor because threads must be terminated before deleting any
-/// static objects while still in main().
+/// done in the destructor because threads must be terminated before deleting
+/// any static object, so before main() returns.
 
 void ThreadPool::exit() {
 
-  while (size())
-      delete back(), pop_back();
+  main()->wait_for_search_finished();
+  set(0);
 }
 
 
-/// ThreadPool::read_uci_options() updates internal threads parameters from the
-/// corresponding UCI options and creates/destroys threads to match requested
-/// number. Thread objects are dynamically allocated.
+/// ThreadPool::set() creates/destroys threads to match the requested number
 
-void ThreadPool::read_uci_options() {
-
-  size_t requested = Options["Threads"];
-
-  assert(requested > 0);
+void ThreadPool::set(size_t requested) {
 
   while (size() < requested)
-      push_back(new Thread());
+      push_back(new Thread(size()));
 
   while (size() > requested)
       delete back(), pop_back();
 }
 
 
-/// ThreadPool::nodes_searched() returns the number of nodes searched
-
-uint64_t ThreadPool::nodes_searched() const {
-
-  uint64_t nodes = 0;
-  for (Thread* th : *this)
-      nodes += th->nodes.load(std::memory_order_relaxed);
-  return nodes;
-}
-
-
-/// ThreadPool::tb_hits() returns the number of TB hits
-
-uint64_t ThreadPool::tb_hits() const {
-
-  uint64_t hits = 0;
-  for (Thread* th : *this)
-      hits += th->tbHits.load(std::memory_order_relaxed);
-  return hits;
-}
-
-
-/// ThreadPool::start_thinking() wakes up the main thread sleeping in idle_loop()
-/// and starts a new search, then returns immediately.
+/// ThreadPool::start_thinking() wakes up main thread waiting in idle_loop() and
+/// returns immediately. Main thread will wake up other threads and start the search.
 
 void ThreadPool::start_thinking(Position& pos, StateListPtr& states,
                                 const Search::LimitsType& limits, bool ponderMode) {
@@ -183,7 +146,7 @@ void ThreadPool::start_thinking(Position& pos, StateListPtr& states,
   for (const auto& m : MoveList<LEGAL>(pos))
       if (   limits.searchmoves.empty()
           || std::count(limits.searchmoves.begin(), limits.searchmoves.end(), m))
-          rootMoves.push_back(Search::RootMove(m));
+          rootMoves.emplace_back(m);
 
   if (!rootMoves.empty())
       Tablebases::filter_root_moves(pos, rootMoves);
@@ -195,18 +158,20 @@ void ThreadPool::start_thinking(Position& pos, StateListPtr& states,
   if (states.get())
       setupStates = std::move(states); // Ownership transfer, states is now empty
 
-  StateInfo tmp = setupStates->back();
+  // We use Position::set() to set root position across threads. So we
+  // need to save and later to restore st->previous, cleared by set().
+  // Note that setupStates is shared by threads but is accessed in read-only mode.
+  StateInfo* previous = setupStates->back().previous;
 
   for (Thread* th : Threads)
   {
-      th->nodes = 0;
-      th->tbHits = 0;
-      th->rootDepth = DEPTH_ZERO;
+      th->nodes = th->tbHits = 0;
+      th->rootDepth = th->completedDepth = DEPTH_ZERO;
       th->rootMoves = rootMoves;
       th->rootPos.set(pos.fen(), pos.is_chess960(), &setupStates->back(), th);
   }
 
-  setupStates->back() = tmp; // Restore st->previous, cleared by Position::set()
+  setupStates->back().previous = previous;
 
   main()->start_searching();
 }

src/thread.h

@@ -35,20 +35,21 @@
 #include "thread_win32.h"
 
 
-/// Thread struct keeps together all the thread-related stuff. We also use
-/// per-thread pawn and material hash tables so that once we get a pointer to an
-/// entry its life time is unlimited and we don't have to care about someone
-/// changing the entry under our feet.
+/// Thread class keeps together all the thread-related stuff. We use
+/// per-thread pawn and material hash tables so that once we get a
+/// pointer to an entry its life time is unlimited and we don't have
+/// to care about someone changing the entry under our feet.
 
 class Thread {
 
-  std::thread nativeThread;
   Mutex mutex;
-  ConditionVariable sleepCondition;
-  bool exit, searching;
+  ConditionVariable cv;
+  size_t idx;
+  bool exit = false, searching = true; // Set before starting std::thread
+  std::thread stdThread;
 
 public:
-  Thread();
+  explicit Thread(size_t);
   virtual ~Thread();
   virtual void search();
   void idle_loop();
@@ -58,52 +59,62 @@ public:
   Pawns::Table pawnsTable;
   Material::Table materialTable;
   Endgames endgames;
-  size_t idx, PVIdx;
+  size_t PVIdx;
   int selDepth;
   std::atomic<uint64_t> nodes, tbHits;
 
   Position rootPos;
   Search::RootMoves rootMoves;
-  Depth rootDepth;
-  Depth completedDepth;
+  Depth rootDepth, completedDepth;
   CounterMoveHistory counterMoves;
   ButterflyHistory mainHistory;
   ContinuationHistory contHistory;
 };
 
 
-/// MainThread is a derived class with a specific overload for the main thread
+/// MainThread is a derived class specific for main thread
 
 struct MainThread : public Thread {
+
+  using Thread::Thread;
+
   virtual void search();
   void check_time();
 
   bool easyMovePlayed, failedLow;
   double bestMoveChanges;
   Value previousScore;
-  int callsCnt = 0;
+  int callsCnt;
 };
 
 
 /// ThreadPool struct handles all the threads-related stuff like init, starting,
 /// parking and, most importantly, launching a thread. All the access to threads
-/// data is done through this class.
+/// is done through this class.
 
 struct ThreadPool : public std::vector<Thread*> {
 
-  void init(); // No constructor and destructor, threads rely on globals that should
-  void exit(); // be initialized and valid during the whole thread lifetime.
-
-  MainThread* main() { return static_cast<MainThread*>(at(0)); }
+  void init(size_t); // No constructor and destructor, threads rely on globals that should
+  void exit();       // be initialized and valid during the whole thread lifetime.
   void start_thinking(Position&, StateListPtr&, const Search::LimitsType&, bool = false);
-  void read_uci_options();
-  uint64_t nodes_searched() const;
-  uint64_t tb_hits() const;
+  void set(size_t);
+
+  MainThread* main()        const { return static_cast<MainThread*>(front()); }
+  uint64_t nodes_searched() const { return accumulate(&Thread::nodes); }
+  uint64_t tb_hits()        const { return accumulate(&Thread::tbHits); }
 
   std::atomic_bool stop, ponder, stopOnPonderhit;
 
 private:
   StateListPtr setupStates;
+
+  uint64_t accumulate(std::atomic<uint64_t> Thread::* member) const {
+
+    uint64_t sum = 0;
+    for (Thread* th : *this)
+        sum += (th->*member).load(std::memory_order_relaxed);
+    return sum;
+  }
 };
 
 extern ThreadPool Threads;

src/ucioption.cpp

@@ -39,7 +39,7 @@ namespace UCI {
 void on_clear_hash(const Option&) { Search::clear(); }
 void on_hash_size(const Option& o) { TT.resize(o); }
 void on_logger(const Option& o) { start_logger(o); }
-void on_threads(const Option&) { Threads.read_uci_options(); }
+void on_threads(const Option& o) { Threads.set(o); }
 void on_tb_path(const Option& o) { Tablebases::init(o); }