From ec871a9a585a34cabc831c33cff4201d62a35ef7 Mon Sep 17 00:00:00 2001 From: David Wells Date: Sat, 8 Apr 2017 00:54:42 -0400 Subject: [PATCH] Prefer std::atomic to volatile for thread data. --- include/deal.II/base/config.h.in | 3 +++ include/deal.II/base/subscriptor.h | 13 +++++++------ source/base/subscriptor.cc | 19 ------------------- source/base/thread_management.cc | 11 ++--------- tests/base/thread_local_storage_01.cc | 3 ++- tests/base/thread_local_storage_02.cc | 3 ++- tests/base/thread_validity_08.cc | 3 ++- tests/base/thread_validity_09.cc | 3 ++- 8 files changed, 20 insertions(+), 38 deletions(-) diff --git a/include/deal.II/base/config.h.in b/include/deal.II/base/config.h.in index fec20fbfb5..c11c5627e3 100644 --- a/include/deal.II/base/config.h.in +++ b/include/deal.II/base/config.h.in @@ -156,6 +156,9 @@ * Depending on the use of threads, we will have to make some variables * volatile. We do this here in a very old-fashioned C-style, but still * convenient way. + * + * @deprecated This macro is deprecated in favor of using the + * std::atomic template class. */ #ifdef DEAL_II_WITH_THREADS # define DEAL_VOLATILE volatile diff --git a/include/deal.II/base/subscriptor.h b/include/deal.II/base/subscriptor.h index f826949f8c..37f3b1acad 100644 --- a/include/deal.II/base/subscriptor.h +++ b/include/deal.II/base/subscriptor.h @@ -20,9 +20,10 @@ #include #include -#include +#include #include #include +#include DEAL_II_NAMESPACE_OPEN @@ -191,12 +192,12 @@ private: * We use the mutable keyword in order to allow subscription to * constant objects also. * - * In multithreaded mode, this counter may be modified by different threads. - * We thus have to mark it volatile. However, this is counter- - * productive in non-MT mode since it may pessimize code. So use the macro - * defined in deal.II/base/config.h to selectively add volatility. + * This counter may be read from and written to concurrently in + * multithreaded code: hence we use the std::atomic class + * template. + * */ - mutable DEAL_VOLATILE unsigned int counter; + mutable std::atomic counter; /** * In this map, we count subscriptions for each different identification diff --git a/source/base/subscriptor.cc b/source/base/subscriptor.cc index e249163e80..ce05a88509 100644 --- a/source/base/subscriptor.cc +++ b/source/base/subscriptor.cc @@ -24,23 +24,6 @@ DEAL_II_NAMESPACE_OPEN -#ifdef DEBUG -namespace -{ -// create a lock that might be used to control subscription to and -// unsubscription from objects, as that might happen in parallel. -// since it should happen rather seldom that several threads try to -// operate on different objects at the same time (the usual case is -// that they subscribe to the same object right after thread -// creation), a global lock should be sufficient, rather than one that -// operates on a per-object base (in which case we would have to -// include the huge file into the -// file). - Threads::Mutex subscription_lock; -} -#endif - - static const char *unknown_subscriber = "unknown subscriber"; @@ -167,7 +150,6 @@ Subscriptor::subscribe(const char *id) const #ifdef DEBUG if (object_info == nullptr) object_info = &typeid(*this); - Threads::Mutex::ScopedLock lock (subscription_lock); ++counter; # ifndef DEAL_II_WITH_THREADS @@ -199,7 +181,6 @@ Subscriptor::unsubscribe(const char *id) const if (counter == 0) return; - Threads::Mutex::ScopedLock lock (subscription_lock); --counter; # ifndef DEAL_II_WITH_THREADS diff --git a/source/base/thread_management.cc b/source/base/thread_management.cc index f60db61786..0c43b4be1a 100644 --- a/source/base/thread_management.cc +++ b/source/base/thread_management.cc @@ -15,10 +15,9 @@ #include -#include +#include #include #include -#include #ifdef DEAL_II_HAVE_UNISTD_H # include @@ -32,15 +31,11 @@ namespace Threads { namespace internal { - // counter and access mutex for the - // number of threads - volatile unsigned int n_existing_threads_counter = 1; - Mutex n_existing_threads_mutex; + static std::atomic n_existing_threads_counter(1); void register_thread () { - Mutex::ScopedLock lock (n_existing_threads_mutex); ++n_existing_threads_counter; } @@ -48,7 +43,6 @@ namespace Threads void deregister_thread () { - Mutex::ScopedLock lock (n_existing_threads_mutex); --n_existing_threads_counter; Assert (n_existing_threads_counter >= 1, ExcInternalError()); @@ -128,7 +122,6 @@ namespace Threads unsigned int n_existing_threads () { - Mutex::ScopedLock lock (internal::n_existing_threads_mutex); return internal::n_existing_threads_counter; } diff --git a/tests/base/thread_local_storage_01.cc b/tests/base/thread_local_storage_01.cc index 70fcdd706a..017af3acc3 100644 --- a/tests/base/thread_local_storage_01.cc +++ b/tests/base/thread_local_storage_01.cc @@ -17,6 +17,7 @@ // verify that thread local storage works as advertised #include "../tests.h" +#include #include #include @@ -39,7 +40,7 @@ struct X Threads::ThreadLocalStorage *tls_data; -volatile int counter = 0; +static std::atomic counter(0); void execute (int i) { diff --git a/tests/base/thread_local_storage_02.cc b/tests/base/thread_local_storage_02.cc index 683e164921..d57e520426 100644 --- a/tests/base/thread_local_storage_02.cc +++ b/tests/base/thread_local_storage_02.cc @@ -18,6 +18,7 @@ // the initialization with an exemplar #include "../tests.h" +#include #include #include @@ -49,7 +50,7 @@ struct X Threads::ThreadLocalStorage *tls_data; -volatile int counter = 0; +static std::atomic counter(0); void execute (int i) { diff --git a/tests/base/thread_validity_08.cc b/tests/base/thread_validity_08.cc index 7581aec84e..8cdbfcd2bf 100644 --- a/tests/base/thread_validity_08.cc +++ b/tests/base/thread_validity_08.cc @@ -17,6 +17,7 @@ // see if we can detach from threads #include "../tests.h" +#include #include #include #include @@ -25,7 +26,7 @@ Threads::Mutex mutex; -volatile int spin_lock = 0; +static std::atomic spin_lock(0); void worker () diff --git a/tests/base/thread_validity_09.cc b/tests/base/thread_validity_09.cc index 0ba39f6d64..e21fa0c049 100644 --- a/tests/base/thread_validity_09.cc +++ b/tests/base/thread_validity_09.cc @@ -20,6 +20,7 @@ // and makes sure nobody writes into it at undue times #include "../tests.h" +#include #include #include #include @@ -28,7 +29,7 @@ Threads::Mutex mutex; -volatile int spin_lock = 0; +static std::atomic spin_lock(0); int worker () -- 2.39.5