Fixed and improved mergeAndReset for counters

In particuler :
  - made it a free function
  - fixed bad usage of move semantic in favor of references

GaudiKernel/include/Gaudi/Accumulators.h

@@ -464,7 +464,7 @@ namespace Gaudi::Accumulators {
     OutputType value() const { return OutputTransform{}( ValueHandler::getValue( m_value ) ); }
     void       reset() { reset( ValueHandler::DefaultValue() ); }
     template <atomicity ato, typename VH>
-    void mergeAndReset( GenericAccumulator<InputType, InnerType, ato, InputTransform, OutputTransform, VH>&& other ) {
+    void mergeAndReset( GenericAccumulator<InputType, InnerType, ato, InputTransform, OutputTransform, VH>& other ) {
       ValueHandler::merge( m_value, VH::exchange( other.m_value, VH::DefaultValue() ) );
     }
     template <atomicity ato, typename VH>
@@ -509,8 +509,8 @@ namespace Gaudi::Accumulators {
     OutputType value() const { return std::make_tuple( Bases<Atomicity, Arithmetic>::value()... ); }
     void       reset() { ( Bases<Atomicity, Arithmetic>::reset(), ... ); }
     template <atomicity Ato>
-    void mergeAndReset( AccumulatorSet<Arithmetic, Ato, InputType, Bases...>&& other ) {
-      ( Bases<Atomicity, Arithmetic>::mergeAndReset( static_cast<Bases<Ato, Arithmetic>&&>( other ) ), ... );
+    void mergeAndReset( AccumulatorSet<Arithmetic, Ato, InputType, Bases...>& other ) {
+      ( Bases<Atomicity, Arithmetic>::mergeAndReset( static_cast<Bases<Ato, Arithmetic>&>( other ) ), ... );
     }
     template <atomicity Ato>
     void operator+( AccumulatorSet<Arithmetic, Ato, InputType, Bases...>&& other ) {
@@ -696,10 +696,10 @@ namespace Gaudi::Accumulators {
     };
     BinomialAccumulator& operator+=( binomial_t b ) {
       assert( b.nPass <= b.nTotal );
-      TrueAccumulator<Atomicity, bool>::mergeAndReset(
-          TrueAccumulator<atomicity::none, bool>{ std::in_place, b.nPass } );
-      FalseAccumulator<Atomicity, bool>::mergeAndReset(
-          FalseAccumulator<atomicity::none, bool>{ std::in_place, b.nTotal - b.nPass } );
+      TrueAccumulator<atomicity::none, bool> t{ std::in_place, b.nPass };
+      TrueAccumulator<Atomicity, bool>::mergeAndReset( t );
+      FalseAccumulator<atomicity::none, bool> f{ std::in_place, b.nTotal - b.nPass };
+      FalseAccumulator<Atomicity, bool>::mergeAndReset( f );
       return *this;
     }
   };
@@ -819,7 +819,7 @@ namespace Gaudi::Accumulators {
     Buffer( const Buffer& ) = delete;
     void operator=( const Buffer& ) = delete;
     Buffer( Buffer&& other ) : base_type( other ), m_prime( other.m_prime ) { other.reset(); }
-    void push() { m_prime.mergeAndReset( static_cast<base_type&&>( *this ) ); }
+    void push() { m_prime.mergeAndReset( static_cast<base_type&>( *this ) ); }
     ~Buffer() { push(); }
 
   private:
@@ -926,6 +926,7 @@ namespace Gaudi::Accumulators {
     MsgStream&  print( MsgStream& o, bool tableFormat = false ) const override { return printImpl( o, tableFormat ); }
     bool        toBePrinted() const override { return this->nEntries() > 0; }
     friend void reset( Counter& c ) { c.reset(); }
+    friend void mergeAndReset( Counter& c, Counter& o ) { c.mergeAndReset( o ); }
     friend void to_json( nlohmann::json& j, Counter const& c ) {
       j = { { "type", c.typeString }, { "empty", c.nEntries() == 0 }, { "nEntries", c.nEntries() } };
     }
@@ -961,6 +962,7 @@ namespace Gaudi::Accumulators {
 
     bool        toBePrinted() const override { return this->nEntries() > 0; }
     friend void reset( AveragingCounter& c ) { return c.reset(); }
+    friend void mergeAndReset( AveragingCounter& c, AveragingCounter& o ) { c.mergeAndReset( o ); }
     friend void to_json( nlohmann::json& j, AveragingCounter const& c ) {
       j = { { "type", c.typeString },
             { "empty", c.nEntries() == 0 },
@@ -1001,6 +1003,7 @@ namespace Gaudi::Accumulators {
     MsgStream&  print( MsgStream& o, bool tableFormat = false ) const override { return printImpl( o, tableFormat ); }
     bool        toBePrinted() const override { return this->nEntries() > 0; }
     friend void reset( SigmaCounter& c ) { c.reset(); }
+    friend void mergeAndReset( SigmaCounter& c, SigmaCounter& o ) { c.mergeAndReset( o ); }
     friend void to_json( nlohmann::json& j, SigmaCounter const& c ) {
       j = { { "type", c.typeString },
             { "empty", c.nEntries() == 0 },
@@ -1042,6 +1045,7 @@ namespace Gaudi::Accumulators {
     MsgStream&  print( MsgStream& o, bool tableFormat = false ) const override { return printImpl( o, tableFormat ); }
     bool        toBePrinted() const override { return this->nEntries() > 0; }
     friend void reset( StatCounter& c ) { c.reset(); }
+    friend void mergeAndReset( StatCounter& c, StatCounter& o ) { c.mergeAndReset( o ); }
     friend void to_json( nlohmann::json& j, StatCounter const& c ) {
       j = { { "type", c.typeString },
             { "empty", c.nEntries() == 0 },
@@ -1096,6 +1100,7 @@ namespace Gaudi::Accumulators {
     MsgStream&    print( MsgStream& o, std::string_view tag ) const override { return printImpl( o, tag ); }
     bool          toBePrinted() const override { return this->nEntries() > 0; }
     friend void   reset( BinomialCounter& c ) { c.reset(); }
+    friend void   mergeAndReset( BinomialCounter& c, BinomialCounter& o ) { c.mergeAndReset( o ); }
     friend void   to_json( nlohmann::json& j, BinomialCounter const& c ) {
       j = { { "type", c.typeString },
             { "empty", c.nEntries() == 0 },
@@ -1158,6 +1163,7 @@ namespace Gaudi::Accumulators {
     MsgStream&    print( MsgStream& os, bool tableFormat ) const override { return printImpl( os, tableFormat ); }
     bool          toBePrinted() const override { return this->value() > 0; }
     friend void   reset( MsgCounter& c ) { c.reset(); }
+    friend void   mergeAndReset( MsgCounter& c, MsgCounter& o ) { c.mergeAndReset( o ); }
     friend void   to_json( nlohmann::json& j, MsgCounter const& c ) {
       j = { { "type", c.typeString },  { "empty", c.value() == 0 },
             { "nEntries", c.value() }, { "level", level },

GaudiKernel/include/Gaudi/Accumulators/Histogram.h

@@ -320,10 +320,10 @@ namespace Gaudi::Accumulators {
       for ( unsigned int index = 0; index < m_totNBins; index++ ) accumulator( index ).reset();
     }
     template <atomicity ato>
-    void mergeAndReset( HistogramingAccumulatorInternal<ato, InputType, Arithmetic, ND, BaseAccumulatorT>&& other ) {
+    void mergeAndReset( HistogramingAccumulatorInternal<ato, InputType, Arithmetic, ND, BaseAccumulatorT>& other ) {
       assert( m_totNBins == other.m_totNBins );
       for ( unsigned int index = 0; index < m_totNBins; index++ ) {
-        accumulator( index ).mergeAndReset( std::move( other.accumulator( index ) ) );
+        accumulator( index ).mergeAndReset( other.accumulator( index ) );
       }
     }
     [[nodiscard]] auto operator[]( typename InputType::ValueType v ) {
@@ -481,6 +481,9 @@ namespace Gaudi::Accumulators {
     }
     MsgStream&  print( MsgStream& o, bool tableFormat = false ) const override { return printImpl( o, tableFormat ); }
     friend void reset( HistogramingCounterBaseInternal& c ) { c.reset(); }
+    friend void mergeAndReset( HistogramingCounterBaseInternal& h, HistogramingCounterBaseInternal& o ) {
+      h.mergeAndReset( o );
+    }
     friend void to_json( nlohmann::json& j, HistogramingCounterBaseInternal const& h ) {
       // get all bin values and compute total nbEntries
       std::vector<typename AccumulatorType::BaseAccumulator::OutputType> bins;

GaudiKernel/include/Gaudi/MonitoringHub.h

@@ -66,9 +66,17 @@ namespace Gaudi::Monitoring {
 
   } // namespace details
 
+  struct ClashingEntityName : std::logic_error {
+    using logic_error::logic_error;
+  };
+
   /// default (empty) implementation of reset method for types stored into an entity
   template <typename T>
-  void reset( T const& t ) {}
+  void reset( T& ) {}
+
+  /// default (empty) implementation of mergeAndReset method for types stored into an entity
+  template <typename T>
+  void mergeAndReset( T&, T& ) {}
 
   /// Central entity in a Gaudi application that manages monitoring objects (i.e. counters, histograms, etc.).
   ///
@@ -81,6 +89,13 @@ namespace Gaudi::Monitoring {
      * Any object can be used as internal data and wrapped into an Entity as long as they can
      * be translated to json format. It is enough to make them aware to the nlohmann library
      * through a dedicated to_json method.
+     *
+     * another 2 free functions can be overloaded to adapt the behavior of an Entity for a
+     * given type T :
+     *   - void reset( T& t )
+     *     called to reset the entity. The default provided implementation is empty
+     *   - void mergeAndReset( T& ent, T&& other )
+     *     called to merge other into entity and reset other. The default provided implementation is empty
      */
     class Entity {
     public:
@@ -93,10 +108,11 @@ namespace Gaudi::Monitoring {
           , m_typeIndex{ []( const void* ptr ) {
             return std::type_index( typeid( *reinterpret_cast<const T*>( ptr ) ) );
           } }
-          , m_reset{ []( void* ptr ) { reset( *reinterpret_cast<T*>( ptr ) ); } }
-          , m_mergeAndReset{ details::makeMergeAndResetFor<T>() }
-          , m_getJSON{ []( const void* ptr ) -> nlohmann::json { return *reinterpret_cast<const T*>( ptr ); } }
-          , m_mergeAndResetFromJSON{ details::makeMergeAndResetFromJSONFor<T>() } {}
+          , m_getJSON{ []( Entity const& e ) -> nlohmann::json { return *reinterpret_cast<const T*>( e.m_ptr ); } }
+          , m_reset{ []( Entity& e ) { reset( *reinterpret_cast<T*>( e.m_ptr ) ); } }
+          , m_mergeAndReset{ []( Entity& e, Entity& o ) {
+            mergeAndReset( *reinterpret_cast<T*>( e.m_ptr ), *reinterpret_cast<T*>( o.m_ptr ) );
+          } } {}
       /// name of the component owning the Entity
       std::string component;
       /// name of the entity
@@ -106,26 +122,21 @@ namespace Gaudi::Monitoring {
       /// function to get internal type
       std::type_index typeIndex() const { return ( *m_typeIndex )( m_ptr ); }
       /// conversion to json via nlohmann library
-      friend void to_json( nlohmann::json& j, Gaudi::Monitoring::Hub::Entity const& e ) {
-        j = ( *e.m_getJSON )( e.m_ptr );
-      }
+      friend void to_json( nlohmann::json& j, Gaudi::Monitoring::Hub::Entity const& e ) { j = ( *e.m_getJSON )( e ); }
       /// function resetting internal data
-      friend void reset( Entity& e ) { ( *e.m_reset )( e.m_ptr ); }
-      // The following function does not protect against usage with entities with different internal types
-      // The user should ensure that entities are compatible before calling this function
-      /// function calling merge and reset on internal data with the internal data of another entity
-      void mergeAndReset( Entity const& ent ) {
-        if ( typeIndex() != ent.typeIndex() ) {
+      friend void reset( Entity& e ) { ( *e.m_reset )( e ); }
+      /**
+       * function calling merge and reset on internal data with the internal data of another entity
+       *
+       * This function does not protect against usage with entities with different internal types
+       * The user should ensure that entities are compatible before calling this function
+       */
+      friend void mergeAndReset( Entity& ent, Entity& other ) {
+        if ( ent.typeIndex() != other.typeIndex() ) {
           throw std::runtime_error( std::string( "Entity: mergeAndReset called on different types: " ) +
-                                    typeIndex().name() + " and " + ent.typeIndex().name() );
+                                    ent.typeIndex().name() + " and " + other.typeIndex().name() );
         }
-        return ( *m_mergeAndReset )( m_ptr, ent.m_ptr );
-      }
-      /// tell if the Entity data can be updated from JSON input
-      bool canMergeFromJSON() const { return m_mergeAndResetFromJSON; }
-      /// allow merging to internal data from JSON input
-      void mergeAndReset( nlohmann::json const& j ) {
-        if ( canMergeFromJSON() ) ( *m_mergeAndResetFromJSON )( m_ptr, j );
+        ( *ent.m_mergeAndReset )( ent, other );
       }
       /// operator== for comparison with raw pointer
       bool operator==( void* ent ) { return m_ptr == ent; }
@@ -140,14 +151,12 @@ namespace Gaudi::Monitoring {
       // (see Constructor above and the usage of T in the reinterpret_cast)
       /// function to get internal type.
       std::type_index ( *m_typeIndex )( const void* );
+      /// function converting the internal data to json.
+      nlohmann::json ( *m_getJSON )( Entity const& );
       /// function reseting internal data.
-      void ( *m_reset )( void* );
+      void ( *m_reset )( Entity& );
       /// function calling merge and reset on internal data with the internal data of another entity
-      details::MergeAndReset_t m_mergeAndReset{ nullptr };
-      /// function converting the internal data to json.
-      nlohmann::json ( *m_getJSON )( const void* );
-      /// function calling merge and reset on internal data from JSON input
-      details::MergeAndResetFromJSON_t m_mergeAndResetFromJSON{ nullptr };
+      void ( *m_mergeAndReset )( Entity&, Entity& );
     };
 
     /// Interface reporting services must implement.

GaudiKernel/include/GaudiKernel/StatEntity.h

@@ -74,7 +74,7 @@ public:
     return *this;
   }
   StatEntity& operator+=( StatEntity by ) {
-    mergeAndReset( std::move( by ) );
+    mergeAndReset( by );
     return *this;
   }
   unsigned long add( const double v ) {

GaudiKernel/tests/src/CounterHistosUnitTest.cpp

@@ -261,16 +261,16 @@ BOOST_AUTO_TEST_CASE( test_histos_merge_reset, *boost::unit_test::tolerance( 1e-
     hist2[i] += 2 * i;
   }
 
-  ent1a.mergeAndReset( ent2a );
+  mergeAndReset( ent1a, ent2a );
   BOOST_TEST( toJSON( hist1 ).at( "nEntries" ).get<unsigned long>() == 135 );
   BOOST_TEST( toJSON( hist2 ).at( "nEntries" ).get<unsigned long>() == 0 );
-  ent2b.mergeAndReset( ent1b );
+  mergeAndReset( ent2b, ent1b );
   BOOST_TEST( toJSON( hist1 ).at( "nEntries" ).get<unsigned long>() == 0 );
   BOOST_TEST( toJSON( hist2 ).at( "nEntries" ).get<unsigned long>() == 135 );
-  ent1a.mergeAndReset( ent2b );
+  mergeAndReset( ent1a, ent2b );
   BOOST_TEST( toJSON( hist1 ).at( "nEntries" ).get<unsigned long>() == 135 );
   BOOST_TEST( toJSON( hist2 ).at( "nEntries" ).get<unsigned long>() == 0 );
-  ent2b.mergeAndReset( ent1a );
+  mergeAndReset( ent2b, ent1a );
   BOOST_TEST( toJSON( hist1 ).at( "nEntries" ).get<unsigned long>() == 0 );
   BOOST_TEST( toJSON( hist2 ).at( "nEntries" ).get<unsigned long>() == 135 );
 }

GaudiKernel/tests/src/GenericSinkUnitTest.cpp

@@ -30,7 +30,7 @@ namespace {
   };
 
   // A class to be used in Entities and through the Sink/Hub machinery but
-  // not deriving from counters and with no mergeAndReset method
+  // not deriving from counters
   // Basically accumulate data into a string
   struct Store {
     std::string             m_data;

GaudiKernel/tests/src/MonitoringEntityUnitTest.cpp

@@ -86,44 +86,17 @@ TEST_CASE( "Gaudi::Monitoring::Hub::Entity" ) {
       other += 10;
       CHECK( other.nEntries() == 10 );
       Hub::Entity other_entity( "", "", other.typeString, other );
-      e.mergeAndReset( other_entity );
+      mergeAndReset( e, other_entity );
       CHECK( c.nEntries() == 15 );
     }
-    SECTION( "from JSON" ) {
-      REQUIRE( e.canMergeFromJSON() );
-      CHECK( c.nEntries() == 0 );
-      c += 5;
-      CHECK( c.nEntries() == 5 );
-      e.mergeAndReset( nlohmann::json{ { "nEntries", 10 } } );
-      CHECK( c.nEntries() == 15 );
-    }
-    SECTION( "self registered" ) {
-      MiniSink  sink;
-      Owner     owner;
-      Counter<> owned( &owner, "counter" );
-      REQUIRE( sink.entity.has_value() );
-      CHECK( sink.entity->canMergeFromJSON() );
-      CHECK( owned.nEntries() == 0 );
-      owned += 5;
-      CHECK( owned.nEntries() == 5 );
-      sink.entity->mergeAndReset( nlohmann::json{ { "nEntries", 10 } } );
-      CHECK( owned.nEntries() == 15 );
-    }
   }
   SECTION( "no merge" ) {
     SECTION( "from Entity" ) {
       DummyData   other;
       Hub::Entity other_entity( "", "", de.type, other );
-      de.mergeAndReset( other_entity );
+      mergeAndReset( de, other_entity );
       // we just check there is no error
     }
-    SECTION( "from JSON" ) {
-      CHECK_FALSE( de.canMergeFromJSON() );
-      de.mergeAndReset( nlohmann::json{} ); // can be invoked, but still no-op
-    }
-    SECTION( "invalid data" ) {
-      CHECK_THROWS( e.mergeAndReset( de ) );
-      CHECK_THROWS( e.mergeAndReset( nlohmann::json{} ) );
-    }
+    SECTION( "invalid data" ) { CHECK_THROWS( mergeAndReset( e, de ) ); }
   }
 }