Commit 707970f4 authored by Robert Maynard's avatar Robert Maynard

VTK-m StorageBasic is now able to give/take ownership of user allocated memory.

This fixes the three following issues with StorageBasic.

1. Memory that was allocated by VTK-m and Stolen by the user needed the
proper free function called which is generally StorageBasicAllocator::deallocate.
But that was hard for the user to hold onto. So now we provide a function
pointer to the correct free function.

2. Memory that was allocated outside of VTK-m was impossible to transfer to
VTK-m as we didn't know how to free it. This is now resolved by allowing the
user to specify a free function to be called on release.

3. When the CUDA backend allocates memory for an ArrayHandle that has no
control representation, and the location we are running on supports concurrent
managed access we want to specify that cuda managed memory as also the host memory.
This requires that StorageBasic be able to call an arbitrary new delete function
which is chosen at runtime.
parent eca65146
......@@ -52,6 +52,19 @@ namespace cont
namespace internal
{
void free_memory(void* mem)
{
#if defined(VTKM_MEMALIGN_POSIX)
free(mem);
#elif defined(VTKM_MEMALIGN_WIN)
_aligned_free(mem);
#elif defined(VTKM_MEMALIGN_SSE)
_mm_free(mem);
#else
free(mem);
#endif
}
void* StorageBasicAllocator::allocate(size_t size, size_t align)
{
#if defined(VTKM_MEMALIGN_POSIX)
......@@ -70,25 +83,11 @@ void* StorageBasicAllocator::allocate(size_t size, size_t align)
return mem;
}
void StorageBasicAllocator::free_memory(void* mem)
{
#if defined(VTKM_MEMALIGN_POSIX)
free(mem);
#elif defined(VTKM_MEMALIGN_WIN)
_aligned_free(mem);
#elif defined(VTKM_MEMALIGN_SSE)
_mm_free(mem);
#else
free(mem);
#endif
}
StorageBasicBase::StorageBasicBase()
: Array(nullptr)
, AllocatedByteSize(0)
, NumberOfValues(0)
, DeallocateOnRelease(true)
, DeleteFunction(internal::free_memory)
{
}
......@@ -98,7 +97,18 @@ StorageBasicBase::StorageBasicBase(const void* array,
: Array(const_cast<void*>(array))
, AllocatedByteSize(static_cast<vtkm::UInt64>(numberOfValues) * sizeOfValue)
, NumberOfValues(numberOfValues)
, DeallocateOnRelease(array == nullptr ? true : false)
, DeleteFunction(array == nullptr ? internal::free_memory : nullptr)
{
}
StorageBasicBase::StorageBasicBase(const void* array,
vtkm::Id numberOfValues,
vtkm::UInt64 sizeOfValue,
void (*deleteFunction)(void*))
: Array(const_cast<void*>(array))
, AllocatedByteSize(static_cast<vtkm::UInt64>(numberOfValues) * sizeOfValue)
, NumberOfValues(numberOfValues)
, DeleteFunction(deleteFunction)
{
}
......@@ -111,10 +121,10 @@ StorageBasicBase::StorageBasicBase(const StorageBasicBase& src)
: Array(src.Array)
, AllocatedByteSize(src.AllocatedByteSize)
, NumberOfValues(src.NumberOfValues)
, DeallocateOnRelease(src.DeallocateOnRelease)
, DeleteFunction(src.DeleteFunction)
{
if (src.DeallocateOnRelease)
if (src.DeleteFunction)
{
throw vtkm::cont::ErrorBadValue(
"Attempted to copy a storage array that needs deallocation. "
......@@ -124,7 +134,7 @@ StorageBasicBase::StorageBasicBase(const StorageBasicBase& src)
StorageBasicBase StorageBasicBase::operator=(const StorageBasicBase& src)
{
if (src.DeallocateOnRelease)
if (src.DeleteFunction)
{
throw vtkm::cont::ErrorBadValue(
"Attempted to copy a storage array that needs deallocation. "
......@@ -135,29 +145,10 @@ StorageBasicBase StorageBasicBase::operator=(const StorageBasicBase& src)
this->Array = src.Array;
this->AllocatedByteSize = src.AllocatedByteSize;
this->NumberOfValues = src.NumberOfValues;
this->DeallocateOnRelease = src.DeallocateOnRelease;
this->DeleteFunction = src.DeleteFunction;
return *this;
}
void StorageBasicBase::ReleaseResources()
{
if (this->AllocatedByteSize > 0)
{
VTKM_ASSERT(this->Array != nullptr);
if (this->DeallocateOnRelease)
{
AllocatorType{}.free_memory(this->Array);
}
this->Array = nullptr;
this->AllocatedByteSize = 0;
this->NumberOfValues = 0;
}
else
{
VTKM_ASSERT(this->Array == nullptr);
}
}
void StorageBasicBase::AllocateValues(vtkm::Id numberOfValues, vtkm::UInt64 sizeOfValue)
{
if (numberOfValues < 0)
......@@ -181,7 +172,7 @@ void StorageBasicBase::AllocateValues(vtkm::Id numberOfValues, vtkm::UInt64 size
return;
}
if (!this->DeallocateOnRelease)
if (!this->DeleteFunction)
{
throw vtkm::cont::ErrorBadValue("User allocated arrays cannot be reallocated.");
}
......@@ -193,9 +184,10 @@ void StorageBasicBase::AllocateValues(vtkm::Id numberOfValues, vtkm::UInt64 size
this->Array = AllocatorType{}.allocate(allocsize, VTKM_ALLOCATION_ALIGNMENT);
this->AllocatedByteSize = allocsize;
this->NumberOfValues = numberOfValues;
this->DeleteFunction = internal::free_memory;
if (this->Array == nullptr)
{
// Make sureour state is OK.
// Make sure our state is OK.
this->AllocatedByteSize = 0;
this->NumberOfValues = 0;
throw vtkm::cont::ErrorBadAllocation("Could not allocate basic control array.");
......@@ -207,7 +199,6 @@ void StorageBasicBase::AllocateValues(vtkm::Id numberOfValues, vtkm::UInt64 size
VTKM_ASSERT(this->NumberOfValues == 0);
VTKM_ASSERT(this->AllocatedByteSize == 0);
}
this->DeallocateOnRelease = true;
}
void StorageBasicBase::Shrink(vtkm::Id numberOfValues)
......@@ -220,6 +211,37 @@ void StorageBasicBase::Shrink(vtkm::Id numberOfValues)
this->NumberOfValues = numberOfValues;
}
void StorageBasicBase::ReleaseResources()
{
if (this->AllocatedByteSize > 0)
{
VTKM_ASSERT(this->Array != nullptr);
if (this->DeleteFunction)
{
this->DeleteFunction(this->Array);
}
this->Array = nullptr;
this->AllocatedByteSize = 0;
this->NumberOfValues = 0;
}
else
{
VTKM_ASSERT(this->Array == nullptr);
}
}
void StorageBasicBase::SetBasePointer(const void* ptr,
vtkm::Id numberOfValues,
vtkm::UInt64 sizeOfValue,
void (*deleteFunction)(void*))
{
this->ReleaseResources();
this->Array = const_cast<void*>(ptr);
this->AllocatedByteSize = static_cast<vtkm::UInt64>(numberOfValues) * sizeOfValue;
this->NumberOfValues = numberOfValues;
this->DeleteFunction = deleteFunction;
}
void* StorageBasicBase::GetBasePointer() const
{
return this->Array;
......
......@@ -41,6 +41,12 @@ struct VTKM_ALWAYS_EXPORT StorageTagBasic
namespace internal
{
/// Function that does all of VTK-m de-allocations for storage basic.
/// This is exists so that stolen arrays can call the correct free
/// function ( _aligned_malloc / cuda_free ).
VTKM_CONT_EXPORT void free_memory(void* ptr);
/// Class that does all of VTK-m allocations
/// for storage basic. This is exists so that
/// stolen arrays can call the correct free
......@@ -48,14 +54,14 @@ namespace internal
struct VTKM_CONT_EXPORT StorageBasicAllocator
{
void* allocate(size_t size, size_t align);
void free_memory(void* p);
template <typename T>
void deallocate(T* p)
inline void deallocate(T* p)
{
this->free_memory(static_cast<void*>(p));
internal::free_memory(static_cast<void*>(p));
}
};
/// Base class for basic storage classes. This allow us to implement
/// vtkm::cont::Storage<T, StorageTagBasic > for any T type with no overhead
/// as all heavy logic is provide by a type-agnostic API including allocations, etc.
......@@ -64,7 +70,17 @@ class VTKM_CONT_EXPORT StorageBasicBase
public:
using AllocatorType = StorageBasicAllocator;
VTKM_CONT StorageBasicBase();
/// A non owning view of already allocated memory
VTKM_CONT StorageBasicBase(const void* array, vtkm::Id size, vtkm::UInt64 sizeOfValue);
/// Transfer the ownership of already allocated memory to VTK-m
VTKM_CONT StorageBasicBase(const void* array,
vtkm::Id size,
vtkm::UInt64 sizeOfValue,
void (*deleteFunction)(void*));
VTKM_CONT ~StorageBasicBase();
VTKM_CONT StorageBasicBase(const StorageBasicBase& src);
......@@ -106,7 +122,23 @@ public:
/// \brief Returns if vtkm will deallocate this memory. VTK-m StorageBasic
/// is designed that VTK-m will not deallocate user passed memory, or
/// instances that have been stolen (\c StealArray)
VTKM_CONT bool WillDeallocate() const { return this->DeallocateOnRelease; }
VTKM_CONT bool WillDeallocate() const { return this->DeleteFunction != nullptr; }
/// \brief Return the free function that will be used to free this memory.
///
/// Get the function that VTK-m will call to deallocate the memory. This
/// is useful when stealing memory from VTK-m so that you call the correct
/// free/delete function when releasing the memory
using DeleteFunctionSignature = void (*)(void*);
DeleteFunctionSignature GetDeleteFunction() const { return this->DeleteFunction; }
/// \brief Change the Change the pointer that this class is using. Should only be used
/// by ExecutionArrayInterface sublcasses.
/// Note: This will release any previous allocated memory!
VTKM_CONT void SetBasePointer(const void* ptr,
vtkm::Id numberOfValues,
vtkm::UInt64 sizeOfValue,
void (*deleteFunction)(void*));
/// Return the memory location of the first element of the array data.
VTKM_CONT void* GetBasePointer() const;
......@@ -121,7 +153,7 @@ protected:
void* Array;
vtkm::UInt64 AllocatedByteSize;
vtkm::Id NumberOfValues;
bool DeallocateOnRelease;
void (*DeleteFunction)(void*);
};
/// A basic implementation of an Storage object.
......@@ -146,7 +178,11 @@ public:
VTKM_CONT Storage();
/// \brief construct storage that VTK-m is not responsible for
VTKM_CONT Storage(const ValueType* array, vtkm::Id numberOfValues = 0);
VTKM_CONT Storage(const ValueType* array, vtkm::Id numberOfValues);
/// \brief construct storage that was previously allocated and now VTK-m is
/// responsible for
VTKM_CONT Storage(const ValueType* array, vtkm::Id numberOfValues, void (*deleteFunction)(void*));
VTKM_CONT void Allocate(vtkm::Id numberOfValues);
......@@ -171,7 +207,8 @@ public:
/// Storage will never deallocate the array or be able to reallocate it. This
/// is helpful for taking a reference for an array created internally by
/// VTK-m and not having to keep a VTK-m object around. Obviously the caller
/// becomes responsible for destroying the memory.
/// becomes responsible for destroying the memory, and should before hand
/// grab the correct DeleteFunction by calling \c GetDeleteFunction
///
VTKM_CONT ValueType* StealArray();
};
......
......@@ -41,6 +41,14 @@ Storage<T, vtkm::cont::StorageTagBasic>::Storage(const T* array, vtkm::Id number
{
}
template <typename T>
Storage<T, vtkm::cont::StorageTagBasic>::Storage(const T* array,
vtkm::Id numberOfValues,
void (*deleteFunction)(void*))
: StorageBasicBase(const_cast<T*>(array), numberOfValues, sizeof(T), deleteFunction)
{
}
template <typename T>
void Storage<T, vtkm::cont::StorageTagBasic>::Allocate(vtkm::Id numberOfValues)
{
......@@ -78,7 +86,7 @@ const T* Storage<T, vtkm::cont::StorageTagBasic>::GetArray() const
template <typename T>
T* Storage<T, vtkm::cont::StorageTagBasic>::StealArray()
{
this->DeallocateOnRelease = false;
this->DeleteFunction = nullptr;
return static_cast<T*>(this->Array);
}
......
......@@ -98,6 +98,15 @@ void ExecutionArrayInterfaceBasic<DeviceAdapterTagCuda>::Allocate(TypelessExecut
err << "Failed to allocate " << numBytes << " bytes on device: " << error.what();
throw vtkm::cont::ErrorBadAllocation(err.str());
}
// If we just allocated managed cuda memory and don't a host memory pointer
// we can share out managed memory. This allows for the use case of where we
// first allocate on CUDA and than want to use it on the host
if (CudaAllocator::IsManagedPointer(execArray.Array) && execArray.ArrayControl == nullptr)
{
this->ControlStorage.SetBasePointer(
execArray.Array, numberOfValues, sizeOfValue, [](void* ptr) { CudaAllocator::Free(ptr); });
}
}
void ExecutionArrayInterfaceBasic<DeviceAdapterTagCuda>::Free(
......
......@@ -135,11 +135,30 @@ struct TemplatedTests
}
}
void UserFreeFunction()
{
ValueType* temp = new ValueType[ARRAY_SIZE];
StorageType arrayStorage(
temp, ARRAY_SIZE, [](void* ptr) { delete[] static_cast<ValueType*>(ptr); });
VTKM_TEST_ASSERT(temp == arrayStorage.GetArray(),
"improper pointer after telling storage to own user allocated memory");
const ValueType BASIC_ALLOC_VALUE = ValueType(48);
this->SetStorage(arrayStorage, BASIC_ALLOC_VALUE);
VTKM_TEST_ASSERT(this->CheckStorage(arrayStorage, BASIC_ALLOC_VALUE),
"Array not holding value.");
arrayStorage.Allocate(ARRAY_SIZE * 2);
VTKM_TEST_ASSERT(arrayStorage.GetNumberOfValues() == ARRAY_SIZE * 2,
"Array not reallocated correctly.");
}
void operator()()
{
ValueType* stolenArray = StealArray1();
BasicAllocation();
UserFreeFunction();
StealArray2(stolenArray);
}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment