Skip to content

[SYCL][L0] Correctly use global timestamps, instead of context #6122

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
May 10, 2022
Merged

[SYCL][L0] Correctly use global timestamps, instead of context #6122

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions sycl/plugins/level_zero/pi_level_zero.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5331,15 +5331,15 @@ pi_result piEventGetProfilingInfo(pi_event Event, pi_profiling_info ParamName,
case PI_PROFILING_INFO_COMMAND_START: {
ZE_CALL(zeEventQueryKernelTimestamp, (Event->ZeEvent, &tsResult));
uint64_t ContextStartTime =
(tsResult.context.kernelStart & TimestampMaxValue) * ZeTimerResolution;
Copy link
Contributor

@smaslov-intel smaslov-intel May 9, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

any comments why using "context" is incorrect here? (the event being profiled belongs to a L0 context)

Copy link
Contributor Author

@jandres742 jandres742 May 9, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@smaslov-intel

from https://spec.oneapi.io/level-zero/latest/core/api.html?highlight=timestamp#ze-kernel-timestamp-result-t

global: [out] wall-clock data

context: [out] context-active data; only includes clocks while device context was actively executing.

So with global we get actual global time, including any time the workload has been preempted.

Copy link
Contributor

@smaslov-intel smaslov-intel May 10, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. I don't see SYCL spec saying anything about what the timestamp values should include: https://www.khronos.org/registry/SYCL/specs/sycl-2020/html/sycl-2020.html#_event_information_and_profiling_descriptors

So I guess it's OK to return either one. Just curious what was the problem while returning "context" timestamps?

Copy link
Contributor Author

@jandres742 jandres742 May 10, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@smaslov-intel

Right, i didnt see anything in particular in SYCL specl.

Now, let's say we submit a kernel for execution, but because of scheduling and other hardware mechanisms, the kernel doesn't start execution right away. Now, it starts execution and gets preempted, and then resume again.

context timestamp would include only the time where the kernel was actively being executed, while the global timestamp would include even the times the kernel is waiting for active resources to execute. I would say that since SYCL is not clear which one is used, we could assume global is the one of importance here, as you said.

(tsResult.global.kernelStart & TimestampMaxValue) * ZeTimerResolution;
return ReturnValue(ContextStartTime);
}
case PI_PROFILING_INFO_COMMAND_END: {
ZE_CALL(zeEventQueryKernelTimestamp, (Event->ZeEvent, &tsResult));

uint64_t ContextStartTime =
(tsResult.context.kernelStart & TimestampMaxValue);
uint64_t ContextEndTime = (tsResult.context.kernelEnd & TimestampMaxValue);
(tsResult.global.kernelStart & TimestampMaxValue);
uint64_t ContextEndTime = (tsResult.global.kernelEnd & TimestampMaxValue);

//
// Handle a possible wrap-around (the underlying HW counter is < 64-bit).
Expand Down