Shuenhoy/vtk-m
1
0
Fork 0
mirror of https://gitlab.kitware.com/vtk/vtk-m synced 2024-09-19 10:35:42 +00:00
Code Issues 2 Actions Packages Projects Releases Wiki Activity

Fix issues with ArrayCopy

Browse Source 
C++ was not resolving the overloads of `ArrayCopyImpl` as expected,
which was causing  `ArrayCopy` to sometimes use a less efficient method
for copying.

Also fix an issue with `Buffer::DeepCopy` that caused a deadlock when
copying a buffer that was not actually allocated anywhere (as well as
failing to copy the metadata, which was probably the whole point).
This commit is contained in:
Kenneth Moreland 2020-08-26 08:00:16 -06:00
parent c907ce6419
commit 8298d33f52
2 changed files with 80 additions and 39 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

44
vtkm/cont/ArrayCopy.h
View File

@ -32,10 +32,10 @@ namespace detail
// Element-wise copy.
// TODO: Remove last argument once ArryHandleNewStyle becomes ArrayHandle
template <typename InArrayType, typename OutArrayType>
void ArrayCopyImpl(const InArrayType& in, OutArrayType& out, std::false_type /* Copy storage */)
void ArrayCopyWithAlgorithm(const InArrayType& source, OutArrayType& destination)
{
// Find the device that already has a copy of the data:
vtkm::cont::DeviceAdapterId devId = in.GetDeviceAdapterId();
vtkm::cont::DeviceAdapterId devId = source.GetDeviceAdapterId();
// If the data is not on any device, let the runtime tracker pick an available
// parallel copy algorithm.
@ -44,14 +44,14 @@ void ArrayCopyImpl(const InArrayType& in, OutArrayType& out, std::false_type /*
devId = vtkm::cont::make_DeviceAdapterId(VTKM_DEVICE_ADAPTER_ANY);
}
bool success = vtkm::cont::Algorithm::Copy(devId, in, out);
bool success = vtkm::cont::Algorithm::Copy(devId, source, destination);
if (!success && devId.GetValue() != VTKM_DEVICE_ADAPTER_ANY)
{ // Retry on any device if the first attempt failed.
VTKM_LOG_S(vtkm::cont::LogLevel::Error,
"Failed to run ArrayCopy on device '" << devId.GetName()
<< "'. Retrying on any device.");
success = vtkm::cont::Algorithm::Copy(vtkm::cont::DeviceAdapterTagAny{}, in, out);
success = vtkm::cont::Algorithm::Copy(vtkm::cont::DeviceAdapterTagAny{}, source, destination);
}
if (!success)
@ -60,10 +60,17 @@ void ArrayCopyImpl(const InArrayType& in, OutArrayType& out, std::false_type /*
}
}
// TODO: Remove last argument once ArryHandleNewStyle becomes ArrayHandle
template <typename InArrayType, typename OutArrayType>
void ArrayCopyOldImpl(const InArrayType& in, OutArrayType& out, std::false_type /* Copy storage */)
{
ArrayCopyWithAlgorithm(in, out);
}
// Copy storage for implicit arrays, must be of same type:
// TODO: This will go away once ArrayHandleNewStyle becomes ArrayHandle.
template <typename ArrayType>
void ArrayCopyImpl(const ArrayType& in, ArrayType& out, std::true_type /* Copy storage */)
void ArrayCopyOldImpl(const ArrayType& in, ArrayType& out, std::true_type /* Copy storage */)
{
// This is only called if in/out are the same type and the handle is not
// writable. This allows read-only implicit array handles to be copied.
@ -73,19 +80,33 @@ void ArrayCopyImpl(const ArrayType& in, ArrayType& out, std::true_type /* Copy s
// TODO: This will go away once ArrayHandleNewStyle becomes ArrayHandle.
template <typename InArrayType, typename OutArrayType>
VTKM_CONT void ArrayCopyImpl(const InArrayType& source, OutArrayType& destination)
VTKM_CONT void ArrayCopyImpl(const InArrayType& source,
OutArrayType& destination,
std::false_type /* New style */)
{
using SameTypes = std::is_same<InArrayType, OutArrayType>;
using IsWritable = vtkm::cont::internal::IsWritableArrayHandle<OutArrayType>;
using JustCopyStorage = std::integral_constant<bool, SameTypes::value && !IsWritable::value>;
ArrayCopyImpl(source, destination, JustCopyStorage{});
ArrayCopyOldImpl(source, destination, JustCopyStorage{});
}
// TODO: ArrayHandleNewStyle will eventually become ArrayHandle, in which case this
// will become ArrayCopyWithAlgorithm
template <typename T1, typename S1, typename T2, typename S2>
VTKM_CONT void ArrayCopyImpl(const vtkm::cont::ArrayHandle<T1, S1>& source,
vtkm::cont::ArrayHandle<T2, S2>& destination,
std::true_type /* New style */)
{
VTKM_STATIC_ASSERT((!std::is_same<T1, T2>::value || !std::is_same<S1, S2>::value));
ArrayCopyWithAlgorithm(source, destination);
}
// TODO: ArrayHandleNewStyle will eventually become ArrayHandle, in which case this
// will become the only version with the same array types.
template <typename T, typename S>
VTKM_CONT void ArrayCopyImpl(const vtkm::cont::ArrayHandleNewStyle<T, S>& source,
vtkm::cont::ArrayHandleNewStyle<T, S>& destination)
VTKM_CONT void ArrayCopyImpl(const vtkm::cont::ArrayHandle<T, S>& source,
vtkm::cont::ArrayHandle<T, S>& destination,
std::true_type /* New style */)
{
destination = source.DeepCopy();
}
@ -128,8 +149,11 @@ VTKM_CONT void ArrayCopy(const vtkm::cont::ArrayHandle<InValueType, InStorage>&
"Cannot copy to a read-only array with a different "
"type than the source.");
using IsNewStyle =
std::is_base_of<vtkm::cont::ArrayHandleNewStyle<InValueType, InStorage>, InArrayType>;
// Static dispatch cases 1 & 2
detail::ArrayCopyImpl(source, destination);
detail::ArrayCopyImpl(source, destination, std::integral_constant<bool, IsNewStyle::value>{});
}
// Forward declaration

75
vtkm/cont/internal/Buffer.cxx
View File

@ -322,6 +322,42 @@ struct VTKM_NEVER_EXPORT BufferHelper
}
}
static void SetNumberOfBytes(const std::shared_ptr<Buffer::InternalsStruct>& internals,
std::unique_lock<std::mutex>& lock,
vtkm::BufferSizeType numberOfBytes,
vtkm::CopyFlag preserve,
vtkm::cont::Token& token)
{
VTKM_ASSUME(numberOfBytes >= 0);
if (internals->GetNumberOfBytes(lock) == numberOfBytes)
{
// Allocation has not changed. Just return.
// Note, if you set the size to the old size and then try to get the buffer on a different
// place, a copy might happen.
return;
}
// We are altering the array, so make sure we can write to it.
BufferHelper::WaitToWrite(internals, lock, token);
internals->SetNumberOfBytes(lock, numberOfBytes);
if ((preserve == vtkm::CopyFlag::Off) || (numberOfBytes == 0))
{
// No longer need these buffers. Just release them.
internals->GetHostBuffer(lock).Release();
for (auto&& deviceBuffer : internals->GetDeviceBuffers(lock))
{
deviceBuffer.second.Release();
}
}
else
{
// Do nothing (other than resetting numberOfBytes). Buffers will get resized when you get the
// pointer.
}
}
static void AllocateOnHost(const std::shared_ptr<Buffer::InternalsStruct>& internals,
std::unique_lock<std::mutex>& lock,
vtkm::cont::Token& token,
@ -597,35 +633,8 @@ void Buffer::SetNumberOfBytes(vtkm::BufferSizeType numberOfBytes,
vtkm::CopyFlag preserve,
vtkm::cont::Token& token)
{
VTKM_ASSUME(numberOfBytes >= 0);
LockType lock = this->Internals->GetLock();
if (this->Internals->GetNumberOfBytes(lock) == numberOfBytes)
{
// Allocation has not changed. Just return.
// Note, if you set the size to the old size and then try to get the buffer on a different
// place, a copy might happen.
return;
}
// We are altering the array, so make sure we can write to it.
detail::BufferHelper::WaitToWrite(this->Internals, lock, token);
this->Internals->SetNumberOfBytes(lock, numberOfBytes);
if ((preserve == vtkm::CopyFlag::Off) || (numberOfBytes == 0))
{
// No longer need these buffers. Just release them.
this->Internals->GetHostBuffer(lock).Release();
for (auto&& deviceBuffer : this->Internals->GetDeviceBuffers(lock))
{
deviceBuffer.second.Release();
}
}
else
{
// Do nothing (other than resetting numberOfBytes). Buffers will get resized when you get the
// pointer.
}
detail::BufferHelper::SetNumberOfBytes(this->Internals, lock, numberOfBytes, preserve, token);
}
vtkm::cont::internal::BufferMetaData* Buffer::GetMetaData() const
@ -790,7 +799,15 @@ void Buffer::DeepCopy(vtkm::cont::internal::Buffer& dest) const
else
{
// Nothing actually allocated. Just create allocation for dest. (Allocation happens lazily.)
dest.SetNumberOfBytes(this->GetNumberOfBytes(), vtkm::CopyFlag::Off, token);
detail::BufferHelper::SetNumberOfBytes(dest.Internals,
destLock,
this->Internals->GetNumberOfBytes(srcLock),
vtkm::CopyFlag::Off,
token);
if (this->Internals->MetaData)
{
dest.Internals->MetaData = this->Internals->MetaData->DeepCopy();
}
}
}
}