[SYCL] Make host task timestamps share the same base as device tasks #18710
Conversation
againull
force-pushed
the
host_task_time_base
branch
from
92c0ce6 to
55afe43
Compare
Currently, we use std::chrono to record `command_submit`, `command_start` and `command_end` timestamps for host tasks. So, if there there is a mix of device tasks and host tasks submitted to the queue, they have a different time base and inconvenient to use/compare. This PR makes host task timestamps to have the same time base as device tasks.
againull
force-pushed
the
host_task_time_base
branch
from
55afe43 to
c79ef94
Compare
sycl/source/detail/event_impl.cpp
Outdated
| const std::weak_ptr<queue_impl> &QueueWeakPtr) { | ||
| if (QueueImplPtr Queue = QueueWeakPtr.lock()) { | ||
| try { | ||
| return Queue->getDeviceImpl().getCurrentDeviceTime(); |
There was a problem hiding this comment.
If all we need is the device, could it be safer to save a pointer to the device instead? Since devices don't change over the life of an application, we should not have to worry about it dying either, so we can avoid weak_ptr and locking.
There was a problem hiding this comment.
Sounds reasonable, I have changed it to store a pointer to the device.
Though it seems like the way how event_impl is constructed probably can be improved. Currently, at host task submission, event_impl is constructed with nullptr value for MQueue which is basically used to distinguish host and non-host task in the constructor. Then the queue where host task is submitted (MSubmittedQueue) is set via setSubmittedQueue. Probably better/safer to pass SubmittedQueue right at construction. That will also allow to get rid of setDevice in HostProfilingInfo and pass device at construction.
There was a problem hiding this comment.
I agree. We should avoid set*() functions that are meant for single-call initialization of members as much as possible.
|
syclcompat/kernel/kernel_win.cpp failure is known: #17561 |
Currently, we use
std::chronoto recordcommand_submit,command_startandcommand_endtimestamps for host tasks. So, if there there is a mix of device tasks and host tasks submitted to the queue, they have different time base and inconvenient to use/compare. This PR makes host task timestamps to have the same time base as device tasks.