-
Notifications
You must be signed in to change notification settings - Fork 754
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[SYCL][XPTI] Refactoring framework to use 128-bit keys for collision elimination #14467
Changes from 21 commits
12a61e4
d04a1c1
72324dd
120a824
955f805
5b060da
dc15a7c
7e00d87
5019c69
ece9bd0
749f075
cd64e97
1cbf8c7
52321bd
5386f2d
efbb76f
05bf6e1
5b4c18d
db18ee8
e98d31c
e4b01ee
7e71519
c7e26fb
ebab90c
908c103
9ccc216
0e4128a
26f1bce
351e9c9
b0118e7
ca9a437
205fdc3
2b028fb
c2a405b
f722748
47722da
dacb7e1
230dd7f
984ec95
e2cc25e
ab81cfe
04c5f2d
fc74bd3
547cb71
02944f3
ec3b345
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -176,9 +176,6 @@ void *event_impl::instrumentationProlog(std::string &Name, int32_t StreamID, | |
constexpr uint16_t NotificationTraceType = xpti::trace_wait_begin; | ||
if (!xptiCheckTraceEnabled(StreamID, NotificationTraceType)) | ||
return TraceEvent; | ||
// Use a thread-safe counter to get a unique instance ID for the wait() on the | ||
// event | ||
static std::atomic<uint64_t> InstanceID = {1}; | ||
xpti::trace_event_data_t *WaitEvent = nullptr; | ||
|
||
// Create a string with the event address so it | ||
|
@@ -193,11 +190,20 @@ void *event_impl::instrumentationProlog(std::string &Name, int32_t StreamID, | |
Command *Cmd = (Command *)MCommand; | ||
WaitEvent = Cmd->MTraceEvent ? static_cast<xpti_td *>(Cmd->MTraceEvent) | ||
: GSYCLGraphEvent; | ||
} else | ||
WaitEvent = GSYCLGraphEvent; | ||
|
||
} else { | ||
// If queue.wait() is used, we want to make sure the information about the | ||
// queue is available with the eait events. We check to see if the | ||
// TraceEvent is available in the Queue object. | ||
void *TraceEvent; | ||
if (QueueImplPtr Queue = MQueue.lock()) { | ||
TraceEvent = Queue->getTraceEvent(); | ||
WaitEvent = | ||
(TraceEvent ? static_cast<xpti_td *>(TraceEvent) : GSYCLGraphEvent); | ||
} else | ||
WaitEvent = GSYCLGraphEvent; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: the following looks a bit more simpler void *TraceEvent = nullptr; WaitEvent = (TraceEvent ? static_cast<xpti_td *>(TraceEvent) : GSYCLGraphEvent); There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't we handle the if(QueueImplPtr Queue = MQueue.lock()) being false case? |
||
} | ||
// Record the current instance ID for use by Epilog | ||
IId = InstanceID++; | ||
IId = xptiGetUniqueId(); | ||
KseniyaTikhomirova marked this conversation as resolved.
Show resolved
Hide resolved
|
||
xptiNotifySubscribers(StreamID, NotificationTraceType, nullptr, WaitEvent, | ||
IId, static_cast<const void *>(Name.c_str())); | ||
TraceEvent = (void *)WaitEvent; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -174,37 +174,10 @@ class queue_impl { | |
// We enable XPTI tracing events using the TLS mechanism; if the code | ||
// location data is available, then the tracing data will be rich. | ||
#if XPTI_ENABLE_INSTRUMENTATION | ||
constexpr uint16_t NotificationTraceType = | ||
static_cast<uint16_t>(xpti::trace_point_type_t::queue_create); | ||
// Using the instance override constructor for use with queues as queues | ||
// maintain instance IDs in the object | ||
XPTIScope PrepareNotify((void *)this, NotificationTraceType, | ||
SYCL_STREAM_NAME, MQueueID, "queue_create"); | ||
// Cache the trace event, stream id and instance IDs for the destructor | ||
if (xptiCheckTraceEnabled(PrepareNotify.streamID(), | ||
NotificationTraceType)) { | ||
MTraceEvent = (void *)PrepareNotify.traceEvent(); | ||
MStreamID = PrepareNotify.streamID(); | ||
MInstanceID = PrepareNotify.instanceID(); | ||
// Add the function to capture meta data for the XPTI trace event | ||
PrepareNotify.addMetadata([&](auto TEvent) { | ||
xpti::addMetadata(TEvent, "sycl_context", | ||
reinterpret_cast<size_t>(MContext->getHandleRef())); | ||
if (MDevice) { | ||
xpti::addMetadata(TEvent, "sycl_device_name", | ||
MDevice->getDeviceName()); | ||
xpti::addMetadata(TEvent, "sycl_device", | ||
reinterpret_cast<size_t>(MDevice->getHandleRef())); | ||
} | ||
xpti::addMetadata(TEvent, "is_inorder", MIsInorder); | ||
xpti::addMetadata(TEvent, "queue_id", MQueueID); | ||
xpti::addMetadata(TEvent, "queue_handle", | ||
reinterpret_cast<size_t>(getHandleRef())); | ||
}); | ||
// Also publish to TLS | ||
xpti::framework::stash_tuple(XPTI_QUEUE_INSTANCE_ID_KEY, MQueueID); | ||
PrepareNotify.notify(); | ||
} | ||
// Emit a trace event for queue creation; we currently do not get code | ||
// location information, so all queueus will have the same UID with a | ||
// different instance ID until this gets added. | ||
constructorNotification(); | ||
#endif | ||
} | ||
|
||
|
@@ -237,35 +210,10 @@ class queue_impl { | |
// is the prolog section and the epilog section will initiate the | ||
// notification. | ||
#if XPTI_ENABLE_INSTRUMENTATION | ||
constexpr uint16_t NotificationTraceType = | ||
static_cast<uint16_t>(xpti::trace_point_type_t::queue_create); | ||
XPTIScope PrepareNotify((void *)this, NotificationTraceType, | ||
SYCL_STREAM_NAME, MQueueID, "queue_create"); | ||
if (xptiCheckTraceEnabled(PrepareNotify.streamID(), | ||
NotificationTraceType)) { | ||
// Cache the trace event, stream id and instance IDs for the destructor | ||
MTraceEvent = (void *)PrepareNotify.traceEvent(); | ||
MStreamID = PrepareNotify.streamID(); | ||
MInstanceID = PrepareNotify.instanceID(); | ||
|
||
// Add the function to capture meta data for the XPTI trace event | ||
PrepareNotify.addMetadata([&](auto TEvent) { | ||
xpti::addMetadata(TEvent, "sycl_context", | ||
reinterpret_cast<size_t>(MContext->getHandleRef())); | ||
if (MDevice) { | ||
xpti::addMetadata(TEvent, "sycl_device_name", | ||
MDevice->getDeviceName()); | ||
xpti::addMetadata(TEvent, "sycl_device", | ||
reinterpret_cast<size_t>(MDevice->getHandleRef())); | ||
} | ||
xpti::addMetadata(TEvent, "is_inorder", MIsInorder); | ||
xpti::addMetadata(TEvent, "queue_id", MQueueID); | ||
xpti::addMetadata(TEvent, "queue_handle", getHandleRef()); | ||
}); | ||
// Also publish to TLS before notification | ||
xpti::framework::stash_tuple(XPTI_QUEUE_INSTANCE_ID_KEY, MQueueID); | ||
PrepareNotify.notify(); | ||
} | ||
// Emit a trace event for queue creation; we currently do not get code | ||
// location information, so all queueus will have the same UID with a | ||
// different instance ID until this gets added. | ||
constructorNotification(); | ||
#endif | ||
} | ||
|
||
|
@@ -311,16 +259,10 @@ class queue_impl { | |
// lifetime of the queue object as member variables when ABI breakage is | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. probably old comment should be removed now. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done! It was a stale comment. |
||
// allowed. This example shows MTraceEvent as a member variable. | ||
#if XPTI_ENABLE_INSTRUMENTATION | ||
constexpr uint16_t NotificationTraceType = | ||
static_cast<uint16_t>(xpti::trace_point_type_t::queue_destroy); | ||
if (xptiCheckTraceEnabled(MStreamID, NotificationTraceType)) { | ||
// Used cached information in member variables | ||
xptiNotifySubscribers(MStreamID, NotificationTraceType, nullptr, | ||
(xpti::trace_event_data_t *)MTraceEvent, | ||
MInstanceID, | ||
static_cast<const void *>("queue_destroy")); | ||
xptiReleaseEvent((xpti::trace_event_data_t *)MTraceEvent); | ||
} | ||
// The trace event created in the constructor should be active through the | ||
// lifetime of the queue object as member variable. We will send a | ||
// notification and destroy the trace event for this queue. | ||
destructorNotification(); | ||
#endif | ||
throw_asynchronous(); | ||
cleanup_fusion_cmd(); | ||
|
@@ -739,6 +681,8 @@ class queue_impl { | |
|
||
unsigned long long getQueueID() { return MQueueID; } | ||
|
||
void *getTraceEvent() { return MTraceEvent; } | ||
|
||
void setExternalEvent(const event &Event) { | ||
std::lock_guard<std::mutex> Lock(MInOrderExternalEventMtx); | ||
MInOrderExternalEvent = Event; | ||
|
@@ -925,6 +869,13 @@ class queue_impl { | |
void instrumentationEpilog(void *TelementryEvent, std::string &Name, | ||
int32_t StreamID, uint64_t IId); | ||
|
||
// We need to emit a queue_create notification when a queue object is created | ||
void constructorNotification(); | ||
|
||
// We need to emit a queue_destroy notification when a queue object is | ||
// destroyed | ||
void destructorNotification(); | ||
|
||
/// queue_impl.addEvent tracks events with weak pointers | ||
/// but some events have no other owners. addSharedEvent() | ||
/// follows events with a shared pointer. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mistype, "wait"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done!