[hist] Test forwarding of Fill arguments

They should not be copied, which might be expensive.

Author: hahnjo

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<CopyArgument> 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;

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<RHist<float>>(Bins, std::make_pair(0, Bins));
+
+   {
+      RHistConcurrentFiller filler(hist);
+      auto context = filler.CreateFillContext();
+      std::tuple<CopyArgument> 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;

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<float> engine(Bins, {0, Bins});
+
+   std::tuple<CopyArgument> 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;

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<float> engine(Bins, {0, Bins});
+
+   std::tuple<CopyArgument> 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;

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<float> hist(Bins, {0, Bins});
+
+   std::tuple<CopyArgument> 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;

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<CopyArgument> 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);

hist/histv7/test/hist_test.hxx

@@ -45,6 +45,29 @@ using ROOT::Experimental::Internal::RSliceBinIndexMapper;
 #include <thread>
 #include <vector>
 
+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 <typename Work>
 void StressInParallel(std::size_t nThreads, Work &&w)
 {