From 09a22315c1fcc5e9507f21c19f9d93df2d0af39d Mon Sep 17 00:00:00 2001 From: Jonas Hahnfeld Date: Mon, 15 Jun 2026 10:37:38 +0200 Subject: [PATCH] [hist] Test forwarding of Fill arguments They should not be copied, which might be expensive. --- hist/histv7/test/hist_axes.cxx | 14 ++++++++++++ hist/histv7/test/hist_concurrent.cxx | 30 +++++++++++++++++++++++++ hist/histv7/test/hist_engine.cxx | 20 +++++++++++++++++ hist/histv7/test/hist_engine_atomic.cxx | 20 +++++++++++++++++ hist/histv7/test/hist_hist.cxx | 22 ++++++++++++++++++ hist/histv7/test/hist_stats.cxx | 19 ++++++++++++++++ hist/histv7/test/hist_test.hxx | 23 +++++++++++++++++++ 7 files changed, 148 insertions(+) diff --git a/hist/histv7/test/hist_axes.cxx b/hist/histv7/test/hist_axes.cxx index b73125f378f80..ceef371f59cc7 100644 --- a/hist/histv7/test/hist_axes.cxx +++ b/hist/histv7/test/hist_axes.cxx @@ -251,6 +251,20 @@ TEST(RAxes, ComputeGlobalIndexNoFlowBins) } } +TEST(RAxes, ComputeGlobalIndexForward) +{ + static constexpr std::size_t Bins = 20; + const RRegularAxis axis(Bins, {0, Bins}); + const RAxes axes({axis}); + + std::tuple args(1.5); + auto globalIndex = axes.ComputeGlobalIndex(args); + EXPECT_EQ(globalIndex.fIndex, 2); + EXPECT_TRUE(globalIndex.fValid); + + ASSERT_FALSE(CopyArgument::HasBeenCopied()); +} + TEST(RAxes, ComputeGlobalIndexInvalidNumberOfArguments) { static constexpr std::size_t Bins = 20; diff --git a/hist/histv7/test/hist_concurrent.cxx b/hist/histv7/test/hist_concurrent.cxx index 73691cbe5762b..08bf7e1497e2b 100644 --- a/hist/histv7/test/hist_concurrent.cxx +++ b/hist/histv7/test/hist_concurrent.cxx @@ -190,6 +190,36 @@ TEST(RHistFillContext, FillCategoricalWeight) EXPECT_FLOAT_EQ(hist->ComputeNEffectiveEntries(), 1.9931034); } +TEST(RHistFillContext, FillForward) +{ + 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(); + std::tuple args(1.5); + context->Fill(args); + context->Fill(args, RWeight(0.5)); + } + EXPECT_EQ(hist->GetNEntries(), 2); + EXPECT_EQ(hist->GetBinContent(1), 1.5); + + ASSERT_FALSE(CopyArgument::HasBeenCopied()); + + { + RHistConcurrentFiller filler(hist); + auto context = filler.CreateFillContext(); + CopyArgument arg(2.5); + context->Fill(arg); + context->Fill(arg, RWeight(0.5)); + } + EXPECT_EQ(hist->GetNEntries(), 4); + EXPECT_EQ(hist->GetBinContent(2), 1.5); + + ASSERT_FALSE(CopyArgument::HasBeenCopied()); +} + TEST(RHistFillContext, Flush) { static constexpr std::size_t Bins = 20; diff --git a/hist/histv7/test/hist_engine.cxx b/hist/histv7/test/hist_engine.cxx index e606e3ecc60a3..2d5ede53787e2 100644 --- a/hist/histv7/test/hist_engine.cxx +++ b/hist/histv7/test/hist_engine.cxx @@ -492,6 +492,26 @@ TEST(RHistEngine, FillWeightNegative) EXPECT_EQ(engine.GetBinContent(RBinIndex(1)), 0); } +TEST(RHistEngine, FillForward) +{ + static constexpr std::size_t Bins = 20; + RHistEngine engine(Bins, {0, Bins}); + + std::tuple args(1.5); + engine.Fill(args); + engine.Fill(args, RWeight(0.5)); + EXPECT_EQ(engine.GetBinContent(1), 1.5); + + ASSERT_FALSE(CopyArgument::HasBeenCopied()); + + CopyArgument arg(2.5); + engine.Fill(arg); + engine.Fill(arg, RWeight(0.5)); + EXPECT_EQ(engine.GetBinContent(2), 1.5); + + ASSERT_FALSE(CopyArgument::HasBeenCopied()); +} + TEST(RHistEngine, Scale) { static constexpr std::size_t Bins = 20; diff --git a/hist/histv7/test/hist_engine_atomic.cxx b/hist/histv7/test/hist_engine_atomic.cxx index 3d141ae7c1107..798b9eacf2adc 100644 --- a/hist/histv7/test/hist_engine_atomic.cxx +++ b/hist/histv7/test/hist_engine_atomic.cxx @@ -237,6 +237,26 @@ TEST(RHistEngine, FillAtomicTupleWeightInvalidNumberOfArguments) EXPECT_THROW(engine2.FillAtomic(std::make_tuple(1, 2, 3), RWeight(1)), std::invalid_argument); } +TEST(RHistEngine, FillAtomicForward) +{ + static constexpr std::size_t Bins = 20; + RHistEngine engine(Bins, {0, Bins}); + + std::tuple args(1.5); + engine.FillAtomic(args); + engine.FillAtomic(args, RWeight(0.5)); + EXPECT_EQ(engine.GetBinContent(1), 1.5); + + ASSERT_FALSE(CopyArgument::HasBeenCopied()); + + CopyArgument arg(2.5); + engine.FillAtomic(arg); + engine.FillAtomic(arg, RWeight(0.5)); + EXPECT_EQ(engine.GetBinContent(2), 1.5); + + ASSERT_FALSE(CopyArgument::HasBeenCopied()); +} + TEST(RHistEngine, StressFillAddAtomicWeight) { static constexpr std::size_t NThreads = 4; diff --git a/hist/histv7/test/hist_hist.cxx b/hist/histv7/test/hist_hist.cxx index 2cc6d2190f2cf..f6e52b1a46c9e 100644 --- a/hist/histv7/test/hist_hist.cxx +++ b/hist/histv7/test/hist_hist.cxx @@ -350,6 +350,28 @@ TEST(RHist, FillExceptionSafety) EXPECT_EQ(hist.GetStats().GetDimensionStats(1).fSumWX, 2.5); } +TEST(RHist, FillForward) +{ + static constexpr std::size_t Bins = 20; + RHist hist(Bins, {0, Bins}); + + std::tuple args(1.5); + hist.Fill(args); + hist.Fill(args, RWeight(0.5)); + EXPECT_EQ(hist.GetNEntries(), 2); + EXPECT_EQ(hist.GetBinContent(1), 1.5); + + ASSERT_FALSE(CopyArgument::HasBeenCopied()); + + CopyArgument arg(2.5); + hist.Fill(arg); + hist.Fill(arg, RWeight(0.5)); + EXPECT_EQ(hist.GetNEntries(), 4); + EXPECT_EQ(hist.GetBinContent(2), 1.5); + + ASSERT_FALSE(CopyArgument::HasBeenCopied()); +} + TEST(RHist, Scale) { static constexpr std::size_t Bins = 20; diff --git a/hist/histv7/test/hist_stats.cxx b/hist/histv7/test/hist_stats.cxx index 5748490ddf4bc..d015a790bda42 100644 --- a/hist/histv7/test/hist_stats.cxx +++ b/hist/histv7/test/hist_stats.cxx @@ -664,6 +664,25 @@ TEST(RHistStats, FillExceptionSafety) EXPECT_EQ(stats.GetDimensionStats(1).fSumWX, 2); } +TEST(RHistEngine, FillForward) +{ + RHistStats stats(1); + + std::tuple args(1.5); + stats.Fill(args); + EXPECT_EQ(stats.GetNEntries(), 1); + EXPECT_EQ(stats.GetDimensionStats(0).fSumWX, 1.5); + + ASSERT_FALSE(CopyArgument::HasBeenCopied()); + + CopyArgument arg(2.5); + stats.Fill(arg); + EXPECT_EQ(stats.GetNEntries(), 2); + EXPECT_EQ(stats.GetDimensionStats(0).fSumWX, 4); + + ASSERT_FALSE(CopyArgument::HasBeenCopied()); +} + TEST(RHistStats, Scale) { RHistStats stats(3); diff --git a/hist/histv7/test/hist_test.hxx b/hist/histv7/test/hist_test.hxx index bec8784885387..a1589f92a30cf 100644 --- a/hist/histv7/test/hist_test.hxx +++ b/hist/histv7/test/hist_test.hxx @@ -45,6 +45,29 @@ using ROOT::Experimental::Internal::RSliceBinIndexMapper; #include #include +class CopyArgument final { + static int gCopies; + + double fArgument; + +public: + CopyArgument(double argument) : fArgument(argument) {} + CopyArgument(const CopyArgument &) { gCopies++; } + CopyArgument(CopyArgument &&) = default; + CopyArgument &operator=(const CopyArgument &) + { + gCopies++; + return *this; + } + CopyArgument &operator=(CopyArgument &&) = default; + + operator double() const { return fArgument; } + + static bool HasBeenCopied() { return gCopies > 0; } +}; + +int CopyArgument::gCopies = 0; + template void StressInParallel(std::size_t nThreads, Work &&w) {