Refactor VirtualObjectHandle to support new virtual design

vtkm/cont/ArrayHandleVirtualCoordinates.h

@@ -23,6 +23,7 @@
 #include <vtkm/cont/ArrayHandle.h>
 #include <vtkm/cont/ArrayHandleCartesianProduct.h>
 #include <vtkm/cont/ArrayHandleUniformPointCoordinates.h>
+#include <vtkm/cont/ErrorBadType.h>
 #include <vtkm/cont/Logging.h>
 #include <vtkm/cont/TryExecute.h>
 #include <vtkm/cont/VirtualObjectHandle.h>

vtkm/cont/CMakeLists.txt

@@ -139,6 +139,8 @@ set(sources
   internal/ArrayManagerExecutionShareWithControl.cxx
   internal/DeviceAdapterTag.cxx
   internal/SimplePolymorphicContainer.cxx
+  internal/SimplePolymorphicContainer.cxx
+  internal/VirtualObjectTransfer.cxx
   Logging.cxx
   MultiBlock.cxx
   PresetColorTables.cxx

vtkm/cont/VirtualObjectHandle.h

@@ -21,10 +21,7 @@
 #define vtk_m_cont_VirtualObjectHandle_h
 
 #include <vtkm/cont/DeviceAdapterListTag.h>
-#include <vtkm/cont/ErrorBadType.h>
-#include <vtkm/cont/ErrorBadValue.h>
 #include <vtkm/cont/internal/DeviceAdapterListHelpers.h>
-#include <vtkm/cont/internal/DeviceAdapterTag.h>
 #include <vtkm/cont/internal/VirtualObjectTransfer.h>
 
 #include <array>
@@ -35,6 +32,22 @@ namespace vtkm
 namespace cont
 {
 
+namespace internal
+{
+struct CreateTransferInterface
+{
+  template <typename VirtualDerivedType, typename DeviceAdapter>
+  VTKM_CONT inline void operator()(DeviceAdapter device,
+                                   internal::TransferState* transfers,
+                                   const VirtualDerivedType* virtualObject) const
+  {
+    using TransferImpl = TransferInterfaceImpl<VirtualDerivedType, DeviceAdapter>;
+    auto i = static_cast<std::size_t>(device.GetValue());
+    transfers->DeviceTransferState[i].reset(new TransferImpl(virtualObject));
+  }
+};
+}
+
 /// \brief Implements VTK-m's execution side <em> Virtual Methods </em> functionality.
 ///
 /// The template parameter \c VirtualBaseType is the base class that acts as the
@@ -62,7 +75,7 @@ class VTKM_ALWAYS_EXPORT VirtualObjectHandle
 
 public:
   VTKM_CONT VirtualObjectHandle()
-    : Internals(new InternalStruct)
+    : Internals(std::make_shared<internal::TransferState>())
   {
   }
 
@@ -71,19 +84,23 @@ public:
   VTKM_CONT explicit VirtualObjectHandle(VirtualDerivedType* derived,
                                          bool acquireOwnership = true,
                                          DeviceAdapterList devices = DeviceAdapterList())
-    : Internals(new InternalStruct)
+    : Internals(std::make_shared<internal::TransferState>())
   {
     this->Reset(derived, acquireOwnership, devices);
   }
 
   /// Get if in a valid state (a target is bound)
-  VTKM_CONT bool GetValid() const { return this->Internals->VirtualObject != nullptr; }
+  VTKM_CONT bool GetValid() const { return this->Internals->HostPtr() != nullptr; }
+
 
   /// Get if this handle owns the control side target object
-  VTKM_CONT bool OwnsObject() const { return this->Internals->Owner; }
+  VTKM_CONT bool OwnsObject() const { return this->Internals->WillReleaseHostPointer(); }
 
   /// Get the control side pointer to the virtual object
-  VTKM_CONT VirtualBaseType* Get() const { return this->Internals->VirtualObject; }
+  VTKM_CONT VirtualBaseType* Get() const
+  {
+    return static_cast<VirtualBaseType*>(this->Internals->HostPtr());
+  }
 
   /// Reset the underlying derived type object
   template <typename VirtualDerivedType,
@@ -92,22 +109,28 @@ public:
                        bool acquireOwnership = true,
                        DeviceAdapterList devices = DeviceAdapterList())
   {
-    this->Reset();
+    VTKM_STATIC_ASSERT_MSG((std::is_base_of<VirtualBaseType, VirtualDerivedType>::value),
+                           "Tried to bind a type that is not a subclass of the base class.");
+
+    if (acquireOwnership)
+    {
+      auto deleter = [](void* p) { delete static_cast<VirtualBaseType*>(p); };
+      this->Internals->UpdateHost(derived, deleter);
+    }
+    else
+    {
+      this->Internals->UpdateHost(derived, nullptr);
+    }
+
     if (derived)
     {
-      VTKM_STATIC_ASSERT_MSG((std::is_base_of<VirtualBaseType, VirtualDerivedType>::value),
-                             "Tried to bind a type that is not a subclass of the base class.");
-
-      this->Internals->VirtualObject = derived;
-      this->Internals->Owner = acquireOwnership;
-      vtkm::cont::internal::ForEachValidDevice(devices,
-                                               CreateTransferInterface<VirtualDerivedType>(),
-                                               this->Internals->Transfers.data(),
-                                               derived);
+      vtkm::cont::internal::ForEachValidDevice(
+        devices, internal::CreateTransferInterface(), this->Internals.get(), derived);
     }
   }
 
-  void Reset() { this->Internals->Reset(); }
+  /// Release all host and execution side resources
+  VTKM_CONT void ReleaseResources() { this->Internals->ReleaseResources(); }
 
   /// Release all the execution side resources
   VTKM_CONT void ReleaseExecutionResources() { this->Internals->ReleaseExecutionResources(); }
@@ -121,128 +144,19 @@ public:
   ///
   VTKM_CONT const VirtualBaseType* PrepareForExecution(vtkm::cont::DeviceAdapterId deviceId) const
   {
-    if (!this->GetValid())
-    {
-      throw vtkm::cont::ErrorBadValue("No target object bound");
+    const bool validId = this->Internals->DeviceIdIsValid(deviceId);
+    if (!validId)
+    { //can't be reached since DeviceIdIsValid will through an exception
+      //if deviceId is not valid
+      return nullptr;
     }
 
-    if (!this->Internals->Current || this->Internals->Current->GetDeviceId() != deviceId)
-    {
-      if (!this->Internals->Transfers[static_cast<std::size_t>(deviceId.GetValue())])
-      {
-        std::string msg =
-          "VTK-m was asked to transfer a VirtualObjectHandle for execution on DeviceAdapter '" +
-          deviceId.GetName() + "' (" + std::to_string(deviceId.GetValue()) +
-          "). It can't as this VirtualObjectHandle was not constructed/bound with this "
-          "DeviceAdapter in the list of valid DeviceAdapters.";
-        throw vtkm::cont::ErrorBadDevice(msg);
-      }
-
-      if (this->Internals->Current)
-      {
-        this->Internals->Current->ReleaseResources();
-      }
-      this->Internals->Current =
-        this->Internals->Transfers[static_cast<std::size_t>(deviceId.GetValue())].get();
-    }
-
-    return this->Internals->Current->PrepareForExecution();
+    return static_cast<const VirtualBaseType*>(this->Internals->PrepareForExecution(deviceId));
   }
 
 
 private:
-  class TransferInterface
-  {
-  public:
-    VTKM_CONT virtual ~TransferInterface() = default;
-
-    VTKM_CONT virtual vtkm::cont::DeviceAdapterId GetDeviceId() const = 0;
-    VTKM_CONT virtual const VirtualBaseType* PrepareForExecution() = 0;
-    VTKM_CONT virtual void ReleaseResources() = 0;
-  };
-
-  template <typename VirtualDerivedType, typename DeviceAdapter>
-  class TransferInterfaceImpl : public TransferInterface
-  {
-  public:
-    VTKM_CONT TransferInterfaceImpl(const VirtualDerivedType* virtualObject)
-      : LastModifiedCount(-1)
-      , VirtualObject(virtualObject)
-      , Transfer(virtualObject)
-    {
-    }
-
-    VTKM_CONT vtkm::cont::DeviceAdapterId GetDeviceId() const override { return DeviceAdapter(); }
-
-    VTKM_CONT const VirtualBaseType* PrepareForExecution() override
-    {
-      vtkm::Id modifiedCount = this->VirtualObject->GetModifiedCount();
-      bool updateData = (this->LastModifiedCount != modifiedCount);
-      const VirtualBaseType* executionObject = this->Transfer.PrepareForExecution(updateData);
-      this->LastModifiedCount = modifiedCount;
-      return executionObject;
-    }
-
-    VTKM_CONT void ReleaseResources() override { this->Transfer.ReleaseResources(); }
-
-  private:
-    vtkm::Id LastModifiedCount;
-    const VirtualDerivedType* VirtualObject;
-    vtkm::cont::internal::VirtualObjectTransfer<VirtualDerivedType, DeviceAdapter> Transfer;
-  };
-
-  template <typename VirtualDerivedType>
-  struct CreateTransferInterface
-  {
-    template <typename DeviceAdapter>
-    VTKM_CONT void operator()(DeviceAdapter device,
-                              std::unique_ptr<TransferInterface>* transfers,
-                              const VirtualDerivedType* virtualObject) const
-    {
-      if (!device.IsValueValid())
-      {
-        throwFailedRuntimeDeviceTransfer("VirtualObjectHandle", device);
-      }
-      using TransferImpl = TransferInterfaceImpl<VirtualDerivedType, DeviceAdapter>;
-      transfers[device.GetValue()].reset(new TransferImpl(virtualObject));
-    }
-  };
-
-  struct InternalStruct
-  {
-    VirtualBaseType* VirtualObject = nullptr;
-    bool Owner = false;
-    std::array<std::unique_ptr<TransferInterface>, VTKM_MAX_DEVICE_ADAPTER_ID> Transfers;
-    TransferInterface* Current = nullptr;
-
-    VTKM_CONT void ReleaseExecutionResources()
-    {
-      if (this->Current)
-      {
-        this->Current->ReleaseResources();
-        this->Current = nullptr;
-      }
-    }
-
-    VTKM_CONT void Reset()
-    {
-      this->ReleaseExecutionResources();
-      for (auto& transfer : this->Transfers)
-      {
-        transfer.reset(nullptr);
-      }
-      if (this->Owner)
-      {
-        delete this->VirtualObject;
-      }
-      this->VirtualObject = nullptr;
-      this->Owner = false;
-    }
-
-    VTKM_CONT ~InternalStruct() { this->Reset(); }
-  };
-
-  std::shared_ptr<InternalStruct> Internals;
+  std::shared_ptr<internal::TransferState> Internals;
 };
 }
 } // vtkm::cont

vtkm/cont/VirtualObjectHandle.hxx

+//============================================================================
+//  Copyright (c) Kitware, Inc.
+//  All rights reserved.
+//  See LICENSE.txt for details.
+//  This software is distributed WITHOUT ANY WARRANTY; without even
+//  the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR
+//  PURPOSE.  See the above copyright notice for more information.
+//
+//  Copyright 2017 National Technology & Engineering Solutions of Sandia, LLC (NTESS).
+//  Copyright 2017 UT-Battelle, LLC.
+//  Copyright 2017 Los Alamos National Security.
+//
+//  Under the terms of Contract DE-NA0003525 with NTESS,
+//  the U.S. Government retains certain rights in this software.
+//
+//  Under the terms of Contract DE-AC52-06NA25396 with Los Alamos National
+//  Laboratory (LANL), the U.S. Government retains certain rights in
+//  this software.
+//============================================================================
+#ifndef vtk_m_cont_VirtualObjectHandle_hxx
+#define vtk_m_cont_VirtualObjectHandle_hxx
+
+#include <vtkm/cont/VirtualObjectHandle.h>
+
+#include <vtkm/cont/DeviceAdapterListTag.h>
+#include <vtkm/cont/internal/DeviceAdapterListHelpers.h>
+#include <vtkm/cont/internal/DeviceAdapterTag.h>
+
+#include <vtkm/cont/internal/VirtualObjectTransfer.h>
+#include <vtkm/cont/internal/VirtualObjectTransfer.hxx>
+
+namespace vtkm
+{
+namespace cont
+{
+
+template <typename VirtualBaseType>
+VirtualObjectHandle<VirtualBaseType>::VirtualObjectHandle()
+  : Internals(std::make_shared<internal::TransferState>())
+{
+}
+
+template <typename VirtualBaseType>
+template <typename VirtualDerivedType, typename DeviceAdapterList>
+VirtualObjectHandle<VirtualBaseType>::VirtualObjectHandle(VirtualDerivedType* derived,
+                                                          bool acquireOwnership,
+                                                          DeviceAdapterList devices)
+  : Internals(std::make_shared<internal::TransferState>())
+{
+  this->Reset(derived, acquireOwnership, devices);
+}
+
+template <typename VirtualBaseType>
+bool VirtualObjectHandle<VirtualBaseType>::GetValid() const
+{
+  return this->Internals->HostPtr() != nullptr;
+}
+
+template <typename VirtualBaseType>
+bool VirtualObjectHandle<VirtualBaseType>::OwnsObject() const
+{
+  return this->Internals->WillReleaseHostPointer();
+}
+
+template <typename VirtualBaseType>
+VirtualBaseType* VirtualObjectHandle<VirtualBaseType>::Get()
+{
+  return static_cast<VirtualBaseType*>(this->Internals->HostPtr());
+}
+
+/// Reset the underlying derived type object
+template <typename VirtualBaseType>
+template <typename VirtualDerivedType, typename DeviceAdapterList>
+void VirtualObjectHandle<VirtualBaseType>::Reset(VirtualDerivedType* derived,
+                                                 bool acquireOwnership,
+                                                 DeviceAdapterList devices)
+{
+  VTKM_STATIC_ASSERT_MSG((std::is_base_of<VirtualBaseType, VirtualDerivedType>::value),
+                         "Tried to bind a type that is not a subclass of the base class.");
+
+  if (acquireOwnership)
+  {
+    // auto deleter = [](void* p) { delete static_cast<VirtualBaseType*>(p); };
+    this->Internals->UpdateHost(derived, nullptr);
+  }
+  else
+  {
+    this->Internals->UpdateHost(derived, nullptr);
+  }
+
+  if (derived)
+  {
+    vtkm::cont::internal::ForEachValidDevice(
+      devices, internal::CreateTransferInterface(), this->Internals.get(), derived);
+  }
+}
+
+template <typename VirtualBaseType>
+const VirtualBaseType* VirtualObjectHandle<VirtualBaseType>::PrepareForExecution(
+  vtkm::cont::DeviceAdapterId deviceId) const
+{
+  const bool validId = this->Internals->DeviceIdIsValid(deviceId);
+  if (!validId)
+  { //can't be reached since DeviceIdIsValid will through an exception
+    //if deviceId is not valid
+    return nullptr;
+  }
+
+  return static_cast<const VirtualBaseType*>(this->Internals->PrepareForExecution(deviceId));
+}
+}
+} // vtkm::cont
+
+#endif // vtk_m_cont_VirtualObjectHandle_h

vtkm/cont/internal/VirtualObjectTransfer.cxx

+//============================================================================
+//  Copyright (c) Kitware, Inc.
+//  All rights reserved.
+//  See LICENSE.txt for details.
+//  This software is distributed WITHOUT ANY WARRANTY; without even
+//  the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR
+//  PURPOSE.  See the above copyright notice for more information.
+//
+//  Copyright 2017 National Technology & Engineering Solutions of Sandia, LLC (NTESS).
+//  Copyright 2017 UT-Battelle, LLC.
+//  Copyright 2017 Los Alamos National Security.
+//
+//  Under the terms of Contract DE-NA0003525 with NTESS,
+//  the U.S. Government retains certain rights in this software.
+//
+//  Under the terms of Contract DE-AC52-06NA25396 with Los Alamos National
+//  Laboratory (LANL), the U.S. Government retains certain rights in
+//  this software.
+//============================================================================
+
+#include <vtkm/cont/internal/VirtualObjectTransfer.h>
+
+#include <vtkm/cont/ErrorBadType.h>
+#include <vtkm/cont/ErrorBadValue.h>
+
+#include <array>
+#include <memory>
+
+
+namespace vtkm
+{
+namespace cont
+{
+namespace internal
+{
+
+bool TransferState::DeviceIdIsValid(vtkm::cont::DeviceAdapterId deviceId) const
+{
+  auto index = static_cast<std::size_t>(deviceId.GetValue());
+  auto size = this->DeviceTransferState.size();
+
+  if (!this->HostPointer)
+  {
+    throw vtkm::cont::ErrorBadValue(
+      "No virtual object was bound before being asked to be executed");
+  }
+  if (index >= size)
+  {
+    std::string msg = "An invalid DeviceAdapter[id=" + std::to_string(deviceId.GetValue()) +
+      ", name=" + deviceId.GetName() + "] was used when trying to construct a virtual object.";
+    throw vtkm::cont::ErrorBadType(msg);
+  }
+
+  if (!this->DeviceTransferState[index])
+  {
+    const std::string msg =
+      "VTK-m was asked to transfer a VirtualObjectHandle for execution on DeviceAdapter[id=" +
+      std::to_string(deviceId.GetValue()) + ", name=" + deviceId.GetName() +
+      "]. It can't as this VirtualObjectHandle was not constructed/bound with this "
+      "DeviceAdapter in the list of valid DeviceAdapters.";
+    throw vtkm::cont::ErrorBadType(msg);
+  }
+
+  return true;
+}
+}
+}
+}

vtkm/cont/internal/VirtualObjectTransfer.h

@@ -20,7 +20,13 @@
 #ifndef vtk_m_cont_internal_VirtualObjectTransfer_h
 #define vtk_m_cont_internal_VirtualObjectTransfer_h
 
+#include <vtkm/cont/vtkm_cont_export.h>
+
 #include <vtkm/VirtualObjectBase.h>
+#include <vtkm/cont/internal/DeviceAdapterTag.h>
+
+#include <array>
+#include <memory>
 
 namespace vtkm
 {
@@ -29,6 +35,8 @@ namespace cont
 namespace internal
 {
 
+struct CreateTransferInterface; //forward declare for friendship
+
 template <typename VirtualDerivedType, typename DeviceAdapter>
 struct VirtualObjectTransfer
 #ifdef VTKM_DOXYGEN_ONLY
@@ -57,6 +65,107 @@ struct VirtualObjectTransfer
 }
 #endif
 ;
+
+class TransferInterface
+{
+public:
+  VTKM_CONT virtual ~TransferInterface() = default;
+
+  VTKM_CONT virtual const vtkm::VirtualObjectBase* PrepareForExecution(vtkm::Id) = 0;
+  VTKM_CONT virtual void ReleaseResources() = 0;
+};
+
+template <typename VirtualDerivedType, typename DeviceAdapter>
+class TransferInterfaceImpl final : public TransferInterface
+{
+public:
+  VTKM_CONT TransferInterfaceImpl(const VirtualDerivedType* virtualObject)
+    : LastModifiedCount(-1)
+    , Transfer(virtualObject)
+  {
+  }
+
+  VTKM_CONT const vtkm::VirtualObjectBase* PrepareForExecution(vtkm::Id hostModifiedCount) override
+  {
+    bool updateData = (this->LastModifiedCount != hostModifiedCount);
+    const vtkm::VirtualObjectBase* executionObject = this->Transfer.PrepareForExecution(updateData);
+    this->LastModifiedCount = hostModifiedCount;
+    return executionObject;
+  }
+
+  VTKM_CONT void ReleaseResources() override { this->Transfer.ReleaseResources(); }
+
+private:
+  vtkm::Id LastModifiedCount;
+  vtkm::cont::internal::VirtualObjectTransfer<VirtualDerivedType, DeviceAdapter> Transfer;
+};
+
+
+struct VTKM_CONT_EXPORT TransferState
+{
+  TransferState() = default;
+
+  ~TransferState() { this->ReleaseResources(); }
+
+  bool DeviceIdIsValid(vtkm::cont::DeviceAdapterId deviceId) const;
+
+  bool WillReleaseHostPointer() const { return this->DeleteFunction != nullptr; }
+
+
+  void UpdateHost(vtkm::VirtualObjectBase* host, void (*deleteFunction)(void*))
+  {
+    if (this->HostPointer != host)
+    {
+      this->ReleaseResources();
+      this->HostPointer = host;
+      this->DeleteFunction = deleteFunction;
+    }
+  }
+
+  void ReleaseResources()
+  {
+    this->ReleaseExecutionResources();
+
+    //This needs to be updated to release all execution information
+
+    if (this->DeleteFunction)
+    {
+      this->DeleteFunction(this->HostPointer);
+    }
+    this->HostPointer = nullptr;
+    this->DeleteFunction = nullptr;
+  }
+
+  void ReleaseExecutionResources()
+  {
+    //This needs to be updated to only release the active execution part
+    for (auto& state : this->DeviceTransferState)
+    {
+      if (state)
+      {
+        state->ReleaseResources();
+      }
+    }
+  }
+
+  const vtkm::VirtualObjectBase* PrepareForExecution(vtkm::cont::DeviceAdapterId deviceId) const
+  {
+    //make sure the device is up to date
+    auto index = static_cast<std::size_t>(deviceId.GetValue());
+    vtkm::Id count = this->HostPointer->GetModifiedCount();
+    return this->DeviceTransferState[index]->PrepareForExecution(count);
+  }
+
+  vtkm::VirtualObjectBase* HostPtr() const { return this->HostPointer; }
+
+private:
+  friend struct CreateTransferInterface;
+
+  vtkm::VirtualObjectBase* HostPointer = nullptr;
+  void (*DeleteFunction)(void*) = nullptr;
+
+  std::array<std::unique_ptr<TransferInterface>, 8> DeviceTransferState;
+};
 }
 }
 } // vtkm::cont::internal

vtkm/cont/openmp/internal/VirtualObjectTransferOpenMP.h

@@ -33,13 +33,11 @@ namespace internal
 {
 
 template <typename VirtualDerivedType>
-struct VirtualObjectTransfer<VirtualDerivedType, vtkm::cont::DeviceAdapterTagOpenMP>
+struct VirtualObjectTransfer<VirtualDerivedType, vtkm::cont::DeviceAdapterTagOpenMP> final
   : VirtualObjectTransferShareWithControl<VirtualDerivedType>
 {
-  VTKM_CONT VirtualObjectTransfer(const VirtualDerivedType* virtualObject)
-    : VirtualObjectTransferShareWithControl<VirtualDerivedType>(virtualObject)
-  {
-  }
+  using VirtualObjectTransferShareWithControl<
+    VirtualDerivedType>::VirtualObjectTransferShareWithControl;
 };
 }
 }

vtkm/cont/serial/internal/VirtualObjectTransferSerial.h

@@ -32,13 +32,11 @@ namespace internal
 {
 
 template <typename VirtualDerivedType>
-struct VirtualObjectTransfer<VirtualDerivedType, vtkm::cont::DeviceAdapterTagSerial>
+struct VirtualObjectTransfer<VirtualDerivedType, vtkm::cont::DeviceAdapterTagSerial> final
   : VirtualObjectTransferShareWithControl<VirtualDerivedType>
 {
-  VTKM_CONT VirtualObjectTransfer(const VirtualDerivedType* virtualObject)
-    : VirtualObjectTransferShareWithControl<VirtualDerivedType>(virtualObject)
-  {
-  }
+  using VirtualObjectTransferShareWithControl<
+    VirtualDerivedType>::VirtualObjectTransferShareWithControl;