-
Notifications
You must be signed in to change notification settings - Fork 788
[SYCL] Attach auxiliary resources to memory objects #5588
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] Attach auxiliary resources to memory objects #5588
Conversation
Some reductions may require additional resources. These resources can be buffers, so to avoid introducing unintended synchronization points these resources must be stored until later. Previously however, storing these resources were done as part of the associated command, which could mean that there would be a circular reference keeping both alive, consequently causing a memory leak. These changes introduce a new strategy for these auxiliary resources as being "attached" to memory objects (USM and SYCLMemObjI). Attachments are tracked by the scheduler and when a memory object is being freed it will detach - and for USM defer the deletion of - the resources. Signed-off-by: Steffen Larsen <[email protected]>
/summary:run |
Signed-off-by: Steffen Larsen <[email protected]>
There was an issue with deferring the release of resources attached to USM due to the delayed release happening after context destruction. It has now been changed to release on detachment, which may cause USM @romanovvlad @sergey-semenov @s-kanaev - Would this be a problem? Alternative is to transfer the attached resource to the associated context and have it release it when being destroyed. |
Signed-off-by: Steffen Larsen <[email protected]>
Based on previous testing, the issues do appear to have been fixed now. Added the missing windows symbols. |
/summary:run |
/verify with intel/llvm-test-suite#624 |
It should not be a problem since the operation these buffer would wait during destruction is supposed to be completed in |
Signed-off-by: Steffen Larsen <[email protected]>
Signed-off-by: Steffen Larsen <[email protected]>
After offline discussion with @romanovvlad we agreed that there is no good way to consistently release memory object (not USM) resources managed by contexts. When we can break ABI we should have the memory objects hold the resources, but until then we will let the global handler manage a map for these. |
Signed-off-by: Steffen Larsen <[email protected]>
/verify with intel/llvm-test-suite#624 |
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.
LGTM
Closing this in favor of a stricter solution: #5653 |
Some reductions may require additional resources. These resources can be buffers, so to avoid introducing unintended synchronization points these resources must be stored until later. Previously however, storing these resources were done as part of the associated command, which could mean that there would be a circular reference keeping both alive, consequently causing a memory leak.
These changes introduce a new strategy for these auxiliary resources as being "attached" to memory objects (USM and SYCLMemObjI). Attachments are tracked by the scheduler and when a memory object is being freed it will detach the resources.