Allow VTK-m Buffer to have ownership transferred

docs/changelog/buffer-memory-ownership.md

@@ -0,0 +1,29 @@
+# `vtkm::cont::internal::Buffer` now can have ownership transferred
+
+Memory once transferred to `Buffer` always had to be managed by VTK-m. This is problematic
+for applications that needed VTK-m to allocate memory, but have the memory ownership
+be longer than VTK-m.
+
+`Buffer::TakeHostBufferOwnership` allows for easy transfer ownership of memory out of VTK-m.
+When taking ownership of an VTK-m buffer you are provided the following information:
+
+- Memory: A `void*` pointer to the array
+- Container: A `void*` pointer used to free the memory. This is necessary to support cases such as allocations transferred into VTK-m from a `std::vector`.
+- Delete: The function to call to actually delete the transferred memory
+- Reallocate: The function to call to re-allocate the transferred memory. This will throw an exception if users try
+to reallocate a buffer that was 'view' only
+- Size: The size in number of elements of the array 
+
+ 
+To properly steal memory from VTK-m you do the following:
+```cpp
+  vtkm::cont::ArrayHandle<T> arrayHandle;
+
+  ...
+
+  auto stolen = arrayHandle.GetBuffers()->TakeHostBufferOwnership();
+    
+  ...
+
+  stolen.Delete(stolen.Container);
+```

vtkm/cont/ArrayHandleBasic.cxx

@@ -37,11 +37,6 @@ vtkm::BufferSizeType NumberOfBytes(vtkm::Id numValues, std::size_t typeSize)
 
 } // namespace detail
 
-VTKM_CONT void InvalidRealloc(void*&, void*&, vtkm::BufferSizeType, vtkm::BufferSizeType)
-{
-  vtkm::cont::ErrorBadAllocation("User provided memory does not have a reallocater.");
-}
-
 #define VTKM_STORAGE_INSTANTIATE(Type)                                                             \
   template class VTKM_CONT_EXPORT Storage<Type, StorageTagBasic>;                                  \
   template class VTKM_CONT_EXPORT Storage<vtkm::Vec<Type, 2>, StorageTagBasic>;                    \

vtkm/cont/ArrayHandleBasic.h

@@ -35,11 +35,6 @@ VTKM_CONT_EXPORT VTKM_CONT vtkm::BufferSizeType NumberOfBytes(vtkm::Id numValues
 
 } // namespace detail
 
-VTKM_CONT_EXPORT VTKM_CONT void InvalidRealloc(void*&,
-                                               void*&,
-                                               vtkm::BufferSizeType,
-                                               vtkm::BufferSizeType);
-
 template <typename T>
 class VTKM_ALWAYS_EXPORT Storage<T, vtkm::cont::StorageTagBasic>
 {
@@ -270,79 +265,6 @@ public:
   /// @}
 };
 
-namespace internal
-{
-
-// Deletes a container object by casting it to a pointer of a given type (the template argument)
-// and then using delete[] on the object.
-template <typename T>
-VTKM_CONT inline void SimpleArrayDeleter(void* container_)
-{
-  T* container = reinterpret_cast<T*>(container_);
-  delete[] container;
-}
-
-// Reallocates a standard C array. Note that the allocation method is different than the default
-// host allocation of vtkm::cont::internal::BufferInfo and may be less efficient.
-template <typename T>
-VTKM_CONT inline void SimpleArrayReallocater(void*& memory,
-                                             void*& container,
-                                             vtkm::BufferSizeType oldSize,
-                                             vtkm::BufferSizeType newSize)
-{
-  VTKM_ASSERT(memory == container);
-  VTKM_ASSERT(static_cast<std::size_t>(newSize) % sizeof(T) == 0);
-
-  // If the new size is not much smaller than the old size, just reuse the buffer (and waste a
-  // little memory).
-  if ((newSize > ((3 * oldSize) / 4)) && (newSize <= oldSize))
-  {
-    return;
-  }
-
-  void* newBuffer = new T[static_cast<std::size_t>(newSize) / sizeof(T)];
-  std::memcpy(newBuffer, memory, static_cast<std::size_t>(newSize < oldSize ? newSize : oldSize));
-
-  if (memory != nullptr)
-  {
-    SimpleArrayDeleter<T>(memory);
-  }
-
-  memory = container = newBuffer;
-}
-
-// Deletes a container object by casting it to a pointer of a given type (the template argument)
-// and then using delete on the object.
-template <typename T>
-VTKM_CONT inline void CastDeleter(void* container_)
-{
-  T* container = reinterpret_cast<T*>(container_);
-  delete container;
-}
-
-template <typename T, typename Allocator>
-VTKM_CONT inline void StdVectorDeleter(void* container)
-{
-  CastDeleter<std::vector<T, Allocator>>(container);
-}
-
-template <typename T, typename Allocator>
-VTKM_CONT inline void StdVectorReallocater(void*& memory,
-                                           void*& container,
-                                           vtkm::BufferSizeType oldSize,
-                                           vtkm::BufferSizeType newSize)
-{
-  using vector_type = std::vector<T, Allocator>;
-  vector_type* vector = reinterpret_cast<vector_type*>(container);
-  VTKM_ASSERT(vector->empty() || (memory == vector->data()));
-  VTKM_ASSERT(oldSize == static_cast<vtkm::BufferSizeType>(vector->size()));
-
-  vector->resize(static_cast<std::size_t>(newSize));
-  memory = vector->data();
-}
-
-} // namespace internal
-
 /// A convenience function for creating an ArrayHandle from a standard C array.
 ///
 template <typename T>

vtkm/cont/internal/Buffer.cxx

@@ -799,6 +799,51 @@ vtkm::cont::internal::BufferInfo Buffer::GetHostBufferInfo() const
   return this->Internals->GetHostBuffer(lock);
 }
 
+vtkm::cont::internal::TransferredBuffer Buffer::TakeHostBufferOwnership()
+{
+  // A Token should not be declared within the scope of a lock. when the token goes out of scope
+  // it will attempt to acquire the lock, which is undefined behavior of the thread already has
+  // the lock.
+  vtkm::cont::Token token;
+  {
+    LockType lock = this->Internals->GetLock();
+    detail::BufferHelper::AllocateOnHost(
+      this->Internals, lock, token, detail::BufferHelper::AccessMode::READ);
+    auto& buffer = this->Internals->GetHostBuffer(lock);
+    buffer.Pinned = true;
+    return buffer.Info.TransferOwnership();
+  }
+}
+
+vtkm::cont::internal::TransferredBuffer Buffer::TakeDeviceBufferOwnership(
+  vtkm::cont::DeviceAdapterId device)
+{
+  if (device.IsValueValid())
+  {
+    // A Token should not be declared within the scope of a lock. when the token goes out of scope
+    // it will attempt to acquire the lock, which is undefined behavior of the thread already has
+    // the lock.
+    vtkm::cont::Token token;
+    {
+      LockType lock = this->Internals->GetLock();
+      detail::BufferHelper::AllocateOnDevice(
+        this->Internals, lock, token, device, detail::BufferHelper::AccessMode::READ);
+      auto& buffer = this->Internals->GetDeviceBuffers(lock)[device];
+      buffer.Pinned = true;
+      return buffer.Info.TransferOwnership();
+    }
+  }
+  else if (device == vtkm::cont::DeviceAdapterTagUndefined{})
+  {
+    return this->TakeHostBufferOwnership();
+  }
+  else
+  {
+    throw vtkm::cont::ErrorBadDevice(
+      "Called Buffer::TakeDeviceBufferOwnership with invalid device");
+  }
+}
+
 vtkm::cont::internal::BufferInfo Buffer::GetDeviceBufferInfo(
   vtkm::cont::DeviceAdapterId device) const
 {

vtkm/cont/internal/Buffer.h

@@ -183,6 +183,17 @@ public:
   VTKM_CONT vtkm::cont::internal::BufferInfo GetDeviceBufferInfo(
     vtkm::cont::DeviceAdapterId device) const;
 
+  /// \brief Transfer ownership of the Host `BufferInfo` from this buffer
+  /// to the caller. This is used to allow memory owned by VTK-m to be
+  /// transferred to an owner whose lifespan is longer
+  VTKM_CONT vtkm::cont::internal::TransferredBuffer TakeHostBufferOwnership();
+
+  /// \brief Transfer ownership of the device `BufferInfo` from this buffer
+  /// to the caller. This is used to allow memory owned by VTK-m to be
+  /// transferred to an owner whose lifespan is longer
+  VTKM_CONT vtkm::cont::internal::TransferredBuffer TakeDeviceBufferOwnership(
+    vtkm::cont::DeviceAdapterId device);
+
   VTKM_CONT bool operator==(const vtkm::cont::internal::Buffer& rhs) const
   {
     return (this->Internals == rhs.Internals);

vtkm/cont/internal/DeviceAdapterMemoryManager.cxx

@@ -8,6 +8,7 @@
 //  PURPOSE.  See the above copyright notice for more information.
 //============================================================================
 
+#include <vtkm/cont/ErrorBadAllocation.h>
 #include <vtkm/cont/internal/DeviceAdapterMemoryManager.h>
 
 #include <vtkm/Math.h>
@@ -128,6 +129,12 @@ namespace cont
 namespace internal
 {
 
+VTKM_CONT void InvalidRealloc(void*&, void*&, vtkm::BufferSizeType, vtkm::BufferSizeType)
+{
+  vtkm::cont::ErrorBadAllocation("User provided memory does not have a reallocater.");
+}
+
+
 namespace detail
 {
 
@@ -288,6 +295,22 @@ void BufferInfo::Reallocate(vtkm::BufferSizeType newSize)
   this->Internals->Size = newSize;
 }
 
+TransferredBuffer BufferInfo::TransferOwnership()
+{
+  TransferredBuffer tbufffer = { this->Internals->Memory,
+                                 this->Internals->Container,
+                                 this->Internals->Delete,
+                                 this->Internals->Reallocate,
+                                 this->Internals->Size };
+
+  this->Internals->Delete = [](void*) {};
+  this->Internals->Reallocate = vtkm::cont::internal::InvalidRealloc;
+
+  return tbufffer;
+}
+
+
+
 //----------------------------------------------------------------------------------------
 vtkm::cont::internal::BufferInfo AllocateOnHost(vtkm::BufferSizeType size)
 {

vtkm/cont/internal/DeviceAdapterMemoryManager.h

@@ -17,7 +17,9 @@
 
 #include <vtkm/cont/DeviceAdapterTag.h>
 
+#include <cstring>
 #include <memory>
+#include <vector>
 
 namespace vtkm
 {
@@ -29,6 +31,8 @@ namespace cont
 namespace internal
 {
 
+struct TransferredBuffer;
+
 namespace detail
 {
 
@@ -97,11 +101,38 @@ public:
   ///
   VTKM_CONT void Reallocate(vtkm::BufferSizeType newSize);
 
+  /// Transfers ownership of the underlying allocation and Deleter and Reallocater to the caller.
+  /// After ownership has been transferred this buffer will be equivalant to one that was passed
+  /// to VTK-m as `view` only.
+  ///
+  /// This means that the Deleter will do nothing, and the Reallocater will throw an `ErrorBadAllocation`.
+  ///
+  VTKM_CONT TransferredBuffer TransferOwnership();
+
 private:
   detail::BufferInfoInternals* Internals;
   vtkm::cont::DeviceAdapterId Device;
 };
 
+
+/// Represents the buffer being transferred to external ownership
+///
+/// The Memory pointer represents the actual data allocation to
+/// be used for access and execution
+///
+/// The container represents what needs to be deleted. This might
+/// not be equivalent to \c Memory when we have transferred things
+/// such as std::vector
+///
+struct TransferredBuffer
+{
+  void* Memory;
+  void* Container;
+  BufferInfo::Deleter* Delete;
+  BufferInfo::Reallocater* Reallocate;
+  vtkm::BufferSizeType Size;
+};
+
 /// Allocates a `BufferInfo` object for the host.
 ///
 VTKM_CONT_EXPORT VTKM_CONT vtkm::cont::internal::BufferInfo AllocateOnHost(
@@ -175,6 +206,79 @@ public:
 ///
 template <typename DeviceAdapterTag>
 class DeviceAdapterMemoryManager;
+
+VTKM_CONT_EXPORT VTKM_CONT void InvalidRealloc(void*&,
+                                               void*&,
+                                               vtkm::BufferSizeType,
+                                               vtkm::BufferSizeType);
+
+// Deletes a container object by casting it to a pointer of a given type (the template argument)
+// and then using delete[] on the object.
+template <typename T>
+VTKM_CONT inline void SimpleArrayDeleter(void* container_)
+{
+  T* container = reinterpret_cast<T*>(container_);
+  delete[] container;
+}
+
+// Reallocates a standard C array. Note that the allocation method is different than the default
+// host allocation of vtkm::cont::internal::BufferInfo and may be less efficient.
+template <typename T>
+VTKM_CONT inline void SimpleArrayReallocater(void*& memory,
+                                             void*& container,
+                                             vtkm::BufferSizeType oldSize,
+                                             vtkm::BufferSizeType newSize)
+{
+  VTKM_ASSERT(memory == container);
+  VTKM_ASSERT(static_cast<std::size_t>(newSize) % sizeof(T) == 0);
+
+  // If the new size is not much smaller than the old size, just reuse the buffer (and waste a
+  // little memory).
+  if ((newSize > ((3 * oldSize) / 4)) && (newSize <= oldSize))
+  {
+    return;
+  }
+
+  void* newBuffer = new T[static_cast<std::size_t>(newSize) / sizeof(T)];
+  std::memcpy(newBuffer, memory, static_cast<std::size_t>(newSize < oldSize ? newSize : oldSize));
+
+  if (memory != nullptr)
+  {
+    SimpleArrayDeleter<T>(memory);
+  }
+
+  memory = container = newBuffer;
+}
+
+// Deletes a container object by casting it to a pointer of a given type (the template argument)
+// and then using delete on the object.
+template <typename T>
+VTKM_CONT inline void CastDeleter(void* container_)
+{
+  T* container = reinterpret_cast<T*>(container_);
+  delete container;
+}
+
+template <typename T, typename Allocator>
+VTKM_CONT inline void StdVectorDeleter(void* container)
+{
+  CastDeleter<std::vector<T, Allocator>>(container);
+}
+
+template <typename T, typename Allocator>
+VTKM_CONT inline void StdVectorReallocater(void*& memory,
+                                           void*& container,
+                                           vtkm::BufferSizeType oldSize,
+                                           vtkm::BufferSizeType newSize)
+{
+  using vector_type = std::vector<T, Allocator>;
+  vector_type* vector = reinterpret_cast<vector_type*>(container);
+  VTKM_ASSERT(vector->empty() || (memory == vector->data()));
+  VTKM_ASSERT(oldSize == static_cast<vtkm::BufferSizeType>(vector->size()));
+
+  vector->resize(static_cast<std::size_t>(newSize));
+  memory = vector->data();
+}
 }
 }
 } // namespace vtkm::cont::internal

vtkm/cont/testing/TestingArrayHandles.h

@@ -569,6 +569,73 @@ private:
     }
   };
 
+  struct VerifyVTKMTransferredOwnership
+  {
+    template <typename T>
+    VTKM_CONT void operator()(T) const
+    {
+
+      vtkm::cont::internal::TransferredBuffer transferredMemory;
+
+      //Steal memory from a handle that has multiple copies to verify all
+      //copies are updated correctly
+      {
+        vtkm::cont::ArrayHandle<T> arrayHandle;
+        auto copyOfHandle = arrayHandle;
+
+        VTKM_TEST_ASSERT(arrayHandle.GetNumberOfValues() == 0,
+                         "ArrayHandle has wrong number of entries.");
+        {
+          vtkm::cont::Token token;
+          using ExecutionPortalType =
+            typename vtkm::cont::ArrayHandle<T>::template ExecutionTypes<DeviceAdapterTag>::Portal;
+          ExecutionPortalType executionPortal =
+            arrayHandle.PrepareForOutput(ARRAY_SIZE * 2, DeviceAdapterTag(), token);
+
+          //we drop down to manually scheduling so that we don't need
+          //need to bring in array handle counting
+          AssignTestValue<T, ExecutionPortalType> functor(executionPortal);
+          Algorithm::Schedule(functor, ARRAY_SIZE * 2);
+        }
+
+        transferredMemory = copyOfHandle.GetBuffers()->TakeHostBufferOwnership();
+
+        VTKM_TEST_ASSERT(copyOfHandle.GetNumberOfValues() == ARRAY_SIZE * 2,
+                         "Array not allocated correctly.");
+        array_handle_testing::CheckArray(arrayHandle);
+
+        std::cout << "Try in place operation." << std::endl;
+        {
+          vtkm::cont::Token token;
+          using ExecutionPortalType =
+            typename vtkm::cont::ArrayHandle<T>::template ExecutionTypes<DeviceAdapterTag>::Portal;
+
+          // Reset array data.
+          Algorithm::Schedule(AssignTestValue<T, ExecutionPortalType>{ arrayHandle.PrepareForOutput(
+                                ARRAY_SIZE * 2, DeviceAdapterTag{}, token) },
+                              ARRAY_SIZE * 2);
+
+          ExecutionPortalType executionPortal =
+            arrayHandle.PrepareForInPlace(DeviceAdapterTag(), token);
+
+          //in place can't be done through the dispatcher
+          //instead we have to drop down to manually scheduling
+          InplaceFunctor<T, ExecutionPortalType> functor(executionPortal);
+          Algorithm::Schedule(functor, ARRAY_SIZE * 2);
+        }
+        typename vtkm::cont::ArrayHandle<T>::ReadPortalType controlPortal =
+          arrayHandle.ReadPortal();
+        for (vtkm::Id index = 0; index < ARRAY_SIZE; index++)
+        {
+          VTKM_TEST_ASSERT(test_equal(controlPortal.Get(index), TestValue(index, T()) + T(1)),
+                           "Did not get result from in place operation.");
+        }
+      }
+
+      transferredMemory.Delete(transferredMemory.Container);
+    }
+  };
+
   struct VerifyEqualityOperators
   {
     template <typename T>
@@ -641,6 +708,7 @@ private:
       vtkm::testing::Testing::TryTypes(VerifyVectorMovedMemory{});
       vtkm::testing::Testing::TryTypes(VerifyInitializerList{});
       vtkm::testing::Testing::TryTypes(VerifyVTKMAllocatedHandle{});
+      vtkm::testing::Testing::TryTypes(VerifyVTKMTransferredOwnership{});
       vtkm::testing::Testing::TryTypes(VerifyEqualityOperators{});
     }
   };