deprecate accumulators::thread_safe, replace with optionally thread-safe accumulators::count (#326)

Author: HDembinski

.github/workflows/fast.yml

@@ -38,17 +38,3 @@ jobs:
       run: |
         cd build
         ctest -C Debug --output-on-failure
-
-  gcc5:
-    runs-on: ubuntu-16.04
-    steps:
-    - uses: actions/checkout@v2
-    - name: cmake
-      run: |
-        mkdir build
-        cd build
-        CXX=g++-5 cmake ..
-    - name: ctest
-      run: |
-        cd build
-        ctest -C Debug --output-on-failure

.github/workflows/slow.yml

@@ -80,10 +80,10 @@ jobs:
       run: |
         ./bootstrap.sh
         ./b2 headers
-    - name: Test cxxstd=14
+    - name: Test cxxstd=14 (warnings ignored)
       run: |
         cd libs/histogram
-        ../../b2 $B2_OPTS toolset=gcc-5 cxxstd=14 test//all examples
+        ../../b2 -q -j2 toolset=gcc-5 cxxstd=14 test//all examples
 
   gcc10:
     runs-on: ubuntu-latest

doc/guide.qbk

@@ -425,7 +425,7 @@ There are two ways to generate a single histogram using several threads.
 
 1. Each thread has its own copy of the histogram. Each copy is independently filled. The copies are then added in the main thread. Use this as the default when you can afford having `N` copies of the histogram in memory for `N` threads, because it allows each thread to work on its thread-local memory and utilise the CPU cache without the need to synchronise memory access. The highest performance gains are obtained in this way.
 
-2. There is only one histogram which is filled concurrently by several threads. This requires using a thread-safe storage that can handle concurrent writes. The library provides the [classref boost::histogram::accumulators::thread_safe] accumulator, which combined with the [classref boost::histogram::dense_storage] provides a thread-safe storage.
+2. There is only one histogram which is filled concurrently by several threads. This requires using a thread-safe storage that can handle concurrent writes. The library provides the [classref boost::histogram::accumulators::count] accumulator with a thread-safe option, which combined with the [classref boost::histogram::dense_storage] provides a thread-safe storage.
 
 [note Filling a histogram with growing axes in a multi-threaded environment is safe, but has poor performance since the histogram must be locked on each fill. The locks are required because an axis could grow each time, which changes the number of cells and cell addressing for all other threads. Even without growing axes, there is only a performance gain if the histogram is either very large or when significant time is spend in preparing the value to fill. For small histograms, threads frequently access the same cell, whose state has to be synchronised between the threads. This is slow even with atomic counters and made worse by the effect of false sharing.]
 

examples/guide_parallel_filling.cpp

@@ -26,7 +26,7 @@ int main() {
     Create histogram with container of thread-safe counters for parallel filling in
     several threads. Only filling is thread-safe, other guarantees are not given.
   */
-  auto h = make_histogram_with(dense_storage<accumulators::thread_safe<unsigned>>(),
+  auto h = make_histogram_with(dense_storage<accumulators::count<unsigned, true>>(),
                                axis::integer<>(0, 10));
 
   /*

include/boost/histogram/accumulators/count.hpp

@@ -4,10 +4,11 @@
 // (See accompanying file LICENSE_1_0.txt
 // or copy at http://www.boost.org/LICENSE_1_0.txt)
 
-#ifndef BOOST_HISTOGRAM_ACCUMULATORS_NUMBER_HPP
-#define BOOST_HISTOGRAM_ACCUMULATORS_NUMBER_HPP
+#ifndef BOOST_HISTOGRAM_ACCUMULATORS_COUNT_HPP
+#define BOOST_HISTOGRAM_ACCUMULATORS_COUNT_HPP
 
 #include <boost/core/nvp.hpp>
+#include <boost/histogram/detail/atomic_number.hpp>
 #include <boost/histogram/fwd.hpp> // for count<>
 #include <type_traits>             // for std::common_type
 
@@ -16,32 +17,44 @@ namespace histogram {
 namespace accumulators {
 
 /**
-  Uses a C++ builtin arithmetic type to accumulate a count.
+  Wraps a C++ arithmetic type with optionally thread-safe increments and adds.
 
-  This wrapper class may be used as a base class by users who want to add custom metadata
-  to each bin of a histogram. Otherwise, arithmetic types should be used directly as
-  accumulators in storages for simplicity. In other words, prefer `dense_storage<double>`
-  over `dense_storage<count<double>>`, both are functionally equivalent.
+  This adaptor optionally uses atomic operations to make concurrent increments and
+  additions thread-safe for the stored arithmetic value, which can be integral or
+  floating point. For small histograms, the performance will still be poor because of
+  False Sharing, see https://en.wikipedia.org/wiki/False_sharing for details.
 
-  When weighted data is accumulated and high precision is required, use
-  `accumulators::sum` instead. If a local variance estimate for the weight distribution
-  should be computed as well (generally needed for a detailed statistical analysis), use
-  `accumulators::weighted_sum`.
+  Warning: Assignment is not thread-safe in this implementation, so don't assign
+  concurrently.
+
+  This wrapper class can be used as a base class by users to add arbitrary metadata to
+  each bin of a histogram.
+
+  When weighted samples are accumulated and high precision is required, use
+  `accumulators::sum` instead (at the cost of lower performance). If a local variance
+  estimate for the weight distribution should be computed as well (generally needed for a
+  detailed statistical analysis), use `accumulators::weighted_sum`.
+
+  @tparam T C++ builtin arithmetic type (integer or floating point).
+  @tparam ThreadSafe Set to true to make increments and adds thread-safe.
 */
-template <class ValueType>
+template <class ValueType, bool ThreadSafe>
 class count {
+  using internal_type =
+      std::conditional_t<ThreadSafe, detail::atomic_number<ValueType>, ValueType>;
+
 public:
   using value_type = ValueType;
   using const_reference = const value_type&;
 
-  count() = default;
+  count() noexcept = default;
 
   /// Initialize count to value and allow implicit conversion
-  count(const_reference value) noexcept : value_(value) {}
+  count(const_reference value) noexcept : value_{value} {}
 
   /// Allow implicit conversion from other count
-  template <class T>
-  count(const count<T>& c) noexcept : count(c.value()) {}
+  template <class T, bool B>
+  count(const count<T, B>& c) noexcept : count{c.value()} {}
 
   /// Increment count by one
   count& operator++() noexcept {
@@ -72,16 +85,20 @@ class count {
   bool operator!=(const count& rhs) const noexcept { return !operator==(rhs); }
 
   /// Return count
-  const_reference value() const noexcept { return value_; }
+  value_type value() const noexcept { return value_; }
 
   // conversion to value_type must be explicit
   explicit operator value_type() const noexcept { return value_; }
 
   template <class Archive>
   void serialize(Archive& ar, unsigned /* version */) {
-    ar& make_nvp("value", value_);
+    auto v = value();
+    ar& make_nvp("value", v);
+    value_ = v;
   }
 
+  static constexpr bool thread_safe() noexcept { return ThreadSafe; }
+
   // begin: extra operators to make count behave like a regular number
 
   count& operator*=(const count& rhs) noexcept {
@@ -114,10 +131,33 @@ class count {
 
   bool operator>=(const count& rhs) const noexcept { return value_ >= rhs.value_; }
 
+  friend bool operator==(const_reference x, const count& rhs) noexcept {
+    return x == rhs.value_;
+  }
+
+  friend bool operator!=(const_reference x, const count& rhs) noexcept {
+    return x != rhs.value_;
+  }
+
+  friend bool operator<(const_reference x, const count& rhs) noexcept {
+    return x < rhs.value_;
+  }
+
+  friend bool operator>(const_reference x, const count& rhs) noexcept {
+    return x > rhs.value_;
+  }
+
+  friend bool operator<=(const_reference x, const count& rhs) noexcept {
+    return x <= rhs.value_;
+  }
+  friend bool operator>=(const_reference x, const count& rhs) noexcept {
+    return x >= rhs.value_;
+  }
+
   // end: extra operators
 
 private:
-  value_type value_{};
+  internal_type value_{};
 };
 
 } // namespace accumulators
@@ -126,10 +166,10 @@ class count {
 
 #ifndef BOOST_HISTOGRAM_DOXYGEN_INVOKED
 namespace std {
-template <class T, class U>
-struct common_type<boost::histogram::accumulators::count<T>,
-                   boost::histogram::accumulators::count<U>> {
-  using type = boost::histogram::accumulators::count<common_type_t<T, U>>;
+template <class T, class U, bool B1, bool B2>
+struct common_type<boost::histogram::accumulators::count<T, B1>,
+                   boost::histogram::accumulators::count<U, B2>> {
+  using type = boost::histogram::accumulators::count<common_type_t<T, U>, (B1 || B2)>;
 };
 } // namespace std
 #endif

include/boost/histogram/accumulators/is_thread_safe.hpp

@@ -0,0 +1,41 @@
+// Copyright 2021 Hans Dembinski
+//
+// Distributed under the Boost Software License, version 1.0.
+// (See accompanying file LICENSE_1_0.txt
+// or copy at http://www.boost.org/LICENSE_1_0.txt)
+
+#ifndef BOOST_HISTOGRAM_ACCUMULATORS_IS_THREAD_SAFE_HPP
+#define BOOST_HISTOGRAM_ACCUMULATORS_IS_THREAD_SAFE_HPP
+
+#include <boost/histogram/detail/priority.hpp>
+#include <boost/histogram/fwd.hpp>
+#include <type_traits>
+
+namespace boost {
+namespace histogram {
+namespace detail {
+
+template <class T>
+constexpr bool is_thread_safe_impl(priority<0>) {
+  return false;
+}
+
+template <class T>
+constexpr auto is_thread_safe_impl(priority<1>) -> decltype(T::thread_safe()) {
+  return T::thread_safe();
+}
+
+} // namespace detail
+
+namespace accumulators {
+
+template <class T>
+struct is_thread_safe
+    : std::integral_constant<bool,
+                             detail::is_thread_safe_impl<T>(detail::priority<1>{})> {};
+
+} // namespace accumulators
+} // namespace histogram
+} // namespace boost
+
+#endif

include/boost/histogram/accumulators/ostream.hpp

@@ -54,9 +54,9 @@ std::basic_ostream<CharT, Traits>& handle_nonzero_width(
 
 namespace accumulators {
 
-template <class CharT, class Traits, class U>
+template <class CharT, class Traits, class U, bool B>
 std::basic_ostream<CharT, Traits>& operator<<(std::basic_ostream<CharT, Traits>& os,
-                                              const count<U>& x) {
+                                              const count<U, B>& x) {
   return os << x.value();
 }
 
@@ -92,12 +92,15 @@ std::basic_ostream<CharT, Traits>& operator<<(std::basic_ostream<CharT, Traits>&
   return detail::handle_nonzero_width(os, x);
 }
 
+#include <boost/histogram/detail/ignore_deprecation_warning_begin.hpp>
 template <class CharT, class Traits, class T>
 std::basic_ostream<CharT, Traits>& operator<<(std::basic_ostream<CharT, Traits>& os,
                                               const thread_safe<T>& x) {
   os << static_cast<T>(x);
   return os;
 }
+#include <boost/histogram/detail/ignore_deprecation_warning_end.hpp>
+
 } // namespace accumulators
 } // namespace histogram
 } // namespace boost