Repository: randombit/botan
Pull request #2113 (2019-09-24 03:05:27)

Add a RWLock
Reduces test pervf somewhat vs using TLS but it eliminates overhead giving a 3-5% improvement for AES with AES-NI


Patch:
diff --git a/src/lib/utils/cpuid/cpuid.cpp b/src/lib/utils/cpuid/cpuid.cpp
index 18ccff419..81fabf0a3 100644
--- a/src/lib/utils/cpuid/cpuid.cpp
+++ b/src/lib/utils/cpuid/cpuid.cpp
@@ -13,13 +13,6 @@
 
 namespace Botan {
 
-//static
-CPUID::CPUID_Data& CPUID::state()
-   {
-   static BOTAN_THREAD_LOCAL CPUID::CPUID_Data g_cpuid;
-   return g_cpuid;
-   }
-
 bool CPUID::has_simd_32()
    {
 #if defined(BOTAN_TARGET_SUPPORTS_SSE2)
diff --git a/src/lib/utils/cpuid/cpuid.h b/src/lib/utils/cpuid/cpuid.h
index bd8c8d95f..7b6b8ebc3 100644
--- a/src/lib/utils/cpuid/cpuid.h
+++ b/src/lib/utils/cpuid/cpuid.h
@@ -401,7 +401,11 @@ class BOTAN_PUBLIC_API(2,1) CPUID final
             Endian_Status m_endian_status;
          };
 
-      static CPUID_Data& state();
+      static CPUID_Data& state()
+         {
+         static CPUID::CPUID_Data g_cpuid;
+         return g_cpuid;
+         }
    };
 
 }
diff --git a/src/lib/utils/thread_utils/info.txt b/src/lib/utils/thread_utils/info.txt
index 80ce2d389..057f53600 100644
--- a/src/lib/utils/thread_utils/info.txt
+++ b/src/lib/utils/thread_utils/info.txt
@@ -1,8 +1,9 @@
 <defines>
-THREAD_UTILS -> 20190122
+THREAD_UTILS -> 20190922
 </defines>
 
 <header:internal>
+rwlock.h
 barrier.h
 semaphore.h
 thread_pool.h
diff --git a/src/lib/utils/thread_utils/rwlock.cpp b/src/lib/utils/thread_utils/rwlock.cpp
new file mode 100644
index 000000000..58500a83d
--- /dev/null
+++ b/src/lib/utils/thread_utils/rwlock.cpp
@@ -0,0 +1,58 @@
+/*
+* (C) 2019 Jack Lloyd
+*
+* Botan is released under the Simplified BSD License (see license.txt)
+*/
+
+#include <botan/internal/rwlock.h>
+
+namespace Botan {
+
+RWLock::RWLock() : m_state(0) {}
+
+void RWLock::lock()
+   {
+   std::unique_lock<std::mutex> lock(m_mutex);
+   while(m_state & is_writing)
+      m_gate1.wait(lock);
+   m_state |= is_writing;
+   while(m_state & readers_mask)
+      m_gate2.wait(lock);
+   }
+
+void RWLock::unlock()
+   {
+   std::unique_lock<std::mutex> lock(m_mutex);
+   m_state = 0;
+   m_gate1.notify_all();
+   }
+
+void RWLock::lock_shared()
+   {
+   std::unique_lock<std::mutex> lock(m_mutex);
+   while((m_state & is_writing) || (m_state & readers_mask) == readers_mask)
+      m_gate1.wait(lock);
+   const uint32_t num_readers = (m_state & readers_mask) + 1;
+   m_state &= ~readers_mask;
+   m_state |= num_readers;
+   }
+
+void RWLock::unlock_shared()
+   {
+   std::unique_lock<std::mutex> lock(m_mutex);
+   const uint32_t num_readers = (m_state & readers_mask) - 1;
+   m_state &= ~readers_mask;
+   m_state |= num_readers;
+   if(m_state & is_writing)
+      {
+      if(num_readers == 0)
+         m_gate2.notify_one();
+      }
+   else
+      {
+      if(num_readers == readers_mask - 1)
+         m_gate1.notify_one();
+      }
+   }
+
+}
diff --git a/src/lib/utils/thread_utils/rwlock.h b/src/lib/utils/thread_utils/rwlock.h
new file mode 100644
index 000000000..324e66753
--- /dev/null
+++ b/src/lib/utils/thread_utils/rwlock.h
@@ -0,0 +1,42 @@
+/*
+* (C) 2019 Jack Lloyd
+*
+* Botan is released under the Simplified BSD License (see license.txt)
+*/
+
+#ifndef BOTAN_RWLOCK_H_
+#define BOTAN_RWLOCK_H_
+
+#include <botan/types.h>
+#include <mutex>
+#include <condition_variable>
+
+namespace Botan {
+
+/**
+* A read-write lock. Writers are favored.
+*/
+class BOTAN_TEST_API RWLock final
+   {
+   public:
+      RWLock();
+
+      void lock();
+      void unlock();
+
+      void lock_shared();
+      void unlock_shared();
+   private:
+      std::mutex m_mutex;
+      std::condition_variable m_gate1;
+      std::condition_variable m_gate2;
+      uint32_t m_state;
+
+      // 2**31 concurrent readers should be enough for anyone
+      static const uint32_t is_writing = static_cast<uint32_t>(1) << 31;
+      static const uint32_t readers_mask = ~is_writing;
+   };
+
+}
+
+#endif


Test patch:
diff --git a/src/tests/test_runner.cpp b/src/tests/test_runner.cpp
index fdc705216..cd10cb874 100644
--- a/src/tests/test_runner.cpp
+++ b/src/tests/test_runner.cpp
@@ -14,6 +14,7 @@
 
 #if defined(BOTAN_HAS_THREAD_UTILS)
    #include <botan/internal/thread_pool.h>
+   #include <botan/internal/rwlock.h>
 #endif
 
 namespace Botan_Tests {
@@ -106,8 +107,8 @@ int Test_Runner::run(const Test_Options& opts)
       */
 
       std::vector<std::string> default_first = {
-         "block", "stream", "hash", "mac", "modes", "aead",
-         "kdf", "pbkdf", "hmac_drbg", "util"
+         "block", "stream", "hash", "mac", "aead",
+         "modes", "kdf", "pbkdf", "hmac_drbg", "util"
       };
 
       for(auto s : default_first)
@@ -298,6 +299,15 @@ std::string test_summary(size_t test_run, size_t tot_test_runs, uint64_t total_n
    return oss.str();
    }
 
+bool needs_serialization(const std::string& test_name)
+   {
+   if(test_name.substr(0, 6) == "pkcs11")
+      return true;
+   if(test_name == "block" || test_name == "hash" || test_name == "mac" || test_name == "stream" || test_name == "aead")
+      return true;
+   return false;
+   }
+
 }
 
 size_t Test_Runner::run_tests(const std::vector<std::string>& tests_to_run,
@@ -313,20 +323,33 @@ size_t Test_Runner::run_tests(const std::vector<std::string>& tests_to_run,
       {
       // If 0 then we let thread pool select the count
       Botan::Thread_Pool pool(test_threads);
+      Botan::RWLock rwlock;
 
       std::vector<std::future<std::vector<Test::Result>>> m_fut_results;
 
+      auto run_test_exclusive = [&](const std::string& test_name) {
+         rwlock.lock();
+         std::vector<Test::Result> results = run_a_test(test_name);
+         rwlock.unlock();
+         return results;
+      };
+
+      auto run_test_shared = [&](const std::string& test_name) {
+         rwlock.lock_shared();
+         std::vector<Test::Result> results = run_a_test(test_name);
+         rwlock.unlock_shared();
+         return results;
+      };
+
       for(auto const& test_name : tests_to_run)
          {
-         if(test_name.substr(0, 6) == "pkcs11")
+         if(needs_serialization(test_name))
             {
-            // Run all PKCS11 tests on the main thread because they need to
-            // be serialized with respect to each other.
-            m_fut_results.push_back(std::async(std::launch::deferred, run_a_test, test_name));
+            m_fut_results.push_back(pool.run(run_test_exclusive, test_name));
             }
          else
             {
-            m_fut_results.push_back(pool.run(run_a_test, test_name));
+            m_fut_results.push_back(pool.run(run_test_shared, test_name));
             }
          }
 
diff --git a/src/tests/test_runner.h b/src/tests/test_runner.h
index 47d4f1363..9f67a273f 100644
--- a/src/tests/test_runner.h
+++ b/src/tests/test_runner.h
@@ -10,6 +10,7 @@
 #include <iosfwd>
 #include <string>
 #include <vector>
+#include <memory>
 
 namespace Botan_Tests {