Shuenhoy/vtk-m
1
0
Fork 0
mirror of https://gitlab.kitware.com/vtk/vtk-m synced 2024-09-08 13:23:51 +00:00
Code Issues 2 Actions Packages Projects Releases Wiki Activity

Allow a token to attach to an ArrayHandle multiple times

Browse Source 
When a single `ArrayHandle` is given to multiple arguments of a worklet
dispatch, the `PrepareFor*` methods will be called multiple times with
the same token. If one of them is a `PrepareForInPlace` or
`PrepareForOutput`, then the two requests will deadlock. To prevent
this, allow the `PrepareFor*` to happen if the same token was used
previously.
This commit is contained in:
Kenneth Moreland 2020-01-19 15:50:48 -07:00
parent 76ce9c87f0
commit 642b59f4fd
10 changed files with 120 additions and 57 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

30
vtkm/cont/ArrayHandle.h
View File

@ -395,7 +395,7 @@ public:
void Allocate(vtkm::Id numberOfValues)
{
LockType lock = this->GetLock();
this->WaitToWrite(lock);
this->WaitToWrite(lock, vtkm::cont::Token{});
this->ReleaseResourcesExecutionInternal(lock);
this->Internals->GetControlArray(lock)->Allocate(numberOfValues);
this->Internals->SetControlArrayValid(lock, true);
@ -417,7 +417,7 @@ public:
VTKM_CONT void ReleaseResourcesExecution()
{
LockType lock = this->GetLock();
this->WaitToWrite(lock);
this->WaitToWrite(lock, vtkm::cont::Token{});
// Save any data in the execution environment by making sure it is synced
// with the control environment.
@ -555,40 +555,46 @@ protected:
/// Returns true if read operations can currently be performed.
///
VTKM_CONT bool CanRead(const LockType& lock) const
VTKM_CONT bool CanRead(const LockType& lock, const vtkm::cont::Token& token) const
{
return (*this->Internals->GetWriteCount(lock) < 1);
return ((*this->Internals->GetWriteCount(lock) < 1) ||
(token.IsAttached(this->Internals->GetWriteCount(lock))));
}
//// Returns true if write operations can currently be performed.
///
VTKM_CONT bool CanWrite(const LockType& lock) const
VTKM_CONT bool CanWrite(const LockType& lock, const vtkm::cont::Token& token) const
{
return (*this->Internals->GetWriteCount(lock) < 1) &&
(*this->Internals->GetReadCount(lock) < 1);
return (((*this->Internals->GetWriteCount(lock) < 1) ||
(token.IsAttached(this->Internals->GetWriteCount(lock)))) &&
((*this->Internals->GetReadCount(lock) < 1) ||
((*this->Internals->GetReadCount(lock) == 1) &&
token.IsAttached(this->Internals->GetReadCount(lock)))));
}
//// Will block the current thread until a read can be performed.
///
VTKM_CONT void WaitToRead(LockType& lock) const
VTKM_CONT void WaitToRead(LockType& lock, const vtkm::cont::Token& token) const
{
// Note that if you deadlocked here, that means that you are trying to do a read operation on
// an array where an object is writing to it. This could happen on the same thread. For
// example, if you call `GetPortalControl()` then no other operation that can result in reading
// or writing data in the array can happen while the resulting portal is still in scope.
this->Internals->ConditionVariable.wait(lock, [&lock, this] { return this->CanRead(lock); });
this->Internals->ConditionVariable.wait(
lock, [&lock, &token, this] { return this->CanRead(lock, token); });
}
//// Will block the current thread until a write can be performed.
///
VTKM_CONT void WaitToWrite(LockType& lock) const
VTKM_CONT void WaitToWrite(LockType& lock, const vtkm::cont::Token& token) const
{
// Note that if you deadlocked here, that means that you are trying to do a write operation on
// an array where an object is reading or writing to it. This could happen on the same thread.
// For example, if you call `GetPortalControl()` then no other operation that can result in
// reading or writing data in the array can happen while the resulting portal is still in
// scope.
this->Internals->ConditionVariable.wait(lock, [&lock, this] { return this->CanWrite(lock); });
this->Internals->ConditionVariable.wait(
lock, [&lock, &token, this] { return this->CanWrite(lock, token); });
}
/// Gets this array handle ready to interact with the given device. If the
@ -614,7 +620,7 @@ protected:
{
if (this->Internals->IsExecutionArrayValid(lock))
{
this->WaitToWrite(lock);
this->WaitToWrite(lock, vtkm::cont::Token{});
// Note that it is possible that while waiting someone else deleted the execution array.
// That is why we check again.
}

16
vtkm/cont/ArrayHandle.hxx
View File

@ -185,7 +185,7 @@ void ArrayHandle<T, S>::Shrink(vtkm::Id numberOfValues)
if (numberOfValues < originalNumberOfValues)
{
this->WaitToWrite(lock);
this->WaitToWrite(lock, vtkm::cont::Token{});
if (this->Internals->IsControlArrayValid(lock))
{
this->Internals->GetControlArray(lock)->Shrink(numberOfValues);
@ -223,7 +223,7 @@ ArrayHandle<T, S>::PrepareForInput(DeviceAdapterTag device, vtkm::cont::Token& t
VTKM_IS_DEVICE_ADAPTER_TAG(DeviceAdapterTag);
LockType lock = this->GetLock();
this->WaitToRead(lock);
this->WaitToRead(lock, token);
if (!this->Internals->IsControlArrayValid(lock) && !this->Internals->IsExecutionArrayValid(lock))
{
@ -255,7 +255,7 @@ ArrayHandle<T, S>::PrepareForOutput(vtkm::Id numberOfValues,
VTKM_IS_DEVICE_ADAPTER_TAG(DeviceAdapterTag);
LockType lock = this->GetLock();
this->WaitToWrite(lock);
this->WaitToWrite(lock, token);
// Invalidate any control arrays.
// Should the control array resource be released? Probably not a good
@ -291,7 +291,7 @@ ArrayHandle<T, S>::PrepareForInPlace(DeviceAdapterTag device, vtkm::cont::Token&
VTKM_IS_DEVICE_ADAPTER_TAG(DeviceAdapterTag);
LockType lock = this->GetLock();
this->WaitToWrite(lock);
this->WaitToWrite(lock, token);
if (!this->Internals->IsControlArrayValid(lock) && !this->Internals->IsExecutionArrayValid(lock))
{
@ -340,7 +340,7 @@ void ArrayHandle<T, S>::PrepareForDevice(LockType& lock, DeviceAdapterTag device
// could change the ExecutionInterface, which would cause problems. In the future we should
// support multiple devices, in which case we would not have to delete one execution array
// to load another.
this->WaitToWrite(lock); // Make sure no one is reading device array
this->WaitToWrite(lock, vtkm::cont::Token{}); // Make sure no one is reading device array
this->SyncControlArray(lock);
// Need to change some state that does not change the logical state from
// an external point of view.
@ -358,7 +358,11 @@ void ArrayHandle<T, S>::SyncControlArray(LockType& lock) const
{
if (!this->Internals->IsControlArrayValid(lock))
{
this->WaitToRead(lock);
// It may be the case that `SyncControlArray` is called from a method that has a `Token`.
// However, if we are here, that `Token` should not already be attached to this array.
// If it were, then there should be no reason to move data arround (unless the `Token`
// was used when preparing for multiple devices, which it should not be used like that).
this->WaitToRead(lock, vtkm::cont::Token{});
// Need to change some state that does not change the logical state from
// an external point of view.

25
vtkm/cont/Token.cxx
View File

@ -82,6 +82,11 @@ void vtkm::cont::Token::Attach(std::unique_ptr<vtkm::cont::Token::ObjectReferenc
std::condition_variable* conditionVariablePointer)
{
LockType localLock = this->Internals->GetLock();
if (this->IsAttached(localLock, referenceCountPointer))
{
// Already attached.
return;
}
if (!lock.owns_lock())
{
lock.lock();
@ -90,3 +95,23 @@ void vtkm::cont::Token::Attach(std::unique_ptr<vtkm::cont::Token::ObjectReferenc
this->Internals->GetHeldReferences(localLock)->emplace_back(
std::move(objectRef), referenceCountPointer, lock.mutex(), conditionVariablePointer);
}
inline bool vtkm::cont::Token::IsAttached(
LockType& lock,
vtkm::cont::Token::ReferenceCount* referenceCountPointer) const
{
for (auto&& heldReference : *this->Internals->GetHeldReferences(lock))
{
if (referenceCountPointer == heldReference.ReferenceCountPointer)
{
return true;
}
}
return false;
}
bool vtkm::cont::Token::IsAttached(vtkm::cont::Token::ReferenceCount* referenceCountPointer) const
{
LockType lock = this->Internals->GetLock();
return this->IsAttached(lock, referenceCountPointer);
}

40
vtkm/cont/Token.h
View File

@ -66,8 +66,8 @@ class VTKM_CONT_EXPORT Token final
};
public:
Token();
~Token();
VTKM_CONT Token();
VTKM_CONT ~Token();
/// Use this type to represent counts of how many tokens are holding a resource.
using ReferenceCount = vtkm::IdComponent;
@ -75,7 +75,7 @@ public:
/// Detaches this `Token` from all resources to allow them to be used elsewhere
/// or deleted.
///
void DetachFromAll();
VTKM_CONT void DetachFromAll();
///@{
/// \brief Add an object to attach to the `Token`.
@ -98,10 +98,10 @@ public:
/// variable.
///
template <typename T>
void Attach(T&& object,
vtkm::cont::Token::ReferenceCount* referenceCountPointer,
std::unique_lock<std::mutex>& lock,
std::condition_variable* conditionVariablePointer)
VTKM_CONT void Attach(T&& object,
vtkm::cont::Token::ReferenceCount* referenceCountPointer,
std::unique_lock<std::mutex>& lock,
std::condition_variable* conditionVariablePointer)
{
this->Attach(std::unique_ptr<ObjectReference>(
new ObjectReferenceImpl<typename std::decay<T>::type>(std::forward<T>(object))),
@ -111,21 +111,31 @@ public:
}
template <typename T>
void Attach(T&& object,
vtkm::cont::Token::ReferenceCount* referenceCountPoiner,
std::mutex* mutexPointer,
std::condition_variable* conditionVariablePointer)
VTKM_CONT void Attach(T&& object,
vtkm::cont::Token::ReferenceCount* referenceCountPoiner,
std::mutex* mutexPointer,
std::condition_variable* conditionVariablePointer)
{
std::unique_lock<std::mutex> lock(*mutexPointer, std::defer_lock);
this->Attach(std::forward<T>(object), referenceCountPoiner, lock, conditionVariablePointer);
}
///@}
/// \brief Determine if this `Token` is already attached to an object.
///
/// Given a reference counter pointer, such as would be passed to the `Attach` method,
/// returns true if this `Token` is already attached, `false` otherwise.
///
VTKM_CONT bool IsAttached(vtkm::cont::Token::ReferenceCount* referenceCountPointer) const;
private:
void Attach(std::unique_ptr<vtkm::cont::Token::ObjectReference>&& objectReference,
vtkm::cont::Token::ReferenceCount* referenceCountPointer,
std::unique_lock<std::mutex>& lock,
std::condition_variable* conditionVariablePointer);
VTKM_CONT void Attach(std::unique_ptr<vtkm::cont::Token::ObjectReference>&& objectReference,
vtkm::cont::Token::ReferenceCount* referenceCountPointer,
std::unique_lock<std::mutex>& lock,
std::condition_variable* conditionVariablePointer);
VTKM_CONT bool IsAttached(std::unique_lock<std::mutex>& lock,
vtkm::cont::Token::ReferenceCount* referenceCountPointer) const;
};
}
} // namespace vtkm::cont

1
vtkm/cont/arg/testing/UnitTestTransportArrayInOut.cxx
View File

@ -62,6 +62,7 @@ struct TryArrayInOutType
kernel.Portal = transport(handle, handle, ARRAY_SIZE, ARRAY_SIZE, token);
vtkm::cont::DeviceAdapterAlgorithm<Device>::Schedule(kernel, ARRAY_SIZE);
token.DetachFromAll();
typename ArrayHandleType::PortalConstControl portal = handle.GetPortalConstControl();
VTKM_TEST_ASSERT(portal.GetNumberOfValues() == ARRAY_SIZE,

1
vtkm/cont/arg/testing/UnitTestTransportArrayOut.cxx
View File

@ -60,6 +60,7 @@ struct TryArrayOutType
"ArrayOut transport did not allocate array correctly.");
vtkm::cont::DeviceAdapterAlgorithm<Device>::Schedule(kernel, ARRAY_SIZE);
token.DetachFromAll();
CheckPortal(handle.GetPortalConstControl());
}

2
vtkm/cont/arg/testing/UnitTestTransportExecObject.cxx
View File

@ -41,7 +41,7 @@ struct TestExecutionObject : public vtkm::cont::ExecutionObjectBase
vtkm::Int32 Number;
template <typename Device>
VTKM_CONT ExecutionObject<Device> PrepareForExecution(Device) const
VTKM_CONT ExecutionObject<Device> PrepareForExecution(Device, vtkm::cont::Token&) const
{
ExecutionObject<Device> object;
object.Number = this->Number;

50
vtkm/cont/internal/ArrayHandleBasicImpl.cxx
View File

@ -95,7 +95,7 @@ vtkm::Id ArrayHandleImpl::GetNumberOfValues(const LockType& lock, vtkm::UInt64 s
void ArrayHandleImpl::Allocate(LockType& lock, vtkm::Id numberOfValues, vtkm::UInt64 sizeOfT)
{
this->WaitToWrite(lock);
this->WaitToWrite(lock, vtkm::cont::Token{});
this->ReleaseResourcesExecutionInternal(lock);
this->Internals->GetControlArray(lock)->AllocateValues(numberOfValues, sizeOfT);
this->Internals->SetControlArrayValid(lock, true);
@ -111,7 +111,7 @@ void ArrayHandleImpl::Shrink(LockType& lock, vtkm::Id numberOfValues, vtkm::UInt
if (numberOfValues < originalNumberOfValues)
{
this->WaitToWrite(lock);
this->WaitToWrite(lock, vtkm::cont::Token{});
if (this->Internals->IsControlArrayValid(lock))
{
this->Internals->GetControlArray(lock)->Shrink(numberOfValues);
@ -145,7 +145,7 @@ void ArrayHandleImpl::Shrink(LockType& lock, vtkm::Id numberOfValues, vtkm::UInt
void ArrayHandleImpl::ReleaseResources(LockType& lock)
{
this->WaitToWrite(lock);
this->WaitToWrite(lock, vtkm::cont::Token{});
this->ReleaseResourcesExecutionInternal(lock);
if (this->Internals->IsControlArrayValid(lock))
@ -159,7 +159,7 @@ void ArrayHandleImpl::PrepareForInput(LockType& lock,
vtkm::UInt64 sizeOfT,
vtkm::cont::Token& token) const
{
this->WaitToRead(lock);
this->WaitToRead(lock, token);
const vtkm::Id numVals = this->GetNumberOfValues(lock, sizeOfT);
const vtkm::UInt64 numBytes = sizeOfT * static_cast<vtkm::UInt64>(numVals);
@ -199,7 +199,7 @@ void ArrayHandleImpl::PrepareForOutput(LockType& lock,
vtkm::UInt64 sizeOfT,
vtkm::cont::Token& token)
{
this->WaitToWrite(lock);
this->WaitToWrite(lock, token);
// Invalidate control arrays since we expect the execution data to be
// overwritten. Don't free control resources in case they're shared with
@ -227,7 +227,7 @@ void ArrayHandleImpl::PrepareForInPlace(LockType& lock,
vtkm::UInt64 sizeOfT,
vtkm::cont::Token& token)
{
this->WaitToWrite(lock);
this->WaitToWrite(lock, token);
const vtkm::Id numVals = this->GetNumberOfValues(lock, sizeOfT);
const vtkm::UInt64 numBytes = sizeOfT * static_cast<vtkm::UInt64>(numVals);
@ -287,7 +287,7 @@ bool ArrayHandleImpl::PrepareForDevice(LockType& lock,
// could change the ExecutionInterface, which would cause problems. In the future we should
// support multiple devices, in which case we would not have to delete one execution array
// to load another.
this->WaitToWrite(lock); // Make sure no one is reading device array
this->WaitToWrite(lock, vtkm::cont::Token{}); // Make sure no one is reading device array
this->SyncControlArray(lock, sizeOfT);
TypelessExecutionArray execArray = this->Internals->MakeTypelessExecutionArray(lock);
this->Internals->GetExecutionInterface(lock)->Free(execArray);
@ -316,7 +316,11 @@ void ArrayHandleImpl::SyncControlArray(LockType& lock, vtkm::UInt64 sizeOfT) con
// an external point of view.
if (this->Internals->IsExecutionArrayValid(lock))
{
this->WaitToRead(lock);
// It may be the case that `SyncControlArray` is called from a method that has a `Token`.
// However, if we are here, that `Token` should not already be attached to this array.
// If it were, then there should be no reason to move data arround (unless the `Token`
// was used when preparing for multiple devices, which it should not be used like that).
this->WaitToRead(lock, vtkm::cont::Token{});
const vtkm::UInt64 numBytes =
static_cast<vtkm::UInt64>(static_cast<char*>(this->Internals->GetExecutionArrayEnd(lock)) -
static_cast<char*>(this->Internals->GetExecutionArray(lock)));
@ -344,39 +348,51 @@ void ArrayHandleImpl::ReleaseResourcesExecutionInternal(LockType& lock)
{
if (this->Internals->IsExecutionArrayValid(lock))
{
this->WaitToWrite(lock);
this->WaitToWrite(lock, vtkm::cont::Token{});
// Note that it is possible that while waiting someone else deleted the execution array.
// That is why we check again.
}
if (this->Internals->IsExecutionArrayValid(lock))
{
TypelessExecutionArray execArray = this->Internals->MakeTypelessExecutionArray(lock);
this->Internals->GetExecutionInterface(lock)->Free(execArray);
this->Internals->SetExecutionArrayValid(lock, false);
}
}
bool ArrayHandleImpl::CanRead(const LockType& lock) const
bool ArrayHandleImpl::CanRead(const LockType& lock, const vtkm::cont::Token& token) const
{
return (*this->Internals->GetWriteCount(lock) < 1);
return ((*this->Internals->GetWriteCount(lock) < 1) ||
(token.IsAttached(this->Internals->GetWriteCount(lock))));
}
bool ArrayHandleImpl::CanWrite(const LockType& lock) const
bool ArrayHandleImpl::CanWrite(const LockType& lock, const vtkm::cont::Token& token) const
{
return (*this->Internals->GetWriteCount(lock) < 1) && (*this->Internals->GetReadCount(lock) < 1);
return (((*this->Internals->GetWriteCount(lock) < 1) ||
(token.IsAttached(this->Internals->GetWriteCount(lock)))) &&
((*this->Internals->GetReadCount(lock) < 1) ||
((*this->Internals->GetReadCount(lock) == 1) &&
token.IsAttached(this->Internals->GetReadCount(lock)))));
}
void ArrayHandleImpl::WaitToRead(LockType& lock) const
void ArrayHandleImpl::WaitToRead(LockType& lock, const vtkm::cont::Token& token) const
{
// Note that if you deadlocked here, that means that you are trying to do a read operation on an
// array where an object is writing to it. This could happen on the same thread. For example, if
// you call `GetPortalControl()` then no other operation that can result in reading or writing
// data in the array can happen while the resulting portal is still in scope.
this->Internals->ConditionVariable.wait(lock, [&lock, this] { return this->CanRead(lock); });
this->Internals->ConditionVariable.wait(
lock, [&lock, &token, this] { return this->CanRead(lock, token); });
}
void ArrayHandleImpl::WaitToWrite(LockType& lock) const
void ArrayHandleImpl::WaitToWrite(LockType& lock, const vtkm::cont::Token& token) const
{
// Note that if you deadlocked here, that means that you are trying to do a write operation on an
// array where an object is reading or writing to it. This could happen on the same thread. For
// example, if you call `GetPortalControl()` then no other operation that can result in reading
// or writing data in the array can happen while the resulting portal is still in scope.
this->Internals->ConditionVariable.wait(lock, [&lock, this] { return this->CanWrite(lock); });
this->Internals->ConditionVariable.wait(
lock, [&lock, &token, this] { return this->CanWrite(lock, token); });
}
} // end namespace internal

8
vtkm/cont/internal/ArrayHandleBasicImpl.h
View File

@ -186,14 +186,14 @@ struct VTKM_CONT_EXPORT ArrayHandleImpl
VTKM_CONT DeviceAdapterId GetDeviceAdapterId(const LockType& lock) const;
/// Returns true if read operations can currently be performed.
VTKM_CONT bool CanRead(const LockType& lock) const;
VTKM_CONT bool CanRead(const LockType& lock, const vtkm::cont::Token& token) const;
//// Returns true if write operations can currently be performed.
VTKM_CONT bool CanWrite(const LockType& lock) const;
VTKM_CONT bool CanWrite(const LockType& lock, const vtkm::cont::Token& token) const;
//// Will block the current thread until a read can be performed.
VTKM_CONT void WaitToRead(LockType& lock) const;
VTKM_CONT void WaitToRead(LockType& lock, const vtkm::cont::Token& token) const;
//// Will block the current thread until a write can be performed.
VTKM_CONT void WaitToWrite(LockType& lock) const;
VTKM_CONT void WaitToWrite(LockType& lock, const vtkm::cont::Token& token) const;
/// Acquires a lock on the internals of this `ArrayHandle`. The calling
/// function should keep the returned lock and let it go out of scope

4
vtkm/cont/testing/UnitTestToken.cxx
View File

@ -115,13 +115,13 @@ void TestBasicAttachDetatch()
std::cout << " Recursively attach outer token" << std::endl;
object1.Attach(outerToken);
CHECK_OBJECT(object1, 3, 4);
CHECK_OBJECT(object1, 2, 3);
CHECK_OBJECT(object2, 2, 3);
CHECK_OBJECT(object3, 2, 3);
std::cout << " Detach from inner token (through scoping)" << std::endl;
}
CHECK_OBJECT(object1, 2, 3);
CHECK_OBJECT(object1, 1, 2);
CHECK_OBJECT(object2, 1, 2);
CHECK_OBJECT(object3, 1, 2);