From 4eff6567db68ed2b0fa681a551f7d5288c2a3454 Mon Sep 17 00:00:00 2001 From: Jonas Hahnfeld Date: Thu, 27 Nov 2025 15:52:42 +0100 Subject: [PATCH] [hist] Implement initial RHistConcurrentFiller In order to support efficient concurrent filling of RHist with global histogram statistics, we keep one local RHistStats object per RHistFillContext. --- hist/histv7/doc/CodeArchitecture.md | 11 ++ hist/histv7/doc/ConcurrentFilling.md | 2 +- hist/histv7/headers.cmake | 2 + hist/histv7/inc/ROOT/RHist.hxx | 6 + .../histv7/inc/ROOT/RHistConcurrentFiller.hxx | 96 +++++++++++ hist/histv7/inc/ROOT/RHistFillContext.hxx | 116 +++++++++++++ hist/histv7/test/CMakeLists.txt | 1 + hist/histv7/test/hist_concurrent.cxx | 160 ++++++++++++++++++ hist/histv7/test/hist_test.hxx | 3 + 9 files changed, 396 insertions(+), 1 deletion(-) create mode 100644 hist/histv7/inc/ROOT/RHistConcurrentFiller.hxx create mode 100644 hist/histv7/inc/ROOT/RHistFillContext.hxx create mode 100644 hist/histv7/test/hist_concurrent.cxx diff --git a/hist/histv7/doc/CodeArchitecture.md b/hist/histv7/doc/CodeArchitecture.md index 94b2d6fb10bf3..470c91a0bcd8e 100644 --- a/hist/histv7/doc/CodeArchitecture.md +++ b/hist/histv7/doc/CodeArchitecture.md @@ -62,6 +62,17 @@ It can be used as a template argument to `RHistEngine` and `RHist`. A wrapper `struct` for a single `double` value, used for weighted filling to distinguish its type. Objects of this type are passed by value. +## Classes for Concurrent Filling + +### `RHistConcurrentFiller` + +A class to orchestrate concurrent filling of `RHist` by creating (multiple) fill contexts. + +### `RHistFillContext` + +Parallel user code uses contexts to fill `RHist`s concurrently. +Each instance has a local `RHistStats` object to avoid contention on the global histogram statistics. + ## Auxiliary Classes ### `RBinIndex` diff --git a/hist/histv7/doc/ConcurrentFilling.md b/hist/histv7/doc/ConcurrentFilling.md index f6c219968f2a6..6b30c32a92494 100644 --- a/hist/histv7/doc/ConcurrentFilling.md +++ b/hist/histv7/doc/ConcurrentFilling.md @@ -46,4 +46,4 @@ For large histograms and reasonable data, contention on individual bins is expec On the other hand, updates of the (global) histogram statistics (`RHistStats`) can easily lead to contention. For this reason, `RHist` does **not** offer a `FillAtomic` method because it cannot be implemented efficiently. Instead, the user has to create a `RHistConcurrentFiller` and (potentially many) `RHistFillContext`s. -These will work together to accumulate the (global) histogram statistics during concurrent filling. +These work together to accumulate the (global) histogram statistics during concurrent filling. diff --git a/hist/histv7/headers.cmake b/hist/histv7/headers.cmake index 2ef345b3ac53c..9fa31fd8f4fd9 100644 --- a/hist/histv7/headers.cmake +++ b/hist/histv7/headers.cmake @@ -6,7 +6,9 @@ set(histv7_headers ROOT/RCategoricalAxis.hxx ROOT/RHist.hxx ROOT/RHistAutoAxisFiller.hxx + ROOT/RHistConcurrentFiller.hxx ROOT/RHistEngine.hxx + ROOT/RHistFillContext.hxx ROOT/RHistStats.hxx ROOT/RHistUtils.hxx ROOT/RLinearizedIndex.hxx diff --git a/hist/histv7/inc/ROOT/RHist.hxx b/hist/histv7/inc/ROOT/RHist.hxx index 0ffed04142115..84de3c8a33e38 100644 --- a/hist/histv7/inc/ROOT/RHist.hxx +++ b/hist/histv7/inc/ROOT/RHist.hxx @@ -24,6 +24,10 @@ class TBuffer; namespace ROOT { namespace Experimental { +// forward declaration for friend declaration +template +class RHistFillContext; + /** A histogram for aggregation of data along multiple dimensions. @@ -55,6 +59,8 @@ Feedback is welcome! */ template class RHist final { + friend class RHistFillContext; + /// The histogram engine including the bin contents. RHistEngine fEngine; /// The global histogram statistics. diff --git a/hist/histv7/inc/ROOT/RHistConcurrentFiller.hxx b/hist/histv7/inc/ROOT/RHistConcurrentFiller.hxx new file mode 100644 index 0000000000000..a9131fb0f4590 --- /dev/null +++ b/hist/histv7/inc/ROOT/RHistConcurrentFiller.hxx @@ -0,0 +1,96 @@ +/// \file +/// \warning This is part of the %ROOT 7 prototype! It will change without notice. It might trigger earthquakes. +/// Feedback is welcome! + +#ifndef ROOT_RHistConcurrentFiller +#define ROOT_RHistConcurrentFiller + +#include "RHist.hxx" +#include "RHistEngine.hxx" +#include "RHistFillContext.hxx" +#include "RWeight.hxx" + +#include +#include +#include +#include +#include + +namespace ROOT { +namespace Experimental { + +/** +A histogram filler to concurrently fill an RHist. + +\code +auto hist = std::make_shared>(10, std::make_pair(5, 15)); +{ + ROOT::Experimental::RHistConcurrentFiller filler(hist); + auto context = filler.CreateFillContext(); + context.Fill(8.5); +} +// hist->GetBinContent(ROOT::Experimental::RBinIndex(3)) will return 1 +\endcode + +\warning This is part of the %ROOT 7 prototype! It will change without notice. It might trigger earthquakes. +Feedback is welcome! +*/ +template +class RHistConcurrentFiller final { + /// A pointer to the filled histogram + std::shared_ptr> fHist; + + /// Mutex to protect access to the list of fill contexts (not for filling itself!) + std::mutex fMutex; + /// The list of fill contexts, for checks during destruction + std::vector>> fFillContexts; + +public: + /// Create a filler object. + /// + /// \param[in] hist a pointer to the histogram + explicit RHistConcurrentFiller(std::shared_ptr> hist) : fHist(hist) + { + if (!hist) { + throw std::invalid_argument("hist must not be nullptr"); + } + } + + RHistConcurrentFiller(const RHistConcurrentFiller &) = delete; + RHistConcurrentFiller(RHistConcurrentFiller &&) = delete; + RHistConcurrentFiller &operator=(const RHistConcurrentFiller &) = delete; + RHistConcurrentFiller &operator=(RHistConcurrentFiller &&) = delete; + + ~RHistConcurrentFiller() + { + for (const auto &context : fFillContexts) { + if (!context.expired()) { + // According to C++ Core Guideline C.36 "A destructor must not fail" and (C.37) "If a destructor tries to + // exit with an exception, it’s a bad design error and the program had better terminate". + std::terminate(); // GCOVR_EXCL_LINE + } + } + } + + const std::shared_ptr> &GetHist() const { return fHist; } + + /// Create a new context for concurrent filling. + std::shared_ptr> CreateFillContext() + { + // Cannot use std::make_shared because the constructor of RHistFillContext is private. Also it would mean that the + // (direct) memory of all contexts stays around until the vector of weak_ptr's is cleared. + std::shared_ptr> context(new RHistFillContext(*fHist)); + + { + std::lock_guard g(fMutex); + fFillContexts.push_back(context); + } + + return context; + } +}; + +} // namespace Experimental +} // namespace ROOT + +#endif diff --git a/hist/histv7/inc/ROOT/RHistFillContext.hxx b/hist/histv7/inc/ROOT/RHistFillContext.hxx new file mode 100644 index 0000000000000..44e184d34854c --- /dev/null +++ b/hist/histv7/inc/ROOT/RHistFillContext.hxx @@ -0,0 +1,116 @@ +/// \file +/// \warning This is part of the %ROOT 7 prototype! It will change without notice. It might trigger earthquakes. +/// Feedback is welcome! + +#ifndef ROOT_RHistFillContext +#define ROOT_RHistFillContext + +#include "RHist.hxx" +#include "RHistEngine.hxx" +#include "RHistStats.hxx" + +namespace ROOT { +namespace Experimental { + +// forward declaration for friend declaration +template +class RHistConcurrentFiller; + +/** +A context to concurrently fill an RHist. + +\sa RHistConcurrentFiller + +\warning This is part of the %ROOT 7 prototype! It will change without notice. It might trigger earthquakes. +Feedback is welcome! +*/ +template +class RHistFillContext final { + friend class RHistConcurrentFiller; + +private: + /// A pointer to the filled histogram + RHist *fHist = nullptr; + + /// Local histogram statistics + RHistStats fStats; + + /// \sa RHistConcurrentFiller::CreateFillContent() + explicit RHistFillContext(RHist &hist) : fHist(&hist), fStats(hist.GetNDimensions()) {} + RHistFillContext(const RHistFillContext &) = delete; + RHistFillContext(RHistFillContext &&) = default; + RHistFillContext &operator=(const RHistFillContext &) = delete; + RHistFillContext &operator=(RHistFillContext &&) = default; + +public: + ~RHistFillContext() { Flush(); } + + /// Fill an entry into the histogram. + /// + /// If one of the arguments is outside the corresponding axis and flow bins are disabled, the entry will be silently + /// discarded. + /// + /// Throws an exception if the number of arguments does not match the axis configuration, or if an argument cannot be + /// converted for the axis type at run-time. + /// + /// \param[in] args the arguments for each axis + /// \sa RHist::Fill(const std::tuple &args) + template + void Fill(const std::tuple &args) + { + fHist->fEngine.FillAtomic(args); + fStats.Fill(args); + } + + /// Fill an entry into the histogram with a weight. + /// + /// This overload is not available for integral bin content types (see \ref RHistEngine::SupportsWeightedFilling). + /// + /// If one of the arguments is outside the corresponding axis and flow bins are disabled, the entry will be silently + /// discarded. + /// + /// Throws an exception if the number of arguments does not match the axis configuration, or if an argument cannot be + /// converted for the axis type at run-time. + /// + /// \param[in] args the arguments for each axis + /// \param[in] weight the weight for this entry + /// \sa RHist::Fill(const std::tuple &args, RWeight weight) + template + void Fill(const std::tuple &args, RWeight weight) + { + fHist->fEngine.FillAtomic(args, weight); + fStats.Fill(args, weight); + } + + /// Fill an entry into the histogram. + /// + /// For weighted filling, pass an RWeight as the last argument. This is not available for integral bin content types + /// (see \ref RHistEngine::SupportsWeightedFilling). + /// + /// If one of the arguments is outside the corresponding axis and flow bins are disabled, the entry will be silently + /// discarded. + /// + /// Throws an exception if the number of arguments does not match the axis configuration, or if an argument cannot be + /// converted for the axis type at run-time. + /// + /// \param[in] args the arguments for each axis + /// \sa RHist::Fill(const A &...args) + template + void Fill(const A &...args) + { + fHist->fEngine.FillAtomic(args...); + fStats.Fill(args...); + } + + /// Flush locally accumulated entries to the histogram. + void Flush() + { + fHist->fStats.AddAtomic(fStats); + fStats.Clear(); + } +}; + +} // namespace Experimental +} // namespace ROOT + +#endif diff --git a/hist/histv7/test/CMakeLists.txt b/hist/histv7/test/CMakeLists.txt index f4da79debb05f..2bd47f9eac7fd 100644 --- a/hist/histv7/test/CMakeLists.txt +++ b/hist/histv7/test/CMakeLists.txt @@ -2,6 +2,7 @@ HIST_ADD_GTEST(hist_atomic hist_atomic.cxx) HIST_ADD_GTEST(hist_auto hist_auto.cxx) HIST_ADD_GTEST(hist_axes hist_axes.cxx) HIST_ADD_GTEST(hist_categorical hist_categorical.cxx) +HIST_ADD_GTEST(hist_concurrent hist_concurrent.cxx) HIST_ADD_GTEST(hist_engine hist_engine.cxx) HIST_ADD_GTEST(hist_engine_atomic hist_engine_atomic.cxx) HIST_ADD_GTEST(hist_hist hist_hist.cxx) diff --git a/hist/histv7/test/hist_concurrent.cxx b/hist/histv7/test/hist_concurrent.cxx new file mode 100644 index 0000000000000..71e0995ddec68 --- /dev/null +++ b/hist/histv7/test/hist_concurrent.cxx @@ -0,0 +1,160 @@ +#include "hist_test.hxx" + +#include +#include +#include + +TEST(RHistConcurrentFiller, Constructor) +{ + static constexpr std::size_t Bins = 20; + auto hist = std::make_shared>(Bins, std::make_pair(0, Bins)); + RHistConcurrentFiller filler(hist); + + std::shared_ptr> histPtr = filler.GetHist(); + EXPECT_EQ(hist, histPtr); + + auto context = filler.CreateFillContext(); + context->Flush(); + + EXPECT_THROW(RHistConcurrentFiller(nullptr), std::invalid_argument); +} + +TEST(RHistConcurrentFiller, OldEntries) +{ + static constexpr std::size_t Bins = 20; + auto hist = std::make_shared>(Bins, std::make_pair(0, Bins)); + hist->Fill(8.5); + ASSERT_EQ(hist->GetNEntries(), 1); + ASSERT_EQ(hist->GetBinContent(8), 1); + + { + RHistConcurrentFiller filler(hist); + auto context = filler.CreateFillContext(); + context->Flush(); + } + + EXPECT_EQ(hist->GetNEntries(), 1); + EXPECT_EQ(hist->GetBinContent(8), 1); +} + +TEST(RHistFillContext, Fill) +{ + static constexpr std::size_t Bins = 20; + auto hist = std::make_shared>(Bins, std::make_pair(0, Bins)); + + { + RHistConcurrentFiller filler(hist); + auto context = filler.CreateFillContext(); + context->Fill(8.5); + context->Fill(std::make_tuple(9.5)); + } + + EXPECT_EQ(hist->GetBinContent(RBinIndex(8)), 1); + std::array indices = {9}; + EXPECT_EQ(hist->GetBinContent(indices), 1); + + EXPECT_EQ(hist->GetNEntries(), 2); + EXPECT_FLOAT_EQ(hist->ComputeNEffectiveEntries(), 2); + EXPECT_FLOAT_EQ(hist->ComputeMean(), 9); + EXPECT_FLOAT_EQ(hist->ComputeStdDev(), 0.5); +} + +TEST(RHistFillContext, StressFill) +{ + static constexpr std::size_t NThreads = 4; + static constexpr std::size_t NFillsPerThread = 10000; + static constexpr std::size_t FlushEveryNFills = 500; + static constexpr std::size_t NFills = NThreads * NFillsPerThread; + + // Fill a single bin, to maximize contention. + auto hist = std::make_shared>(1, std::make_pair(0, 1)); + { + RHistConcurrentFiller filler(hist); + StressInParallel(NThreads, [&] { + auto context = filler.CreateFillContext(); + for (std::size_t i = 0; i < NFillsPerThread; i++) { + context->Fill(0.5); + if (i % FlushEveryNFills == 0) { + context->Flush(); + } + } + }); + } + + EXPECT_EQ(hist->GetBinContent(0), NFills); + EXPECT_EQ(hist->GetNEntries(), NFills); + EXPECT_FLOAT_EQ(hist->ComputeNEffectiveEntries(), NFills); + EXPECT_FLOAT_EQ(hist->ComputeMean(), 0.5); +} + +TEST(RHistFillContext, FillWeight) +{ + static constexpr std::size_t Bins = 20; + auto hist = std::make_shared>(Bins, std::make_pair(0, Bins)); + + { + RHistConcurrentFiller filler(hist); + auto context = filler.CreateFillContext(); + context->Fill(8.5, RWeight(0.8)); + context->Fill(std::make_tuple(9.5), RWeight(0.9)); + } + + EXPECT_FLOAT_EQ(hist->GetBinContent(RBinIndex(8)), 0.8); + std::array indices = {9}; + EXPECT_FLOAT_EQ(hist->GetBinContent(indices), 0.9); + + EXPECT_EQ(hist->GetNEntries(), 2); + EXPECT_FLOAT_EQ(hist->GetStats().GetSumW(), 1.7); + EXPECT_FLOAT_EQ(hist->GetStats().GetSumW2(), 1.45); + // Cross-checked with TH1 + EXPECT_FLOAT_EQ(hist->ComputeNEffectiveEntries(), 1.9931034); + EXPECT_FLOAT_EQ(hist->ComputeMean(), 9.0294118); + EXPECT_FLOAT_EQ(hist->ComputeStdDev(), 0.49913420); +} + +TEST(RHistFillContext, StressFillWeight) +{ + static constexpr std::size_t NThreads = 4; + static constexpr std::size_t NFillsPerThread = 10000; + static constexpr std::size_t FlushEveryNFills = 500; + static constexpr std::size_t NFills = NThreads * NFillsPerThread; + static constexpr double Weight = 0.5; + + // Fill a single bin, to maximize contention. + auto hist = std::make_shared>(1, std::make_pair(0, 1)); + { + RHistConcurrentFiller filler(hist); + StressInParallel(NThreads, [&] { + auto context = filler.CreateFillContext(); + for (std::size_t i = 0; i < NFillsPerThread; i++) { + context->Fill(0.5, RWeight(Weight)); + if (i % FlushEveryNFills == 0) { + context->Flush(); + } + } + }); + } + + EXPECT_EQ(hist->GetBinContent(0), NFills * Weight); + EXPECT_EQ(hist->GetNEntries(), NFills); + EXPECT_FLOAT_EQ(hist->ComputeNEffectiveEntries(), NFills); + EXPECT_FLOAT_EQ(hist->ComputeMean(), 0.5); +} + +TEST(RHistFillContext, Flush) +{ + static constexpr std::size_t Bins = 20; + auto hist = std::make_shared>(Bins, std::make_pair(0, Bins)); + + { + RHistConcurrentFiller filler(hist); + auto context = filler.CreateFillContext(); + context->Fill(8.5); + // Flushing multiple times, explicitly and implicitly (in the destructor) should only add the entries once. + context->Flush(); + context->Flush(); + } + + EXPECT_EQ(hist->GetNEntries(), 1); + EXPECT_EQ(hist->GetBinContent(RBinIndex(8)), 1); +} diff --git a/hist/histv7/test/hist_test.hxx b/hist/histv7/test/hist_test.hxx index aef31febe9c19..f0690a8ddd700 100644 --- a/hist/histv7/test/hist_test.hxx +++ b/hist/histv7/test/hist_test.hxx @@ -8,7 +8,9 @@ #include #include #include +#include #include +#include #include #include #include @@ -21,6 +23,7 @@ using ROOT::Experimental::RBinWithError; using ROOT::Experimental::RCategoricalAxis; using ROOT::Experimental::RHist; using ROOT::Experimental::RHistAutoAxisFiller; +using ROOT::Experimental::RHistConcurrentFiller; using ROOT::Experimental::RHistEngine; using ROOT::Experimental::RHistStats; using ROOT::Experimental::RRegularAxis;