Modernize code base a little bit

Removed sprintf() which generated a warning, because of security reasons.
Replace NULL with nullptr
Replace typedef with using
Do not inherit from std::vector. Use composition instead.
optimize mutex-unlocking

closes https://github.com/official-stockfish/Stockfish/pull/4327

No functional change

diff --git a/AUTHORS b/AUTHORS
index 998399b969dbb5cbc3494ea8dfb05f6b6e4664ca..87fed7b80c708c5c7e98cce2013e7d866e9a66bb 100644 (file)
--- a/AUTHORS
+++ b/AUTHORS
@@ -192,6 +192,7 @@ Shawn Varghese (xXH4CKST3RXx)
 Siad Daboul (Topologist)
 Stefan Geschwentner (locutus2)
 Stefano Cardanobile (Stefano80)
+Stefano Di Martino (StefanoD)
 Steinar Gunderson (sesse)
 Stéphane Nicolet (snicolet)
 Syine Mineta (MinetaS)

diff --git a/src/material.h b/src/material.h
index f6db85c4226d6f7964fbd54bc0d2bdb0eb125e16..73c831006bfad7994adb9ba693d8dd7ccdf20730 100644 (file)
--- a/src/material.h
+++ b/src/material.h
@@ -28,7 +28,7 @@ namespace Stockfish::Material {
 
 /// Material::Entry contains various information about a material configuration.
 /// It contains a material imbalance evaluation, a function pointer to a special
-/// endgame evaluation function (which in most cases is NULL, meaning that the
+/// endgame evaluation function (which in most cases is nullptr, meaning that the
 /// standard evaluation function will be used), and scale factors.
 ///
 /// The scale factors are used to scale the evaluation score up or down. For

diff --git a/src/misc.cpp b/src/misc.cpp
index 1cb515c3e41f20e6d081061e806b77f7436b2df6..be7abba5d017ddae5f5068008f0b914fe8a45942 100644 (file)
--- a/src/misc.cpp
+++ b/src/misc.cpp
@@ -413,7 +413,7 @@ static void* aligned_large_pages_alloc_windows([[maybe_unused]] size_t allocSize
   if (!OpenProcessToken(GetCurrentProcess(), TOKEN_ADJUST_PRIVILEGES | TOKEN_QUERY, &hProcessToken))
       return nullptr;
 
-  if (LookupPrivilegeValue(NULL, SE_LOCK_MEMORY_NAME, &luid))
+  if (LookupPrivilegeValue(nullptr, SE_LOCK_MEMORY_NAME, &luid))
   {
       TOKEN_PRIVILEGES tp { };
       TOKEN_PRIVILEGES prevTp { };
@@ -432,10 +432,10 @@ static void* aligned_large_pages_alloc_windows([[maybe_unused]] size_t allocSize
           // Round up size to full pages and allocate
           allocSize = (allocSize + largePageSize - 1) & ~size_t(largePageSize - 1);
           mem = VirtualAlloc(
-              NULL, allocSize, MEM_RESERVE | MEM_COMMIT | MEM_LARGE_PAGES, PAGE_READWRITE);
+              nullptr, allocSize, MEM_RESERVE | MEM_COMMIT | MEM_LARGE_PAGES, PAGE_READWRITE);
 
           // Privilege no longer needed, restore previous state
-          AdjustTokenPrivileges(hProcessToken, FALSE, &prevTp, 0, NULL, NULL);
+          AdjustTokenPrivileges(hProcessToken, FALSE, &prevTp, 0, nullptr, nullptr);
       }
   }
 
@@ -453,7 +453,7 @@ void* aligned_large_pages_alloc(size_t allocSize) {
 
   // Fall back to regular, page aligned, allocation if necessary
   if (!mem)
-      mem = VirtualAlloc(NULL, allocSize, MEM_RESERVE | MEM_COMMIT, PAGE_READWRITE);
+      mem = VirtualAlloc(nullptr, allocSize, MEM_RESERVE | MEM_COMMIT, PAGE_READWRITE);
 
   return mem;
 }

diff --git a/src/nnue/evaluate_nnue.cpp b/src/nnue/evaluate_nnue.cpp
index a06c1978517b34bede52f8fd873ee85913505b43..06281da06a5368508724ab3987b56848ee03ce94 100644 (file)
--- a/src/nnue/evaluate_nnue.cpp
+++ b/src/nnue/evaluate_nnue.cpp
@@ -26,7 +26,6 @@
 
 #include "../evaluate.h"
 #include "../position.h"
-#include "../misc.h"
 #include "../uci.h"
 #include "../types.h"
 
@@ -245,14 +244,15 @@ namespace Stockfish::Eval::NNUE {
   }
 
 
-  // format_cp_aligned_dot() converts a Value into (centi)pawns and writes it in a buffer,
-  // always keeping two decimals. The buffer must have capacity for at least 7 chars.
-  static void format_cp_aligned_dot(Value v, char* buffer) {
+  // format_cp_aligned_dot() converts a Value into (centi)pawns, always keeping two decimals.
+  static void format_cp_aligned_dot(Value v, std::stringstream &stream) {
+    const double cp = 1.0 * std::abs(int(v)) / UCI::NormalizeToPawnValue;
 
-    buffer[0] = (v < 0 ? '-' : v > 0 ? '+' : ' ');
-
-    double cp = 1.0 * std::abs(int(v)) / UCI::NormalizeToPawnValue;
-    sprintf(&buffer[1], "%6.2f", cp);
+    stream << (v < 0 ? '-' : v > 0 ? '+' : ' ')
+           << std::setiosflags(std::ios::fixed)
+           << std::setw(6)
+           << std::setprecision(2)
+           << cp;
   }
 
 
@@ -332,17 +332,10 @@ namespace Stockfish::Eval::NNUE {
 
     for (std::size_t bucket = 0; bucket < LayerStacks; ++bucket)
     {
-      char buffer[3][8];
-      std::memset(buffer, '\0', sizeof(buffer));
-
-      format_cp_aligned_dot(t.psqt[bucket], buffer[0]);
-      format_cp_aligned_dot(t.positional[bucket], buffer[1]);
-      format_cp_aligned_dot(t.psqt[bucket] + t.positional[bucket], buffer[2]);
-
-      ss <<  "|  " << bucket    << "        "
-         << " |  " << buffer[0] << "  "
-         << " |  " << buffer[1] << "  "
-         << " |  " << buffer[2] << "  "
+      ss <<  "|  " << bucket    << "        ";
+      ss << " |  "; format_cp_aligned_dot(t.psqt[bucket], ss); ss << "  "
+         << " |  "; format_cp_aligned_dot(t.positional[bucket], ss); ss << "  "
+         << " |  "; format_cp_aligned_dot(t.psqt[bucket] + t.positional[bucket], ss); ss << "  "
          << " |";
       if (bucket == t.correctBucket)
           ss << " <-- this bucket is used";

diff --git a/src/thread.cpp b/src/thread.cpp
index 7e71edf10a3037f7f342d1aafba08bfeeb2e6cc1..ca1a7c852731d11b530b10ccea2d42ea5d5573e6 100644 (file)
--- a/src/thread.cpp
+++ b/src/thread.cpp
@@ -73,9 +73,9 @@ void Thread::clear() {
 /// Thread::start_searching() wakes up the thread that will start the search
 
 void Thread::start_searching() {
-
-  std::lock_guard<std::mutex> lk(mutex);
+  mutex.lock();
   searching = true;
+  mutex.unlock(); // Unlock before notifying saves a few CPU-cycles
   cv.notify_one(); // Wake up the thread in idle_loop()
 }
 
@@ -125,20 +125,20 @@ void Thread::idle_loop() {
 
 void ThreadPool::set(size_t requested) {
 
-  if (size() > 0)   // destroy any existing thread(s)
+  if (threads.size() > 0)   // destroy any existing thread(s)
   {
       main()->wait_for_search_finished();
 
-      while (size() > 0)
-          delete back(), pop_back();
+      while (threads.size() > 0)
+          delete threads.back(), threads.pop_back();
   }
 
   if (requested > 0)   // create new thread(s)
   {
-      push_back(new MainThread(0));
+      threads.push_back(new MainThread(0));
 
-      while (size() < requested)
-          push_back(new Thread(size()));
+      while (threads.size() < requested)
+          threads.push_back(new Thread(threads.size()));
       clear();
 
       // Reallocate the hash with the new threadpool size
@@ -154,7 +154,7 @@ void ThreadPool::set(size_t requested) {
 
 void ThreadPool::clear() {
 
-  for (Thread* th : *this)
+  for (Thread* th : threads)
       th->clear();
 
   main()->callsCnt = 0;
@@ -187,7 +187,7 @@ void ThreadPool::start_thinking(Position& pos, StateListPtr& states,
       Tablebases::rank_root_moves(pos, rootMoves);
 
   // After ownership transfer 'states' becomes empty, so if we stop the search
-  // and call 'go' again without setting a new position states.get() == NULL.
+  // and call 'go' again without setting a new position states.get() == nullptr.
   assert(states.get() || setupStates.get());
 
   if (states.get())
@@ -198,7 +198,7 @@ void ThreadPool::start_thinking(Position& pos, StateListPtr& states,
   // be deduced from a fen string, so set() clears them and they are set from
   // setupStates->back() later. The rootState is per thread, earlier states are shared
   // since they are read-only.
-  for (Thread* th : *this)
+  for (Thread* th : threads)
   {
       th->nodes = th->tbHits = th->nmpMinPly = th->bestMoveChanges = 0;
       th->rootDepth = th->completedDepth = 0;
@@ -212,12 +212,12 @@ void ThreadPool::start_thinking(Position& pos, StateListPtr& states,
 
 Thread* ThreadPool::get_best_thread() const {
 
-    Thread* bestThread = front();
+    Thread* bestThread = threads.front();
     std::map<Move, int64_t> votes;
     Value minScore = VALUE_NONE;
 
     // Find minimum score of all threads
-    for (Thread* th: *this)
+    for (Thread* th: threads)
         minScore = std::min(minScore, th->rootMoves[0].score);
 
     // Vote according to score and depth, and select the best thread
@@ -225,10 +225,10 @@ Thread* ThreadPool::get_best_thread() const {
             return (th->rootMoves[0].score - minScore + 14) * int(th->completedDepth);
         };
 
-    for (Thread* th : *this)
+    for (Thread* th : threads)
         votes[th->rootMoves[0].pv[0]] += thread_value(th);
 
-    for (Thread* th : *this)
+    for (Thread* th : threads)
         if (abs(bestThread->rootMoves[0].score) >= VALUE_TB_WIN_IN_MAX_PLY)
         {
             // Make sure we pick the shortest mate / TB conversion or stave off mate the longest
@@ -251,8 +251,8 @@ Thread* ThreadPool::get_best_thread() const {
 
 void ThreadPool::start_searching() {
 
-    for (Thread* th : *this)
-        if (th != front())
+    for (Thread* th : threads)
+        if (th != threads.front())
             th->start_searching();
 }
 
@@ -261,8 +261,8 @@ void ThreadPool::start_searching() {
 
 void ThreadPool::wait_for_search_finished() const {
 
-    for (Thread* th : *this)
-        if (th != front())
+    for (Thread* th : threads)
+        if (th != threads.front())
             th->wait_for_search_finished();
 }
 

diff --git a/src/thread.h b/src/thread.h
index 680da2090d9f38b2d897652640e9491a22da04c0..7566322c54c933ec2ef2ca6369fd13cd4aa8f0c8 100644 (file)
--- a/src/thread.h
+++ b/src/thread.h
@@ -101,13 +101,13 @@ struct MainThread : public Thread {
 /// parking and, most importantly, launching a thread. All the access to threads
 /// is done through this class.
 
-struct ThreadPool : public std::vector<Thread*> {
+struct ThreadPool {
 
   void start_thinking(Position&, StateListPtr&, const Search::LimitsType&, bool = false);
   void clear();
   void set(size_t);
 
-  MainThread* main()        const { return static_cast<MainThread*>(front()); }
+  MainThread* main()        const { return static_cast<MainThread*>(threads.front()); }
   uint64_t nodes_searched() const { return accumulate(&Thread::nodes); }
   uint64_t tb_hits()        const { return accumulate(&Thread::tbHits); }
   Thread* get_best_thread() const;
@@ -116,13 +116,21 @@ struct ThreadPool : public std::vector<Thread*> {
 
   std::atomic_bool stop, increaseDepth;
 
+  auto cbegin() const noexcept { return threads.cbegin(); }
+  auto begin() noexcept { return threads.begin(); }
+  auto end() noexcept { return threads.end(); }
+  auto cend() const noexcept { return threads.cend(); }
+  auto size() const noexcept { return threads.size(); }
+  auto empty() const noexcept { return threads.empty(); }
+
 private:
   StateListPtr setupStates;
+  std::vector<Thread*> threads;
 
   uint64_t accumulate(std::atomic<uint64_t> Thread::* member) const {
 
     uint64_t sum = 0;
-    for (Thread* th : *this)
+    for (Thread* th : threads)
         sum += (th->*member).load(std::memory_order_relaxed);
     return sum;
   }

diff --git a/src/thread_win32_osx.h b/src/thread_win32_osx.h
index 01ff1c77afa07fd6de3b9323fdfd23113f551a62..330a8341dd8187596ff10164323a4a896a862d16 100644 (file)
--- a/src/thread_win32_osx.h
+++ b/src/thread_win32_osx.h
@@ -41,7 +41,7 @@ void* start_routine(void* ptr)
    P* p = reinterpret_cast<P*>(ptr);
    (p->first->*(p->second))(); // Call member function pointer
    delete p;
-   return NULL;
+   return nullptr;
 }
 
 class NativeThread {
@@ -56,7 +56,7 @@ public:
     pthread_attr_setstacksize(attr, TH_STACK_SIZE);
     pthread_create(&thread, attr, start_routine<T>, new P(obj, fun));
   }
-  void join() { pthread_join(thread, NULL); }
+  void join() { pthread_join(thread, nullptr); }
 };
 
 } // namespace Stockfish
@@ -65,7 +65,7 @@ public:
 
 namespace Stockfish {
 
-typedef std::thread NativeThread;
+using NativeThread = std::thread;
 
 } // namespace Stockfish