Merge topic 'less_temporary_copies'

4153c2c7 Found a few more places where we don't need to return by value.
dd85fc13 Document why we certain classes member variables need to be const ref.
6fb86da8 DynamicArrayHandle Casting methods now holds by const * const.
c1560e2d Perform less unnecessary copies when deducing a worklets parameters.

Acked-by: Kitware Robot <kwrobot@kitware.com>
Acked-by: Kenneth Moreland <kmorel@sandia.gov>
Merge-request: !320

vtkm/cont/ArrayHandle.h

@@ -602,7 +602,7 @@ public:
   };
 
   VTKM_CONT_EXPORT
-  ArrayHandle(boost::shared_ptr<InternalStruct> i)
+  ArrayHandle(const boost::shared_ptr<InternalStruct>& i)
     : Internals(i)
   { }
 

vtkm/cont/DynamicArrayHandle.h

@@ -114,7 +114,7 @@ struct DynamicArrayHandleCopyHelper {
   template<typename TypeList, typename StorageList>
   VTKM_CONT_EXPORT
   static
-  boost::shared_ptr<vtkm::cont::detail::PolymorphicArrayHandleContainerBase>
+  const boost::shared_ptr<vtkm::cont::detail::PolymorphicArrayHandleContainerBase>&
   GetArrayHandleContainer(const vtkm::cont::DynamicArrayHandleBase<TypeList,StorageList> &src)
   {
     return src.ArrayContainer;
@@ -148,8 +148,7 @@ template<typename Type, typename Storage>
 VTKM_CONT_EXPORT
 vtkm::cont::ArrayHandle<Type,Storage> *
 DynamicArrayHandleTryCast(
-      boost::shared_ptr<vtkm::cont::detail::PolymorphicArrayHandleContainerBase>
-      arrayContainer)
+  const boost::shared_ptr<vtkm::cont::detail::PolymorphicArrayHandleContainerBase>& arrayContainer)
 {
   return detail::DynamicArrayHandleTryCast<Type,Storage>(arrayContainer.get());
 }
@@ -425,14 +424,14 @@ namespace detail {
 
 template<typename Functor, typename Type>
 struct DynamicArrayHandleTryStorage {
-  const DynamicArrayHandle Array;
+  const DynamicArrayHandle* const Array;
   const Functor &Function;
   bool FoundCast;
 
   VTKM_CONT_EXPORT
   DynamicArrayHandleTryStorage(const DynamicArrayHandle &array,
                                const Functor &f)
-    : Array(array), Function(f), FoundCast(false) {  }
+    : Array(&array), Function(f), FoundCast(false) {  }
 
   template<typename Storage>
   VTKM_CONT_EXPORT
@@ -446,9 +445,9 @@ private:
   void DoCast(Storage, boost::mpl::bool_<true>)
   {
     if (!this->FoundCast &&
-        this->Array.template IsTypeAndStorage<Type,Storage>())
+        this->Array->template IsTypeAndStorage<Type,Storage>())
     {
-      this->Function(this->Array.template CastToTypeStorage<Type,Storage>());
+      this->Function(this->Array->template CastToTypeStorage<Type,Storage>());
       this->FoundCast = true;
     }
   }
@@ -462,13 +461,13 @@ private:
 
 template<typename Functor, typename StorageList>
 struct DynamicArrayHandleTryType {
-  const DynamicArrayHandle Array;
+  const DynamicArrayHandle* const Array;
   const Functor &Function;
   bool FoundCast;
 
   VTKM_CONT_EXPORT
   DynamicArrayHandleTryType(const DynamicArrayHandle &array, const Functor &f)
-    : Array(array), Function(f), FoundCast(false) {  }
+    : Array(&array), Function(f), FoundCast(false) {  }
 
   template<typename Type>
   VTKM_CONT_EXPORT
@@ -476,7 +475,7 @@ struct DynamicArrayHandleTryType {
     if (this->FoundCast) { return; }
     typedef DynamicArrayHandleTryStorage<Functor, Type> TryStorageType;
     TryStorageType tryStorage =
-        TryStorageType(this->Array, this->Function);
+        TryStorageType(*this->Array, this->Function);
     vtkm::ListForEach(tryStorage, StorageList());
     if (tryStorage.FoundCast)
     {
@@ -496,10 +495,17 @@ void DynamicArrayHandleBase<TypeList,StorageList>::
   VTKM_IS_LIST_TAG(TypeList);
   VTKM_IS_LIST_TAG(StorageList);
   typedef detail::DynamicArrayHandleTryType<Functor, StorageList> TryTypeType;
+
   // We cast this to a DynamicArrayHandle because at this point we are ignoring
   // the type/storage lists in it. There is no sense in adding more unnecessary
   // template cases.
-  TryTypeType tryType = TryTypeType(DynamicArrayHandle(*this), f);
+  // The downside to this approach is that a copy is created, causing an
+  // atomic increment, which affects both performance and library size.
+  // For these reasons we have a specialization of this method to remove
+  // the copy when the type/storage lists are the default
+  DynamicArrayHandle t(*this);
+  TryTypeType tryType = TryTypeType(t, f);
+
   vtkm::ListForEach(tryType, TypeList());
   if (!tryType.FoundCast)
   {
@@ -508,6 +514,29 @@ void DynamicArrayHandleBase<TypeList,StorageList>::
   }
 }
 
+template<>
+template<typename Functor>
+VTKM_CONT_EXPORT
+void DynamicArrayHandleBase<VTKM_DEFAULT_TYPE_LIST_TAG,
+                            VTKM_DEFAULT_STORAGE_LIST_TAG>::
+    CastAndCall(const Functor &f) const
+{
+
+  typedef detail::DynamicArrayHandleTryType<Functor,
+                                            VTKM_DEFAULT_STORAGE_LIST_TAG> TryTypeType;
+
+  // We can remove the copy, as the current DynamicArrayHandle is already
+  // the default one, and no reason to do an atomic increment and increase
+  // library size, and reduce performance
+  TryTypeType tryType = TryTypeType(*this, f);
+  vtkm::ListForEach(tryType, VTKM_DEFAULT_TYPE_LIST_TAG());
+  if (!tryType.FoundCast)
+  {
+    throw vtkm::cont::ErrorControlBadValue(
+          "Could not find appropriate cast for array in CastAndCall.");
+  }
+}
+
 namespace internal {
 
 template<typename TypeList, typename StorageList>

vtkm/cont/DynamicCellSet.h

@@ -49,7 +49,7 @@ struct DynamicCellSetCopyHelper {
   template<typename CellSetList>
   VTKM_CONT_EXPORT
   static
-  boost::shared_ptr<vtkm::cont::internal::SimplePolymorphicContainerBase>
+  const boost::shared_ptr<vtkm::cont::internal::SimplePolymorphicContainerBase>&
   GetCellSetContainer(const vtkm::cont::DynamicCellSetBase<CellSetList> &src)
   {
     return src.CellSetContainer;
@@ -83,8 +83,7 @@ template<typename CellSetType>
 VTKM_CONT_EXPORT
 CellSetType *
 DynamicCellSetTryCast(
-      boost::shared_ptr<vtkm::cont::internal::SimplePolymorphicContainerBase>
-      cellSetContainer)
+  const boost::shared_ptr<vtkm::cont::internal::SimplePolymorphicContainerBase>& cellSetContainer)
 {
   return detail::DynamicCellSetTryCast<CellSetType>(cellSetContainer.get());
 }

vtkm/exec/internal/WorkletInvokeFunctor.h

@@ -62,7 +62,11 @@ public:
 
 private:
   WorkletType Worklet;
-  InvocationType Invocation;
+
+  // This is held by const reference so that we don't create a copy of the
+  // parameters causing each ArrayHandle shared pointer to increment and
+  // decrement.
+  const InvocationType& Invocation;
 };
 
 }

vtkm/internal/FunctionInterface.h

@@ -64,7 +64,7 @@ struct IdentityFunctor {
 
 template<vtkm::IdComponent ParameterIndex, typename FunctionSignature>
 VTKM_EXEC_CONT_EXPORT
-typename ParameterContainerAccess<ParameterIndex,FunctionSignature>::ParameterType
+const typename ParameterContainerAccess<ParameterIndex,FunctionSignature>::ParameterType &
 GetParameter(const ParameterContainer<FunctionSignature> &parameters) {
   return ParameterContainerAccess<ParameterIndex,FunctionSignature>::GetParameter(parameters);
 }
@@ -347,7 +347,7 @@ public:
   ///
   template<vtkm::IdComponent ParameterIndex>
   VTKM_EXEC_CONT_EXPORT
-  typename ParameterType<ParameterIndex>::type
+  const typename ParameterType<ParameterIndex>::type &
   GetParameter(
       vtkm::internal::IndexTag<ParameterIndex> =
         vtkm::internal::IndexTag<ParameterIndex>()) const {
@@ -394,7 +394,7 @@ public:
   template<vtkm::IdComponent ParameterIndex>
   VTKM_EXEC_CONT_EXPORT
   void SetParameter(
-      typename ParameterType<ParameterIndex>::type parameter,
+      const typename ParameterType<ParameterIndex>::type& parameter,
       vtkm::internal::IndexTag<ParameterIndex> =
         vtkm::internal::IndexTag<ParameterIndex>())
   {
@@ -506,7 +506,7 @@ public:
   template<typename NewType>
   VTKM_CONT_EXPORT
   typename AppendType<NewType>::type
-  Append(NewType newParameter) const {
+  Append(const NewType& newParameter) const {
     typename AppendType<NewType>::type appendedFuncInterface;
     appendedFuncInterface.Copy(*this);
     appendedFuncInterface.template SetParameter<ARITY+1>(newParameter);
@@ -565,7 +565,7 @@ public:
   template<vtkm::IdComponent ParameterIndex, typename NewType>
   VTKM_CONT_EXPORT
   typename ReplaceType<ParameterIndex, NewType>::type
-  Replace(NewType newParameter,
+  Replace(const NewType& newParameter,
           vtkm::internal::IndexTag<ParameterIndex> =
             vtkm::internal::IndexTag<ParameterIndex>()) const {
     typename ReplaceType<ParameterIndex, NewType>::type replacedFuncInterface;
@@ -801,7 +801,7 @@ public:
 
   template<typename T>
   VTKM_CONT_EXPORT
-  void operator()(T newParameter) const
+  void operator()(const T& newParameter) const
   {
     typedef typename vtkm::internal::FunctionInterface<NewFunction>::template AppendType<T>::type
         NextInterfaceType;

vtkm/internal/FunctionInterfaceDetailPost.h

@@ -274,7 +274,7 @@ template<typename R,
 VTKM_EXEC_CONT_EXPORT
 FunctionInterface<R(P1)>
 make_FunctionInterface(
-      P1 p1
+      const P1& p1
       )
 {
   FunctionInterface<R(P1)> fi;
@@ -301,8 +301,8 @@ template<typename R,
 VTKM_EXEC_CONT_EXPORT
 FunctionInterface<R(P1,P2)>
 make_FunctionInterface(
-      P1 p1,
-      P2 p2
+      const P1& p1,
+      const P2& p2
       )
 {
   FunctionInterface<R(P1,P2)> fi;
@@ -331,9 +331,9 @@ template<typename R,
 VTKM_EXEC_CONT_EXPORT
 FunctionInterface<R(P1,P2,P3)>
 make_FunctionInterface(
-      P1 p1,
-      P2 p2,
-      P3 p3
+      const P1& p1,
+      const P2& p2,
+      const P3& p3
       )
 {
   FunctionInterface<R(P1,P2,P3)> fi;
@@ -364,10 +364,10 @@ template<typename R,
 VTKM_EXEC_CONT_EXPORT
 FunctionInterface<R(P1,P2,P3,P4)>
 make_FunctionInterface(
-      P1 p1,
-      P2 p2,
-      P3 p3,
-      P4 p4
+      const P1& p1,
+      const P2& p2,
+      const P3& p3,
+      const P4& p4
       )
 {
   FunctionInterface<R(P1,P2,P3,P4)> fi;
@@ -400,11 +400,11 @@ template<typename R,
 VTKM_EXEC_CONT_EXPORT
 FunctionInterface<R(P1,P2,P3,P4,P5)>
 make_FunctionInterface(
-      P1 p1,
-      P2 p2,
-      P3 p3,
-      P4 p4,
-      P5 p5
+      const P1& p1,
+      const P2& p2,
+      const P3& p3,
+      const P4& p4,
+      const P5& p5
       )
 {
   FunctionInterface<R(P1,P2,P3,P4,P5)> fi;
@@ -439,12 +439,12 @@ template<typename R,
 VTKM_EXEC_CONT_EXPORT
 FunctionInterface<R(P1,P2,P3,P4,P5,P6)>
 make_FunctionInterface(
-      P1 p1,
-      P2 p2,
-      P3 p3,
-      P4 p4,
-      P5 p5,
-      P6 p6
+      const P1& p1,
+      const P2& p2,
+      const P3& p3,
+      const P4& p4,
+      const P5& p5,
+      const P6& p6
       )
 {
   FunctionInterface<R(P1,P2,P3,P4,P5,P6)> fi;
@@ -481,13 +481,13 @@ template<typename R,
 VTKM_EXEC_CONT_EXPORT
 FunctionInterface<R(P1,P2,P3,P4,P5,P6,P7)>
 make_FunctionInterface(
-      P1 p1,
-      P2 p2,
-      P3 p3,
-      P4 p4,
-      P5 p5,
-      P6 p6,
-      P7 p7
+      const P1& p1,
+      const P2& p2,
+      const P3& p3,
+      const P4& p4,
+      const P5& p5,
+      const P6& p6,
+      const P7& p7
       )
 {
   FunctionInterface<R(P1,P2,P3,P4,P5,P6,P7)> fi;
@@ -526,14 +526,14 @@ template<typename R,
 VTKM_EXEC_CONT_EXPORT
 FunctionInterface<R(P1,P2,P3,P4,P5,P6,P7,P8)>
 make_FunctionInterface(
-      P1 p1,
-      P2 p2,
-      P3 p3,
-      P4 p4,
-      P5 p5,
-      P6 p6,
-      P7 p7,
-      P8 p8
+      const P1& p1,
+      const P2& p2,
+      const P3& p3,
+      const P4& p4,
+      const P5& p5,
+      const P6& p6,
+      const P7& p7,
+      const P8& p8
       )
 {
   FunctionInterface<R(P1,P2,P3,P4,P5,P6,P7,P8)> fi;
@@ -574,15 +574,15 @@ template<typename R,
 VTKM_EXEC_CONT_EXPORT
 FunctionInterface<R(P1,P2,P3,P4,P5,P6,P7,P8,P9)>
 make_FunctionInterface(
-      P1 p1,
-      P2 p2,
-      P3 p3,
-      P4 p4,
-      P5 p5,
-      P6 p6,
-      P7 p7,
-      P8 p8,
-      P9 p9
+      const P1& p1,
+      const P2& p2,
+      const P3& p3,
+      const P4& p4,
+      const P5& p5,
+      const P6& p6,
+      const P7& p7,
+      const P8& p8,
+      const P9& p9
       )
 {
   FunctionInterface<R(P1,P2,P3,P4,P5,P6,P7,P8,P9)> fi;
@@ -625,16 +625,16 @@ template<typename R,
 VTKM_EXEC_CONT_EXPORT
 FunctionInterface<R(P1,P2,P3,P4,P5,P6,P7,P8,P9,P10)>
 make_FunctionInterface(
-      P1 p1,
-      P2 p2,
-      P3 p3,
-      P4 p4,
-      P5 p5,
-      P6 p6,
-      P7 p7,
-      P8 p8,
-      P9 p9,
-      P10 p10
+      const P1& p1,
+      const P2& p2,
+      const P3& p3,
+      const P4& p4,
+      const P5& p5,
+      const P6& p6,
+      const P7& p7,
+      const P8& p8,
+      const P9& p9,
+      const P10& p10
       )
 {
   FunctionInterface<R(P1,P2,P3,P4,P5,P6,P7,P8,P9,P10)> fi;

vtkm/internal/FunctionInterfaceDetailPost.h.in

@@ -124,7 +124,7 @@ VTKM_EXEC_CONT_EXPORT
 FunctionInterface<$signature(num_params)>
 make_FunctionInterface(
 $for(param_index in xrange(1,num_params+1))\
-      $ptype(param_index) p$(param_index)$comma_if(param_index<num_params)
+      const $ptype(param_index)& p$(param_index)$comma_if(param_index<num_params)
 $endfor\
       )
 {

vtkm/internal/FunctionInterfaceDetailPre.h

@@ -236,7 +236,7 @@ struct ParameterContainerAccess<1, FunctionSignature> {
   VTKM_SUPPRESS_EXEC_WARNINGS
   VTKM_EXEC_CONT_EXPORT
   static
-  ParameterType
+  const ParameterType&
   GetParameter(const ParameterContainer<FunctionSignature> &parameters) {
     return parameters.Parameter1;
   }
@@ -259,7 +259,7 @@ struct ParameterContainerAccess<2, FunctionSignature> {
   VTKM_SUPPRESS_EXEC_WARNINGS
   VTKM_EXEC_CONT_EXPORT
   static
-  ParameterType
+  const ParameterType&
   GetParameter(const ParameterContainer<FunctionSignature> &parameters) {
     return parameters.Parameter2;
   }
@@ -282,7 +282,7 @@ struct ParameterContainerAccess<3, FunctionSignature> {
   VTKM_SUPPRESS_EXEC_WARNINGS
   VTKM_EXEC_CONT_EXPORT
   static
-  ParameterType
+  const ParameterType&
   GetParameter(const ParameterContainer<FunctionSignature> &parameters) {
     return parameters.Parameter3;
   }
@@ -305,7 +305,7 @@ struct ParameterContainerAccess<4, FunctionSignature> {
   VTKM_SUPPRESS_EXEC_WARNINGS
   VTKM_EXEC_CONT_EXPORT
   static
-  ParameterType
+  const ParameterType&
   GetParameter(const ParameterContainer<FunctionSignature> &parameters) {
     return parameters.Parameter4;
   }
@@ -328,7 +328,7 @@ struct ParameterContainerAccess<5, FunctionSignature> {
   VTKM_SUPPRESS_EXEC_WARNINGS
   VTKM_EXEC_CONT_EXPORT
   static
-  ParameterType
+  const ParameterType&
   GetParameter(const ParameterContainer<FunctionSignature> &parameters) {
     return parameters.Parameter5;
   }
@@ -351,7 +351,7 @@ struct ParameterContainerAccess<6, FunctionSignature> {
   VTKM_SUPPRESS_EXEC_WARNINGS
   VTKM_EXEC_CONT_EXPORT
   static
-  ParameterType
+  const ParameterType&
   GetParameter(const ParameterContainer<FunctionSignature> &parameters) {
     return parameters.Parameter6;
   }
@@ -374,7 +374,7 @@ struct ParameterContainerAccess<7, FunctionSignature> {
   VTKM_SUPPRESS_EXEC_WARNINGS
   VTKM_EXEC_CONT_EXPORT
   static
-  ParameterType
+  const ParameterType&
   GetParameter(const ParameterContainer<FunctionSignature> &parameters) {
     return parameters.Parameter7;
   }
@@ -397,7 +397,7 @@ struct ParameterContainerAccess<8, FunctionSignature> {
   VTKM_SUPPRESS_EXEC_WARNINGS
   VTKM_EXEC_CONT_EXPORT
   static
-  ParameterType
+  const ParameterType&
   GetParameter(const ParameterContainer<FunctionSignature> &parameters) {
     return parameters.Parameter8;
   }
@@ -420,7 +420,7 @@ struct ParameterContainerAccess<9, FunctionSignature> {
   VTKM_SUPPRESS_EXEC_WARNINGS
   VTKM_EXEC_CONT_EXPORT
   static
-  ParameterType
+  const ParameterType&
   GetParameter(const ParameterContainer<FunctionSignature> &parameters) {
     return parameters.Parameter9;
   }
@@ -443,7 +443,7 @@ struct ParameterContainerAccess<10, FunctionSignature> {
   VTKM_SUPPRESS_EXEC_WARNINGS
   VTKM_EXEC_CONT_EXPORT
   static
-  ParameterType
+  const ParameterType&
   GetParameter(const ParameterContainer<FunctionSignature> &parameters) {
     return parameters.Parameter10;
   }

vtkm/internal/FunctionInterfaceDetailPre.h.in

@@ -138,7 +138,7 @@ struct ParameterContainerAccess<$(param_index), FunctionSignature> {
   VTKM_SUPPRESS_EXEC_WARNINGS
   VTKM_EXEC_CONT_EXPORT
   static
-  ParameterType
+  const ParameterType&
   GetParameter(const ParameterContainer<FunctionSignature> &parameters) {
     return parameters.Parameter$(param_index);
   }

vtkm/internal/Invocation.h

@@ -88,8 +88,10 @@ struct Invocation
   ///
   typedef _VisitArrayType VisitArrayType;
 
+  /// \brief Default Invocation constructors that holds the given parameters
+  /// by reference.
   VTKM_CONT_EXPORT
-  Invocation(ParameterInterface parameters,
+  Invocation(const ParameterInterface& parameters,
              OutputToInputMapType outputToInputMap = OutputToInputMapType(),
              VisitArrayType visitArray = VisitArrayType())
     : Parameters(parameters),
@@ -116,7 +118,7 @@ struct Invocation
   template<typename NewParameterInterface>
   VTKM_CONT_EXPORT
   typename ChangeParametersType<NewParameterInterface>::type
-  ChangeParameters(NewParameterInterface newParameters) const {
+  ChangeParameters(const NewParameterInterface& newParameters) const {
     return typename ChangeParametersType<NewParameterInterface>::type(
           newParameters, this->OutputToInputMap, this->VisitArray);
   }
@@ -242,7 +244,7 @@ struct Invocation
   ///
   VTKM_SUPPRESS_EXEC_WARNINGS
   VTKM_EXEC_CONT_EXPORT
-  InputDomainType GetInputDomain() const
+  const InputDomainType& GetInputDomain() const
   {
     return this->Parameters.template GetParameter<InputDomainIndex>();
   }
@@ -250,7 +252,10 @@ struct Invocation
   /// The state of an \c Invocation object holds the parameters of the
   /// invocation. As well as the output to input map and the visit array.
   ///
-  ParameterInterface Parameters;
+  /// This is held by const reference so that we don't create a copy of the
+  /// parameters causing each ArrayHandle shared pointer to increment and
+  //  decrement.
+  const ParameterInterface& Parameters;
   OutputToInputMapType OutputToInputMap;
   VisitArrayType VisitArray;
 };

vtkm/worklet/DispatcherMapField.h

@@ -56,7 +56,7 @@ public:
 
     // We can pull the input domain parameter (the data specifying the input
     // domain) from the invocation object.
-    InputDomainType inputDomain = invocation.GetInputDomain();
+    const InputDomainType &inputDomain = invocation.GetInputDomain();
 
     // For a DispatcherMapField, the inputDomain must be an ArrayHandle (or
     // a DynamicArrayHandle that gets cast to one). The size of the domain

vtkm/worklet/DispatcherMapTopology.h

@@ -64,7 +64,7 @@ public:
 
     // We can pull the input domain parameter (the data specifying the input
     // domain) from the invocation object.
-    InputDomainType inputDomain = invocation.GetInputDomain();
+    const InputDomainType &inputDomain = invocation.GetInputDomain();
 
     // Now that we have the input domain, we can extract the range of the
     // scheduling and call BadicInvoke.

vtkm/worklet/internal/DispatcherBase.h

@@ -204,13 +204,15 @@ private:
 template<typename ControlInterface>
 struct DispatcherBaseDynamicTransform
 {
+  vtkm::cont::internal::DynamicTransform BasicDynamicTransform;
+
   template<typename InputType,
            typename ContinueFunctor,
            vtkm::IdComponent Index>
   VTKM_CONT_EXPORT
   void operator()(const InputType &input,
                   const ContinueFunctor &continueFunc,
-                  vtkm::internal::IndexTag<Index> indexTag) const
+                  const vtkm::internal::IndexTag<Index>& indexTag) const
   {
     typedef typename ControlInterface::template ParameterType<Index>::type
         ControlSignatureTag;
@@ -219,9 +221,7 @@ struct DispatcherBaseDynamicTransform
         ContinueFunctor, typename ControlSignatureTag::TypeCheckTag, Index>
         TypeCheckFunctor;
 
-    vtkm::cont::internal::DynamicTransform basicDynamicTransform;
-
-    basicDynamicTransform(input, TypeCheckFunctor(continueFunc), indexTag);
+    this->BasicDynamicTransform(input, TypeCheckFunctor(continueFunc), indexTag);
   }
 };