From 973878b8baa91797142958c35923ca04f2fb9061 Mon Sep 17 00:00:00 2001 From: Robert Maynard Date: Fri, 27 Sep 2019 12:46:24 -0400 Subject: [PATCH] Improve the performance of the Image and Graph Connectivity algorithms The collection of connectivity algorithms had a couple of inefficiencies. By moving to using WorkId we can remove a couple of arrays of the same size as the input domain. In addition by moving to using atomics we can remove an bool output array with a size equivalent to the input domain and a call to reduce. --- vtkm/exec/AtomicArrayExecutionObject.h | 27 +++++++++ .../connectivities/GraphConnectivity.h | 46 +++++++++------- .../connectivities/ImageConnectivity.h | 55 ++++++++++--------- vtkm/worklet/connectivities/UnionFind.h | 20 ++++--- 4 files changed, 95 insertions(+), 53 deletions(-) diff --git a/vtkm/exec/AtomicArrayExecutionObject.h b/vtkm/exec/AtomicArrayExecutionObject.h index fcc9cc22c..7fdf8f8ea 100644 --- a/vtkm/exec/AtomicArrayExecutionObject.h +++ b/vtkm/exec/AtomicArrayExecutionObject.h @@ -95,6 +95,33 @@ public: static_cast(value))); } + /// \brief Peform an atomic store to memory while enforcing, at minimum, "release" + /// memory ordering. + /// \param index The index of the array element that will be added to. + /// \param value The value to write for the atomic store operation. + /// \warning Using something like: + /// ``` + /// Set(index, Get(index)+N) + /// ``` + /// Should not be done as it is not thread safe, instead you should use + /// the provided Add method instead. + /// + VTKM_SUPPRESS_EXEC_WARNINGS + VTKM_EXEC + void Set(vtkm::Id index, const ValueType& value) const + { + // We only support 32/64 bit signed/unsigned ints, and AtomicInterface + // currently only provides API for unsigned types. + // We'll cast the signed types to unsigned to work around this. + // This is safe, since the only difference between signed/unsigned types + // is how overflow works, and signed overflow is already undefined. We also + // document that overflow is undefined for this operation. + using APIType = typename std::make_unsigned::type; + + AtomicInterface::Store(reinterpret_cast(this->Data + index), + static_cast(value)); + } + /// \brief Perform an atomic CAS operation with sequentially consistent /// memory ordering. /// \param index The index of the array element that will be atomically diff --git a/vtkm/worklet/connectivities/GraphConnectivity.h b/vtkm/worklet/connectivities/GraphConnectivity.h index eb0283b70..9a8fdb511 100644 --- a/vtkm/worklet/connectivities/GraphConnectivity.h +++ b/vtkm/worklet/connectivities/GraphConnectivity.h @@ -11,6 +11,7 @@ #ifndef vtk_m_worklet_connectivity_graph_connectivity_h #define vtk_m_worklet_connectivity_graph_connectivity_h +#include #include #include #include @@ -26,10 +27,12 @@ namespace detail class Graft : public vtkm::worklet::WorkletMapField { public: - using ControlSignature = - void(FieldIn index, FieldIn start, FieldIn degree, WholeArrayIn ids, WholeArrayInOut comp); + using ControlSignature = void(FieldIn start, + FieldIn degree, + WholeArrayIn ids, + WholeArrayInOut comp); - using ExecutionSignature = void(_1, _2, _3, _4, _5); + using ExecutionSignature = void(WorkIndex, _1, _2, _3, _4); using InputDomain = _1; @@ -64,29 +67,29 @@ public: const InputPortalType& connectivityArray, OutputPortalType& componentsOut) const { - bool allStar = false; + bool everythingIsAStar = false; vtkm::cont::ArrayHandle components; - vtkm::cont::ArrayHandle isStar; - vtkm::cont::ArrayHandle cellIds; Algorithm::Copy( vtkm::cont::ArrayHandleCounting(0, 1, numIndicesArray.GetNumberOfValues()), - cellIds); - Algorithm::Copy(cellIds, components); + components); + + //used as an atomic bool, so we use Int32 as it the + //smallest type that VTK-m supports as atomics + vtkm::cont::ArrayHandle allStars; + allStars.Allocate(1); + + vtkm::cont::Invoker invoke; do { - vtkm::worklet::DispatcherMapField graftDispatcher; - graftDispatcher.Invoke( - cellIds, indexOffsetsArray, numIndicesArray, connectivityArray, components); + allStars.GetPortalControl().Set(0, 1); //reset the atomic state + invoke(detail::Graft{}, indexOffsetsArray, numIndicesArray, connectivityArray, components); - // Detection of allStar has to come before pointer jumping. Don't try to rearrange it. - vtkm::worklet::DispatcherMapField isStarDisp; - isStarDisp.Invoke(cellIds, components, isStar); - allStar = Algorithm::Reduce(isStar, true, vtkm::LogicalAnd()); - - vtkm::worklet::DispatcherMapField pointJumpingDispatcher; - pointJumpingDispatcher.Invoke(cellIds, components); - } while (!allStar); + // Detection of allStars has to come before pointer jumping. Don't try to rearrange it. + invoke(IsStar{}, components, allStars); + everythingIsAStar = (allStars.GetPortalControl().Get(0) == 1); + invoke(PointerJumping{}, components); + } while (!everythingIsAStar); // renumber connected component to the range of [0, number of components). vtkm::cont::ArrayHandle uniqueComponents; @@ -94,6 +97,11 @@ public: Algorithm::Sort(uniqueComponents); Algorithm::Unique(uniqueComponents); + vtkm::cont::ArrayHandle cellIds; + Algorithm::Copy( + vtkm::cont::ArrayHandleCounting(0, 1, numIndicesArray.GetNumberOfValues()), + cellIds); + vtkm::cont::ArrayHandle uniqueColor; Algorithm::Copy( vtkm::cont::ArrayHandleCounting(0, 1, uniqueComponents.GetNumberOfValues()), diff --git a/vtkm/worklet/connectivities/ImageConnectivity.h b/vtkm/worklet/connectivities/ImageConnectivity.h index 1c51866a5..232e03c0f 100644 --- a/vtkm/worklet/connectivities/ImageConnectivity.h +++ b/vtkm/worklet/connectivities/ImageConnectivity.h @@ -11,12 +11,10 @@ #ifndef vtk_m_worklet_connectivity_ImageConnectivity_h #define vtk_m_worklet_connectivity_ImageConnectivity_h -#include -#include +#include #include #include -#include #include #include @@ -34,13 +32,12 @@ class ImageGraft : public vtkm::worklet::WorkletPointNeighborhood { public: using ControlSignature = void(CellSetIn, - FieldIn index, FieldInNeighborhood compIn, FieldInNeighborhood color, WholeArrayInOut compOut, - FieldOut changed); + AtomicArrayInOut changed); - using ExecutionSignature = _6(_2, _3, _4, _5); + using ExecutionSignature = void(WorkIndex, _2, _3, _4, _5); template VTKM_EXEC vtkm::Id findRoot(Comp& comp, vtkm::Id index) const @@ -51,11 +48,12 @@ public: } // compOut is an alias of compIn such that we can update component labels - template - VTKM_EXEC bool operator()(const vtkm::Id index, + template + VTKM_EXEC void operator()(const vtkm::Id index, const NeighborComp& neighborComp, const NeighborColor& neighborColor, - CompOut& compOut) const + CompOut& compOut, + AtomicInOut& updated) const { vtkm::Id myComp = neighborComp.Get(0, 0, 0); auto minComp = myComp; @@ -88,8 +86,11 @@ public: else if (myRoot > newRoot) compOut.Set(myRoot, newRoot); - // return if the labeling has changed. - return myComp != minComp; + // mark an update occurred if no other worklets have done so yet + if (myComp != minComp && updated.Get(0) == 0) + { + updated.Set(0, 1); + } } }; } @@ -110,23 +111,18 @@ public: Algorithm::Copy(vtkm::cont::ArrayHandleCounting(0, 1, pixels.GetNumberOfValues()), components); - vtkm::cont::ArrayHandle pixelIds; - Algorithm::Copy(vtkm::cont::ArrayHandleCounting(0, 1, pixels.GetNumberOfValues()), - pixelIds); - - vtkm::cont::ArrayHandle changed; - - using DispatcherType = vtkm::worklet::DispatcherPointNeighborhood; + //used as an atomic bool, so we use Int32 as it the + //smallest type that VTK-m supports as atomics + vtkm::cont::ArrayHandle updateRequired; + updateRequired.Allocate(1); + vtkm::cont::Invoker invoke; do { - DispatcherType dispatcher; - dispatcher.Invoke(input, pixelIds, components, pixels, components, changed); - - vtkm::worklet::DispatcherMapField pointJumpingDispatcher; - pointJumpingDispatcher.Invoke(pixelIds, components); - - } while (Algorithm::Reduce(changed, false, vtkm::LogicalOr())); + updateRequired.GetPortalControl().Set(0, 0); //reset the atomic state + invoke(detail::ImageGraft{}, input, components, pixels, components, updateRequired); + invoke(PointerJumping{}, components); + } while (updateRequired.GetPortalControl().Get(0) > 0); // renumber connected component to the range of [0, number of components). vtkm::cont::ArrayHandle uniqueComponents; @@ -134,10 +130,15 @@ public: Algorithm::Sort(uniqueComponents); Algorithm::Unique(uniqueComponents); + vtkm::cont::ArrayHandle pixelIds; + Algorithm::Copy(vtkm::cont::ArrayHandleCounting(0, 1, pixels.GetNumberOfValues()), + pixelIds); + vtkm::cont::ArrayHandle uniqueColor; Algorithm::Copy( vtkm::cont::ArrayHandleCounting(0, 1, uniqueComponents.GetNumberOfValues()), uniqueColor); + vtkm::cont::ArrayHandle cellColors; vtkm::cont::ArrayHandle pixelIdsOut; InnerJoin().Run( @@ -176,8 +177,8 @@ public: vtkm::cont::CastAndCall(pixels, RunImpl(), input, componentsOut); } - template - void Run(const vtkm::cont::DynamicCellSet& input, + template + void Run(const vtkm::cont::DynamicCellSetBase& input, const vtkm::cont::ArrayHandle& pixels, OutputPortalType& componentsOut) const { diff --git a/vtkm/worklet/connectivities/UnionFind.h b/vtkm/worklet/connectivities/UnionFind.h index 47d647c8c..9e8f5fca6 100644 --- a/vtkm/worklet/connectivities/UnionFind.h +++ b/vtkm/worklet/connectivities/UnionFind.h @@ -23,8 +23,8 @@ namespace connectivity class PointerJumping : public vtkm::worklet::WorkletMapField { public: - using ControlSignature = void(FieldIn index, WholeArrayInOut comp); - using ExecutionSignature = void(_1, _2); + using ControlSignature = void(WholeArrayInOut comp); + using ExecutionSignature = void(WorkIndex, _1); using InputDomain = _1; template @@ -47,14 +47,20 @@ public: class IsStar : public vtkm::worklet::WorkletMapField { public: - using ControlSignature = void(FieldIn index, WholeArrayIn comp, FieldOut); - using ExecutionSignature = _3(_1, _2); + using ControlSignature = void(WholeArrayIn comp, AtomicArrayInOut); + using ExecutionSignature = void(WorkIndex, _1, _2); using InputDomain = _1; - template - VTKM_EXEC bool operator()(vtkm::Id index, InOutPortalType& comp) const + template + VTKM_EXEC void operator()(vtkm::Id index, InOutPortalType& comp, AtomicInOut& hasStar) const { - return comp.Get(index) == comp.Get(comp.Get(index)); + //hasStar emulates a LogicalAnd across all the values + //where we start with a value of 'true'|1. + const bool isAStar = (comp.Get(index) == comp.Get(comp.Get(index))); + if (!isAStar && hasStar.Get(0) == 1) + { + hasStar.Set(0, 0); + } } };