Rewrite how threads are spawned

Instead of creating a running std::thread and
returning, wait in Thread c'tor that the native
thread of execution goes to sleep in idle_loop().

In this way we can simplify how search is started,
because when main thread is idle we are sure also
all other threads will be idle, in any case, even
at thread creation and startup.

After lazy smp went in, we can simpify and rewrite
a lot of logic that is now no more needed. This is
hopefully the final big cleanup.

Tested for no regression at 5+0.1 with 3 threads:
LLR: 2.95 (-2.94,2.94) [-5.00,0.00]
Total: 17411 W: 3248 L: 3198 D: 10965

No functional change.

src/benchmark.cpp

@@ -158,7 +158,7 @@ void benchmark(const Position& current, istream& is) {
           Search::StateStackPtr st;
           limits.startTime = now();
           Threads.start_thinking(pos, limits, st);
-          Threads.main()->join();
+          Threads.main()->wait_for_search_finished();
           nodes += Threads.nodes_searched();
       }
   }

src/position.h

@@ -28,7 +28,7 @@
 #include "types.h"
 
 class Position;
-struct Thread;
+class Thread;
 
 namespace PSQT {
 

src/search.cpp

@@ -290,12 +290,11 @@ void MainThread::search() {
       {
           th->maxPly = 0;
           th->rootDepth = DEPTH_ZERO;
-          th->searching = true;
           if (th != this)
           {
               th->rootPos = Position(rootPos, th);
               th->rootMoves = rootMoves;
-              th->notify_one(); // Wake up the thread and start searching
+              th->start_searching();
           }
       }
 
@@ -324,7 +323,7 @@ void MainThread::search() {
   // Wait until all threads have finished
   for (Thread* th : Threads)
       if (th != this)
-          th->join();
+          th->wait_for_search_finished();
 
   // Check if there are threads with a better score than main thread.
   Thread* bestThread = this;

src/thread.cpp

@@ -29,52 +29,46 @@ using namespace Search;
 
 ThreadPool Threads; // Global object
 
-// Thread constructor makes some init and launches the thread that will go to
+/// Thread constructor launch the thread and then wait until it goes to sleep
-// sleep in idle_loop().
+/// in idle_loop().
 
 Thread::Thread() {
 
-  searching = true; // Avoid a race with start_thinking()
+  resetCalls = exit = false;
-  exit = resetCalls = false;
   maxPly = callsCnt = 0;
   history.clear();
   counterMoves.clear();
-  idx = Threads.size(); // Starts from 0
+  idx = Threads.size(); // Start from 0
-  std::thread::operator=(std::thread(&Thread::idle_loop, this));
+
+  std::unique_lock<Mutex> lk(mutex);
+  searching = true;
+  nativeThread = std::thread(&Thread::idle_loop, this);
+  sleepCondition.wait(lk, [&]{ return !searching; });
 }
 
 
-// Thread destructor waits for thread termination before deleting
+/// Thread destructor wait for thread termination before returning
 
 Thread::~Thread() {
 
   mutex.lock();
-  exit = true; // Search must be already finished
+  exit = true;
+  sleepCondition.notify_one();
   mutex.unlock();
-
+  nativeThread.join();
-  notify_one();
-  std::thread::join(); // Wait for thread termination
 }
 
 
-// Thread::join() waits for the thread to finish searching
+/// Thread::wait_for_search_finished() wait on sleep condition until not searching
-void Thread::join() {
+
+void Thread::wait_for_search_finished() {
 
   std::unique_lock<Mutex> lk(mutex);
   sleepCondition.wait(lk, [&]{ return !searching; });
 }
 
 
-// Thread::notify_one() wakes up the thread when there is some work to do
+/// Thread::wait() wait on sleep condition until condition is true
-
-void Thread::notify_one() {
-
-  std::unique_lock<Mutex> lk(mutex);
-  sleepCondition.notify_one();
-}
-
-
-// Thread::wait() set the thread to sleep until 'condition' turns true
 
 void Thread::wait(std::atomic_bool& condition) {
 
@@ -83,7 +77,20 @@ void Thread::wait(std::atomic_bool& condition) {
 }
 
 
-// Thread::idle_loop() is where the thread is parked when it has no work to do
+/// Thread::start_searching() wake up the thread that will start the search
+
+void Thread::start_searching(bool resume) {
+
+  std::unique_lock<Mutex> lk(mutex);
+
+  if (!resume)
+      searching = true;
+
+  sleepCondition.notify_one();
+}
+
+
+/// Thread::idle_loop() is where the thread is parked when it has no work to do
 
 void Thread::idle_loop() {
 
@@ -95,22 +102,22 @@ void Thread::idle_loop() {
 
       while (!searching && !exit)
       {
-          sleepCondition.notify_one(); // Wake up main thread if needed
+          sleepCondition.notify_one(); // Wake up any waiting thread
           sleepCondition.wait(lk);
       }
 
       lk.unlock();
 
-      if (!exit && searching)
+      if (!exit)
           search();
   }
 }
 
 
-// ThreadPool::init() is called at startup to create and launch requested threads,
+/// ThreadPool::init() create and launch requested threads, that will go
-// that will go immediately to sleep. We cannot use a constructor because Threads
+/// immediately to sleep. We cannot use a constructor because Threads is a
-// is a static object and we need a fully initialized engine at this point due to
+/// static object and we need a fully initialized engine at this point due to
-// allocation of Endgames in the Thread constructor.
+/// allocation of Endgames in the Thread constructor.
 
 void ThreadPool::init() {
 
@@ -119,23 +126,20 @@ void ThreadPool::init() {
 }
 
 
-// ThreadPool::exit() terminates the threads before the program exits. Cannot be
+/// ThreadPool::exit() terminate threads before the program exits. Cannot be
-// done in destructor because threads must be terminated before freeing us.
+/// done in destructor because threads must be terminated before deleting any
+/// static objects, so while still in main().
 
 void ThreadPool::exit() {
 
-  for (Thread* th : *this)
+  while (size())
-      delete th;
+      delete back(), pop_back();
-
-  clear(); // Get rid of stale pointers
 }
 
 
-// ThreadPool::read_uci_options() updates internal threads parameters from the
+/// ThreadPool::read_uci_options() updates internal threads parameters from the
-// corresponding UCI options and creates/destroys threads to match the requested
+/// corresponding UCI options and creates/destroys threads to match requested
-// number. Thread objects are dynamically allocated to avoid creating all possible
+/// number. Thread objects are dynamically allocated.
-// threads in advance (which include pawns and material tables), even if only a
-// few are to be used.
 
 void ThreadPool::read_uci_options() {
 
@@ -147,31 +151,28 @@ void ThreadPool::read_uci_options() {
       push_back(new Thread);
 
   while (size() > requested)
-  {
+      delete back(), pop_back();
-      delete back();
-      pop_back();
-  }
 }
 
 
-// ThreadPool::nodes_searched() returns the number of nodes searched
+/// ThreadPool::nodes_searched() return the number of nodes searched
 
 int64_t ThreadPool::nodes_searched() {
 
   int64_t nodes = 0;
-  for (Thread *th : *this)
+  for (Thread* th : *this)
       nodes += th->rootPos.nodes_searched();
   return nodes;
 }
 
 
-// ThreadPool::start_thinking() wakes up the main thread sleeping in
+/// ThreadPool::start_thinking() wake up the main thread sleeping in idle_loop()
-// MainThread::idle_loop() and starts a new search, then returns immediately.
+/// and start a new search, then return immediately.
 
 void ThreadPool::start_thinking(const Position& pos, const LimitsType& limits,
                                 StateStackPtr& states) {
-  for (Thread* th : Threads)
+
-      th->join();
+  main()->wait_for_search_finished();
 
   Signals.stopOnPonderhit = Signals.firstRootMove = false;
   Signals.stop = Signals.failedLowAtRoot = false;
@@ -190,6 +191,5 @@ void ThreadPool::start_thinking(const Position& pos, const LimitsType& limits,
           || std::count(limits.searchmoves.begin(), limits.searchmoves.end(), m))
           main()->rootMoves.push_back(RootMove(m));
 
-  main()->searching = true;
+  main()->start_searching();
-  main()->notify_one(); // Wake up main thread: 'searching' must be already set
 }

src/thread.h

@@ -40,20 +40,22 @@
 /// entry its life time is unlimited and we don't have to care about someone
 /// changing the entry under our feet.
 
-struct Thread : public std::thread {
+class Thread {
 
+  std::thread nativeThread;
+  Mutex mutex;
+  ConditionVariable sleepCondition;
+  bool exit, searching;
+
+public:
   Thread();
   virtual ~Thread();
   virtual void search();
   void idle_loop();
-  void join();
+  void start_searching(bool resume = false);
-  void notify_one();
+  void wait_for_search_finished();
   void wait(std::atomic_bool& b);
 
-  std::atomic_bool exit, searching, resetCalls;
-  Mutex mutex;
-  ConditionVariable sleepCondition;
-
   Pawns::Table pawnsTable;
   Material::Table materialTable;
   Endgames endgames;
@@ -66,10 +68,11 @@ struct Thread : public std::thread {
   HistoryStats history;
   MovesStats counterMoves;
   Depth completedDepth;
+  std::atomic_bool resetCalls;
 };
 
 
-/// MainThread is a derived classes used to characterize the the main one
+/// MainThread is a derived class with a specific overload for the main thread
 
 struct MainThread : public Thread {
   virtual void search();
@@ -77,8 +80,8 @@ struct MainThread : public Thread {
 
 
 /// ThreadPool struct handles all the threads related stuff like init, starting,
-/// parking and, most importantly, launching a thread.
+/// parking and, most importantly, launching a thread. All the access to threads
-/// All the access to shared thread data is done through this class.
+/// data is done through this class.
 
 struct ThreadPool : public std::vector<Thread*> {
 
@@ -86,8 +89,8 @@ struct ThreadPool : public std::vector<Thread*> {
   void exit(); // be initialized and valid during the whole thread lifetime.
 
   MainThread* main() { return static_cast<MainThread*>(at(0)); }
-  void read_uci_options();
   void start_thinking(const Position&, const Search::LimitsType&, Search::StateStackPtr&);
+  void read_uci_options();
   int64_t nodes_searched();
 };
 

src/uci.cpp

@@ -170,7 +170,7 @@ void UCI::loop(int argc, char* argv[]) {
           || (token == "ponderhit" && Search::Signals.stopOnPonderhit))
       {
           Search::Signals.stop = true;
-          Threads.main()->notify_one(); // Could be sleeping
+          Threads.main()->start_searching(true); // Could be sleeping
       }
       else if (token == "ponderhit")
           Search::Limits.ponder = 0; // Switch to normal search
@@ -211,7 +211,7 @@ void UCI::loop(int argc, char* argv[]) {
 
   } while (token != "quit" && argc == 1); // Passed args have one-shot behaviour
 
-  Threads.main()->join(); // Cannot quit whilst the search is running
+  Threads.main()->wait_for_search_finished();
 }