Skip to content

Commit bfcd551

Browse files
mfrancepilloisBensuorebleEwanC
committed
[SYCL][Graph] Makes command graph functions thread-safe (#265)
* [SYCL][Graph] Makes command graph functions thread-safe Addresses comments made on the first PR commit. Mutexes are now added to Graph implementation entry points instead of end points as was the case in the previous commit. Adds "build_pthread_inc" lit test macro to facilitate the compilation of the threading tests. Removes std::barrier (std-20) dependency in threading tests. Addresses Issue: #85 * [SYCL][Graph] Makes command graph functions thread-safe Moves threading tests that do not require a device to run to unitests * Update sycl/source/detail/graph_impl.cpp Co-authored-by: Ben Tracy <[email protected]> * [SYCL][Graph] Makes command graph functions thread-safe Adds some comments. * Update sycl/source/handler.cpp Co-authored-by: Pablo Reble <[email protected]> * Update sycl/source/detail/graph_impl.hpp Co-authored-by: Ewan Crawford <[email protected]> * [SYCL][Graph] Makes command graph functions thread-safe Adds dedidacted sub-class to unitests for multi-threading unitests * [SYCL][Graph] Makes command graph functions thread-safe Adds comments * [SYCL][Graph] thread-safe: bug fix after rebase --------- Co-authored-by: Ben Tracy <[email protected]> Co-authored-by: Pablo Reble <[email protected]> Co-authored-by: Ewan Crawford <[email protected]>
1 parent 8922f41 commit bfcd551

File tree

13 files changed

+668
-24
lines changed

13 files changed

+668
-24
lines changed

sycl/source/detail/graph_impl.cpp

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -220,7 +220,6 @@ graph_impl::add(sycl::detail::CG::CGTYPE CGType,
220220
checkForRequirement(Req, NodePtr, UniqueDeps);
221221
}
222222
}
223-
224223
// Add any nodes specified by event dependencies into the dependency list
225224
for (auto Dep : CommandGroup->getEvents()) {
226225
if (auto NodeImpl = MEventsMap.find(Dep); NodeImpl != MEventsMap.end()) {
@@ -474,6 +473,8 @@ void exec_graph_impl::createCommandBuffers(sycl::device Device) {
474473
}
475474

476475
exec_graph_impl::~exec_graph_impl() {
476+
WriteLock Lock(MMutex);
477+
477478
// clear all recording queue if not done before (no call to end_recording)
478479
MGraphImpl->clearQueues();
479480

@@ -499,6 +500,8 @@ exec_graph_impl::~exec_graph_impl() {
499500
sycl::event
500501
exec_graph_impl::enqueue(const std::shared_ptr<sycl::detail::queue_impl> &Queue,
501502
sycl::detail::CG::StorageInitHelper CGData) {
503+
WriteLock Lock(MMutex);
504+
502505
auto CreateNewEvent([&]() {
503506
auto NewEvent = std::make_shared<sycl::detail::event_impl>(Queue);
504507
NewEvent->setContextImpl(Queue->getContextImplPtr());
@@ -612,6 +615,7 @@ node modifiable_command_graph::addImpl(const std::vector<node> &Deps) {
612615
DepImpls.push_back(sycl::detail::getSyclObjImpl(D));
613616
}
614617

618+
graph_impl::WriteLock Lock(impl->MMutex);
615619
std::shared_ptr<detail::node_impl> NodeImpl = impl->add(DepImpls);
616620
return sycl::detail::createSyclObjFromImpl<node>(NodeImpl);
617621
}
@@ -624,6 +628,7 @@ node modifiable_command_graph::addImpl(std::function<void(handler &)> CGF,
624628
DepImpls.push_back(sycl::detail::getSyclObjImpl(D));
625629
}
626630

631+
graph_impl::WriteLock Lock(impl->MMutex);
627632
std::shared_ptr<detail::node_impl> NodeImpl =
628633
impl->add(impl, CGF, {}, DepImpls);
629634
return sycl::detail::createSyclObjFromImpl<node>(NodeImpl);
@@ -635,6 +640,7 @@ void modifiable_command_graph::make_edge(node &Src, node &Dest) {
635640
std::shared_ptr<detail::node_impl> ReceiverImpl =
636641
sycl::detail::getSyclObjImpl(Dest);
637642

643+
graph_impl::WriteLock Lock(impl->MMutex);
638644
impl->makeEdge(SenderImpl, ReceiverImpl);
639645
}
640646

@@ -666,6 +672,7 @@ bool modifiable_command_graph::begin_recording(queue &RecordingQueue) {
666672

667673
if (QueueImpl->getCommandGraph() == nullptr) {
668674
QueueImpl->setCommandGraph(impl);
675+
graph_impl::WriteLock Lock(impl->MMutex);
669676
impl->addQueue(QueueImpl);
670677
return true;
671678
}
@@ -687,12 +694,16 @@ bool modifiable_command_graph::begin_recording(
687694
return QueueStateChanged;
688695
}
689696

690-
bool modifiable_command_graph::end_recording() { return impl->clearQueues(); }
697+
bool modifiable_command_graph::end_recording() {
698+
graph_impl::WriteLock Lock(impl->MMutex);
699+
return impl->clearQueues();
700+
}
691701

692702
bool modifiable_command_graph::end_recording(queue &RecordingQueue) {
693703
auto QueueImpl = sycl::detail::getSyclObjImpl(RecordingQueue);
694704
if (QueueImpl->getCommandGraph() == impl) {
695705
QueueImpl->setCommandGraph(nullptr);
706+
graph_impl::WriteLock Lock(impl->MMutex);
696707
impl->removeQueue(QueueImpl);
697708
return true;
698709
}
@@ -719,6 +730,9 @@ executable_command_graph::executable_command_graph(
719730
const std::shared_ptr<detail::graph_impl> &Graph, const sycl::context &Ctx)
720731
: MTag(rand()),
721732
impl(std::make_shared<detail::exec_graph_impl>(Ctx, Graph)) {
733+
// Graph is read and written in this scope so we lock
734+
// this graph with full priviledges.
735+
graph_impl::WriteLock Lock(Graph->MMutex);
722736
finalizeImpl(); // Create backend representation for executable graph
723737
}
724738

0 commit comments

Comments
 (0)