Skip to content

[SYCL] Record submit time for shortcut operations #18455

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

Merged
merged 3 commits into from
May 28, 2025

Conversation

againull
Copy link
Contributor

@againull againull commented May 14, 2025

We record submission time for commands submitted via handler, but this was missing for shortcut commands.

Also this PR changes the values returned by command_submit, command_start, command_end for non-host events without UR event handle - such events may appear if command is nop, for example, USM operations for 0 bytes.
Such scenario is not explicitly specified in SYCL spec. Before changes, we used to return 0 for all three quieries. But for consistency,I believe it makes sense to return the same recorded submit time for all three: command_submit = command_start = command_end. It seems more convenient from user's perspective, for example if user submits sequence set of commands and there is some nop command in the middle of sequence:
command1
command2 (nop)
command3
and user calculates wants to calculate differences between submission time (or start time) of command2 and command1, then he will get negative value: 0 - submit_time_of_command2 (if we return 0).
And there can be more nop operations (if SYCL runtime optimizes out something), so if we return 0, user always has to check if the returned timestamp is 0 or not before using it in any calculations. So returning non-zero submit time seems better.

@againull againull requested a review from a team as a code owner May 14, 2025 06:38
@againull againull requested a review from sergey-semenov May 14, 2025 06:38
@againull againull temporarily deployed to WindowsCILock May 14, 2025 06:41 — with GitHub Actions Inactive
@againull againull temporarily deployed to WindowsCILock May 14, 2025 07:06 — with GitHub Actions Inactive
@againull againull temporarily deployed to WindowsCILock May 14, 2025 07:06 — with GitHub Actions Inactive
@againull againull marked this pull request as draft May 14, 2025 20:49
@againull againull temporarily deployed to WindowsCILock May 23, 2025 23:39 — with GitHub Actions Inactive
@againull againull marked this pull request as ready for review May 23, 2025 23:51
@againull againull temporarily deployed to WindowsCILock May 24, 2025 00:06 — with GitHub Actions Inactive
@againull againull temporarily deployed to WindowsCILock May 24, 2025 00:06 — with GitHub Actions Inactive
@againull
Copy link
Contributor Author

@sergey-semenov Could you please take a look.

@againull againull merged commit 731b967 into intel:sycl May 28, 2025
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants