Merge topic 'improve-composite-arrays'

d1681f34b2 review(CompositeArr): improve doc + remove md + add final
2351a836c4 improve(CompositeArray): cached array out performs ranges
157fddc8eb add(changelog): for improvements to `vtkCompositeArray`
d52d7d24cf improve(CompositeArray): performance gain by flattening binary tree

Acked-by: Kitware Robot <kwrobot@kitware.com>
Acked-by: buildbot <buildbot@kitware.com>
Merge-request: !9718

Common/ImplicitArrays/Testing/Cxx/TestCompositeImplicitBackend.cxx

@@ -37,7 +37,7 @@ int TestCompositeImplicitBackend(int, char*[])
   std::iota(rightRange.begin(), rightRange.end(), 10);
 
   // Make structure
-  vtkCompositeImplicitBackend<int> composite(left, right);
+  vtkCompositeImplicitBackend<int> composite(std::vector<vtkDataArray*>({ left, right }));
 
   // Do checks on structure
   for (int i = 0; i < 20; ++i)
@@ -62,7 +62,8 @@ int TestCompositeImplicitBackend(int, char*[])
   auto rightMultiRange = vtk::DataArrayValueRange<3>(rightMulti);
   std::iota(rightMultiRange.begin(), rightMultiRange.end(), 30);
 
-  vtkCompositeImplicitBackend<int> compositeMulti(leftMulti, rightMulti);
+  vtkCompositeImplicitBackend<int> compositeMulti(
+    std::vector<vtkDataArray*>({ leftMulti, rightMulti }));
 
   for (int i = 0; i < 60; ++i)
   {

Common/ImplicitArrays/vtkCompositeArray.h

@@ -48,8 +48,9 @@
  * rightArr->SetNumberOfComponents(1);
  * rightArr->SetNumberOfTuples(1);
  * rightArr->SetValue(0, 1);
+ * std::vector<vtkDataArray*> arrays({leftArr, rightArr});
  * vtkNew<vtkCompositeArray<int>> compositeArr;
- * compositeArr->SetBackend(std::make_shared<vtkCompositeImplicitBackend<int>>(leftArr, rightArr));
+ * compositeArr->SetBackend(std::make_shared<vtkCompositeImplicitBackend<int>>(arrays));
  * compositeArr->SetNumberOfComponents(1);
  * compositeArr->SetNumberOfTuples(2);
  * CHECK(compositArr->GetValue(1) == 1);

Common/ImplicitArrays/vtkCompositeArray.txx

@@ -14,14 +14,6 @@ vtkSmartPointer<vtkCompositeArray<T>> ConcatenateDataArrays(
   {
     return nullptr;
   }
-  if (arrays.size() == 1)
-  {
-    vtkNew<vtkCompositeArray<T>> composite;
-    composite->SetBackend(std::make_shared<vtkCompositeImplicitBackend<T>>(arrays[0], nullptr));
-    composite->SetNumberOfComponents(arrays[0]->GetNumberOfComponents());
-    composite->SetNumberOfTuples(arrays[0]->GetNumberOfTuples());
-    return composite;
-  }
   vtkIdType nComps = arrays[0]->GetNumberOfComponents();
   for (auto arr : arrays)
   {
@@ -31,35 +23,14 @@ vtkSmartPointer<vtkCompositeArray<T>> ConcatenateDataArrays(
       return nullptr;
     }
   }
-  std::vector<vtkSmartPointer<vtkDataArray>> lifetimeBuffer;
-  lifetimeBuffer.assign(arrays.begin(), arrays.end());
-  std::vector<vtkSmartPointer<vtkCompositeArray<T>>> newComps;
-  while (lifetimeBuffer.size() != 1)
-  {
-    newComps.clear();
-    for (int i = 0; i < static_cast<int>(lifetimeBuffer.size() - 1); i += 2)
-    {
-      vtkNew<vtkCompositeArray<T>> composite;
-      composite->SetBackend(
-        std::make_shared<vtkCompositeImplicitBackend<T>>(lifetimeBuffer[i], lifetimeBuffer[i + 1]));
-      composite->SetNumberOfComponents(lifetimeBuffer[i]->GetNumberOfComponents());
-      composite->SetNumberOfTuples(
-        lifetimeBuffer[i]->GetNumberOfTuples() + lifetimeBuffer[i + 1]->GetNumberOfTuples());
-      newComps.emplace_back(composite);
-    }
-    if (lifetimeBuffer.size() % 2 != 0)
-    {
-      vtkNew<vtkCompositeArray<T>> composite;
-      composite->SetBackend(
-        std::make_shared<vtkCompositeImplicitBackend<T>>(newComps.back(), lifetimeBuffer.back()));
-      composite->SetNumberOfComponents(lifetimeBuffer.back()->GetNumberOfComponents());
-      composite->SetNumberOfTuples(
-        newComps.back()->GetNumberOfTuples() + lifetimeBuffer.back()->GetNumberOfTuples());
-      newComps.back() = composite;
-    }
-    lifetimeBuffer.assign(newComps.begin(), newComps.end());
-  }
-  return newComps[0];
+  vtkNew<vtkCompositeArray<T>> composite;
+  composite->SetBackend(std::make_shared<vtkCompositeImplicitBackend<T>>(arrays));
+  composite->SetNumberOfComponents(nComps);
+  int nTuples = 0;
+  std::for_each(arrays.begin(), arrays.end(),
+    [&nTuples](vtkDataArray* arr) { nTuples += arr->GetNumberOfTuples(); });
+  composite->SetNumberOfTuples(nTuples);
+  return composite;
 }
 VTK_ABI_NAMESPACE_END
 }

Common/ImplicitArrays/vtkCompositeImplicitBackend.h

@@ -23,9 +23,10 @@
  * This structure can be classified as a closure and can be called using syntax similar to a
  * function call.
  *
- * The class implements a binary tree like structure for facilitating fast access.
+ * This class uses indirect addressing of cached arrays to provide an access compute complexity of
+ * O(log_2(number_of_arrays)) through its `()` operator.
  *
- * At construction it takes two arrays in order to represent their concatenation.
+ * At construction it takes an array arrays in order to represent their concatenation.
  *
  * An example of potential usage in a vtkImplicitArray
  * ```
@@ -39,7 +40,8 @@
  * rightArr->SetValue(0, 1);
  * vtkNew<vtkImplicitArray<vtkCompositeImplicitBackend<int>>> compositeArr; // easier with
  * `vtkNew<vtkCompositeArray<int>> compositeArr;` if applicable
- * compositeArr->SetBackend(std::make_shared<vtkCompositeImplicitBackend<int>>(leftArr, rightArr));
+ * std::vector<vtkDataArray*> arrays({leftArr, rightArr});
+ * compositeArr->SetBackend(std::make_shared<vtkCompositeImplicitBackend<int>>(arrays));
  * CHECK(compositArr->GetValue(1) == 1);
  * ```
  *
@@ -48,26 +50,30 @@
  * > information.
  */
 #include "vtkCommonImplicitArraysModule.h"
+
 #include <memory>
+#include <vector>
 
 VTK_ABI_NAMESPACE_BEGIN
 class vtkDataArray;
 template <typename ValueType>
-class vtkCompositeImplicitBackend
+class vtkCompositeImplicitBackend final
 {
 public:
   /**
    * Constructor for the backend
-   * @param leftArr the array starting the composite at index 0
-   * @param rightArr the array following the leftArr and starting at index
+   * @param arrays std::vector of arrays to composite together
    * leftArr->GetNumberOfTuples()
    */
-  vtkCompositeImplicitBackend(vtkDataArray* leftArr, vtkDataArray* rightArr);
+  vtkCompositeImplicitBackend(const std::vector<vtkDataArray*>& arrays);
   ~vtkCompositeImplicitBackend();
 
   /**
    * Indexing operator for the composite of the two arrays respecting the `vtkImplicitArray`
-   * expectations
+   * expectations.
+   *
+   * Conceptually, the composite array uses a binary search algorithm through the use of
+   * `std::upper_bounds` to offer a compute complexity of O(log_2(n_arrays))
    */
   ValueType operator()(int idx) const;
 

Common/ImplicitArrays/vtkCompositeImplicitBackend.txx

@@ -19,48 +19,156 @@
 #include "vtkArrayDispatchImplicitArrayList.h"
 #include "vtkDataArray.h"
 #include "vtkDataArrayRange.h"
+#include "vtkImplicitArray.h"
 #include "vtkSmartPointer.h"
 
+namespace
+{
+//-----------------------------------------------------------------------
+/*
+ * A generic interface towards a typed get value. Specialized structures should inherit from this
+ * struct in order to abstractify the array type from the GetValue usage.
+ */
+template <typename ValueType>
+struct TypedArrayCache
+{
+  virtual ValueType GetValue(int idx) const = 0;
+  virtual ~TypedArrayCache() = default;
+};
+
+/*
+ * A templated implementation of the TypedArrayCache above that should be used for arrays that
+ * implement the `GetValue` method from the `vtkGenericDataArray` interface.
+ */
+template <typename ValueType, typename ArrayT>
+struct SpecializedCache : public TypedArrayCache<ValueType>
+{
+public:
+  SpecializedCache(ArrayT* arr)
+    : Array(arr)
+  {
+  }
+
+  ValueType GetValue(int idx) const override
+  {
+    return static_cast<ValueType>(this->Array->GetValue(idx));
+  }
+
+private:
+  vtkSmartPointer<ArrayT> Array;
+};
+
+/*
+ * An implementation of TypedArrayCache for `vtkDataArray` that acts as a fallback implementation
+ * for arrays whose base type cannot be determined by `vtkArrayDispatch`.
+ */
+template <typename ValueType>
+struct SpecializedCache<ValueType, vtkDataArray> : public TypedArrayCache<ValueType>
+{
+public:
+  SpecializedCache(vtkDataArray* arr)
+    : Array(arr)
+  {
+  }
+
+  ValueType GetValue(int idx) const override
+  {
+    int iTup = idx / this->Array->GetNumberOfComponents();
+    int iComp = idx - iTup * this->Array->GetNumberOfComponents();
+    return static_cast<ValueType>(this->Array->GetComponent(iTup, iComp));
+  }
+
+private:
+  vtkSmartPointer<vtkDataArray> Array;
+};
+
+//-----------------------------------------------------------------------
+/*
+ * A worker structure to be used with `vtkArrayDispatch` in order to cache typed versions of arrays
+ * once for improving random access speed.
+ */
+template <typename ValueType>
+struct CacheDispatchWorker
+{
+  template <typename ArrayT>
+  void operator()(ArrayT* arr, std::shared_ptr<TypedArrayCache<ValueType>>& cache)
+  {
+    cache = std::make_shared<SpecializedCache<ValueType, ArrayT>>(arr);
+  }
+};
+
+//-----------------------------------------------------------------------
+template <typename ArrayList, typename ValueType>
+/*
+ * A structure that wraps around a TypedArrayCache and can serve as a backend to a
+ * `vtkImplicitArray`. Its constructor is what uses dispatches the underlying array into a typed
+ * cache.
+ */
+struct TypedCacheWrapper
+{
+  TypedCacheWrapper(vtkDataArray* arr)
+  {
+    CacheDispatchWorker<ValueType> worker;
+    if (!Dispatcher::Execute(arr, worker, this->Cache))
+    {
+      worker(arr, this->Cache);
+    }
+  }
+
+  ValueType operator()(int idx) const { return this->Cache->GetValue(idx); }
+
+private:
+  using Dispatcher = vtkArrayDispatch::DispatchByArray<ArrayList>;
+  std::shared_ptr<TypedArrayCache<ValueType>> Cache = nullptr;
+};
+}
+
 VTK_ABI_NAMESPACE_BEGIN
 //-----------------------------------------------------------------------
 template <typename ValueType>
 struct vtkCompositeImplicitBackend<ValueType>::Internals
 {
-  Internals(vtkDataArray* leftArr, vtkDataArray* rightArr)
-    : Left(leftArr)
-    , Right(rightArr)
+  using InternalArrayList = vtkArrayDispatch::AllArrays;
+  using CachedBackend = ::TypedCacheWrapper<InternalArrayList, ValueType>;
+  using CachedArray = vtkImplicitArray<CachedBackend>;
+
+  /*
+   * Construct an internal structure from any range of iterators providing a stream of
+   * `vtkDataArray*`s. At construction, every array is dispatched into a cache and the offsets are
+   * calculated to enable fast binary search in `vtkCompositeImplicitBackend::operator()`.
+   */
+  template <class Iterator>
+  Internals(Iterator first, Iterator last)
   {
-    if (!this->Left && !this->Right)
+    this->CachedArrays.resize(std::distance(first, last));
+    std::transform(first, last, this->CachedArrays.begin(), [](vtkDataArray* arr) {
+      vtkNew<CachedArray> newCache;
+      newCache->SetBackend(std::make_shared<CachedBackend>(arr));
+      newCache->SetNumberOfComponents(1);
+      newCache->SetNumberOfTuples(arr->GetNumberOfTuples() * arr->GetNumberOfComponents());
+      return newCache;
+    });
+    if (this->CachedArrays.size() > 0)
     {
-      vtkWarningWithObjectMacro(nullptr, "Creating composite array with two nullptrs");
+      this->Offsets.resize(this->CachedArrays.size() - 1);
+      std::size_t runningSum = 0;
+      std::transform(this->CachedArrays.begin(), this->CachedArrays.end() - 1,
+        this->Offsets.begin(), [&runningSum](CachedArray* arr) {
+          runningSum += arr->GetNumberOfTuples();
+          return runningSum;
+        });
     }
-    auto checkNullRectify = [](vtkSmartPointer<vtkDataArray>& arr) {
-      if (!arr)
-      {
-        arr = vtkSmartPointer<vtkAOSDataArrayTemplate<ValueType>>::New();
-        arr->SetNumberOfComponents(1);
-        arr->SetNumberOfTuples(0);
-      }
-    };
-    checkNullRectify(this->Left);
-    checkNullRectify(this->Right);
-    this->LeftRange = vtk::DataArrayValueRange(this->Left);
-    this->RightRange = vtk::DataArrayValueRange(this->Right);
-    this->Offset = this->LeftRange.size();
   }
 
-  vtkSmartPointer<vtkDataArray> Left;
-  vtk::detail::SelectValueRange<vtkDataArray*, vtk::detail::DynamicTupleSize>::type LeftRange;
-  vtkSmartPointer<vtkDataArray> Right;
-  vtk::detail::SelectValueRange<vtkDataArray*, vtk::detail::DynamicTupleSize>::type RightRange;
-  int Offset = -1;
+  std::vector<vtkSmartPointer<CachedArray>> CachedArrays;
+  std::vector<std::size_t> Offsets;
 };
 
 //-----------------------------------------------------------------------
 template <typename ValueType>
 vtkCompositeImplicitBackend<ValueType>::vtkCompositeImplicitBackend(
-  vtkDataArray* leftArr, vtkDataArray* rightArr)
-  : Internal(std::unique_ptr<Internals>(new Internals(leftArr, rightArr)))
+  const std::vector<vtkDataArray*>& arrays)
+  : Internal(std::unique_ptr<Internals>(new Internals(arrays.begin(), arrays.end())))
 {
 }
 
@@ -72,8 +180,10 @@ vtkCompositeImplicitBackend<ValueType>::~vtkCompositeImplicitBackend() = default
 template <typename ValueType>
 ValueType vtkCompositeImplicitBackend<ValueType>::operator()(int idx) const
 {
-  return static_cast<ValueType>((idx < this->Internal->Offset)
-      ? this->Internal->LeftRange[idx]
-      : this->Internal->RightRange[idx - this->Internal->Offset]);
+  auto itPos =
+    std::upper_bound(this->Internal->Offsets.begin(), this->Internal->Offsets.end(), idx);
+  int locIdx = itPos == this->Internal->Offsets.begin() ? idx : idx - *(itPos - 1);
+  return this->Internal->CachedArrays[std::distance(this->Internal->Offsets.begin(), itPos)]
+    ->GetValue(locIdx);
 }
 VTK_ABI_NAMESPACE_END