Skip to content

Commit 10da83a

Browse files
[SYCL] Do not lock unconditionally while access queue_iml::MInOrderExternalEvent (#17575)
queue_impl::MInOrderExternalEvent is empty on hot path, use a flag to avoid locking when empty.
1 parent 2ee62ad commit 10da83a

File tree

2 files changed

+44
-14
lines changed

2 files changed

+44
-14
lines changed

sycl/source/detail/queue_impl.cpp

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -284,13 +284,10 @@ event queue_impl::memcpyFromDeviceGlobal(
284284
}
285285

286286
sycl::detail::optional<event> queue_impl::getLastEvent() {
287-
{
288-
// The external event is required to finish last if set, so it is considered
289-
// the last event if present.
290-
std::lock_guard<std::mutex> Lock(MInOrderExternalEventMtx);
291-
if (MInOrderExternalEvent)
292-
return *MInOrderExternalEvent;
293-
}
287+
// The external event is required to finish last if set, so it is considered
288+
// the last event if present.
289+
if (std::optional<event> ExternalEvent = MInOrderExternalEvent.read())
290+
return ExternalEvent;
294291

295292
std::lock_guard<std::mutex> Lock{MMutex};
296293
if (MGraph.expired() && !MDefaultGraphDeps.LastEventPtr)

sycl/source/detail/queue_impl.hpp

Lines changed: 40 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -710,14 +710,18 @@ class queue_impl {
710710
void *getTraceEvent() { return MTraceEvent; }
711711

712712
void setExternalEvent(const event &Event) {
713-
std::lock_guard<std::mutex> Lock(MInOrderExternalEventMtx);
714-
MInOrderExternalEvent = Event;
713+
MInOrderExternalEvent.set([&](std::optional<event> &InOrderExternalEvent) {
714+
InOrderExternalEvent = Event;
715+
});
715716
}
716717

717718
std::optional<event> popExternalEvent() {
718-
std::lock_guard<std::mutex> Lock(MInOrderExternalEventMtx);
719719
std::optional<event> Result = std::nullopt;
720-
std::swap(Result, MInOrderExternalEvent);
720+
721+
MInOrderExternalEvent.unset(
722+
[&](std::optional<event> &InOrderExternalEvent) {
723+
std::swap(Result, InOrderExternalEvent);
724+
});
721725
return Result;
722726
}
723727

@@ -1025,6 +1029,36 @@ class queue_impl {
10251029
}
10261030
} MDefaultGraphDeps, MExtGraphDeps;
10271031

1032+
// Implement check-lock-check pattern to not lock empty MData as the locks
1033+
// come with runtime overhead.
1034+
template <typename DataType> class CheckLockCheck {
1035+
DataType MData;
1036+
std::atomic_bool MIsSet = false;
1037+
mutable std::mutex MDataMtx;
1038+
1039+
public:
1040+
template <typename F> void set(F &&func) {
1041+
std::lock_guard<std::mutex> Lock(MDataMtx);
1042+
MIsSet.store(true, std::memory_order_release);
1043+
std::forward<F>(func)(MData);
1044+
}
1045+
template <typename F> void unset(F &&func) {
1046+
if (MIsSet.load(std::memory_order_acquire)) {
1047+
std::lock_guard<std::mutex> Lock(MDataMtx);
1048+
if (MIsSet.load(std::memory_order_acquire)) {
1049+
std::forward<F>(func)(MData);
1050+
MIsSet.store(false, std::memory_order_release);
1051+
}
1052+
}
1053+
}
1054+
DataType read() {
1055+
if (!MIsSet.load(std::memory_order_acquire))
1056+
return DataType{};
1057+
std::lock_guard<std::mutex> Lock(MDataMtx);
1058+
return MData;
1059+
}
1060+
};
1061+
10281062
const bool MIsInorder;
10291063

10301064
std::vector<EventImplPtr> MStreamsServiceEvents;
@@ -1045,10 +1079,9 @@ class queue_impl {
10451079

10461080
// This event can be optionally provided by users for in-order queues to add
10471081
// an additional dependency for the subsequent submission in to the queue.
1048-
// Access to the event should be guarded with MInOrderExternalEventMtx.
1082+
// Access to the event should be guarded with mutex.
10491083
// NOTE: std::optional must not be exposed in the ABI.
1050-
std::optional<event> MInOrderExternalEvent;
1051-
mutable std::mutex MInOrderExternalEventMtx;
1084+
CheckLockCheck<std::optional<event>> MInOrderExternalEvent;
10521085

10531086
public:
10541087
// Queue constructed with the discard_events property

0 commit comments

Comments
 (0)