Skip to content
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

Jump to bottom

[SYCL] Reusing handler keeping its pre-constructed copy in queue_impl #17130

Draft
wants to merge 5 commits into
base: sycl
Choose a base branch
from
Draft

[SYCL] Reusing handler keeping its pre-constructed copy in queue_impl #17130

wants to merge 5 commits into from

Conversation

Alexandr-Konovalov
Copy link
Contributor

@Alexandr-Konovalov Alexandr-Konovalov commented Feb 24, 2025 •
edited by aelovikov-intel
Loading

Cached sycl::handler is kept in TLS in queue_impl and filled with valid content in the beginning of queue_impl::submit_impl() call. This saves several dynamic memory allocations.

@Alexandr-Konovalov
Copy link
Contributor Author

There is an issue HostInteropTask/interop-task-cuda-buffer-migrate.cpp fails flakily on unrelated changes #17026 describing flaky failure of HostInteropTask/interop-task-cuda-buffer-migrate.cpp.

I unable to reproduce the failure on a machine with NVIDIA GeForce RTX 2060.

@Alexandr-Konovalov Alexandr-Konovalov marked this pull request as ready for review February 27, 2025 15:58
@Alexandr-Konovalov Alexandr-Konovalov requested a review from a team as a code owner February 27, 2025 15:58
@Alexandr-Konovalov Alexandr-Konovalov marked this pull request as draft March 3, 2025 11:53
@Alexandr-Konovalov Alexandr-Konovalov force-pushed the Alexandr-Konovalov/reusehandler branch 2 times, most recently from 4082d40 to 1eb3da3 Compare March 3, 2025 14:55
@Alexandr-Konovalov Alexandr-Konovalov force-pushed the Alexandr-Konovalov/reusehandler branch from db4235a to 5ec607e Compare March 12, 2025 13:15
@Alexandr-Konovalov Alexandr-Konovalov force-pushed the Alexandr-Konovalov/reusehandler branch 2 times, most recently from 7898120 to 1ec87db Compare March 19, 2025 16:21
sergey-semenov and others added 5 commits March 25, 2025 17:12
@sergey-semenov @Alexandr-Konovalov
Try reusing handler
e3e4abb 
(cherry picked from commit e11984f)
@Alexandr-Konovalov
Improve data reset, decrease number of dynamic memory allocation.
c115c86 
Make cached handler std::unique_ptr to have lazy initialization.
Reset MHandler in RAII.
Limit scope of MHandler usage to not have nested submit_impl() calls.
Do handler cleanup only on leaving submit_impl() scope.
Extend support for empty event.
Add make_empty().
Do not reset data on entry to submit.
PrimaryQueue/SecondaryQueue became pointers to queue_impl. This is correct because active handler only exists in automatic scope when queues guaranee to be alive.
@Alexandr-Konovalov
Fix after merge.
9625343
@Alexandr-Konovalov
Call postprocess out of MHandler wrapper scope, as postporocess can c…
1d9b228 
…all submit.
@Alexandr-Konovalov
Add missing variable initialization.
5b6b80e
@Alexandr-Konovalov Alexandr-Konovalov force-pushed the Alexandr-Konovalov/reusehandler branch from fa5e8c1 to 5b6b80e Compare March 25, 2025 16:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aelovikov-intel aelovikov-intel Awaiting requested review from aelovikov-intel aelovikov-intel is a code owner automatically assigned from intel/llvm-reviewers-runtime

@sergey-semenov sergey-semenov Awaiting requested review from sergey-semenov

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Alexandr-Konovalov @sergey-semenov