[cudax] Add synchronous_resource_adapter and use it in async_buffer #6432
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This comment has been minimized.
This comment has been minimized.
pciolkosz
force-pushed
the
add_synchronous_resource_adapter_and_use_it_in_async_buffer
branch
2 times, most recently
from
9169f81 to
29e3ddf
Compare
This comment has been minimized.
This comment has been minimized.
pciolkosz
force-pushed
the
add_synchronous_resource_adapter_and_use_it_in_async_buffer
branch
from
29e3ddf to
9d8784f
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
davebayer
approved these changes
Nov 6, 2025
Comment on lines
+43
to
+44
| template <class _Resource> | ||
| struct synchronous_resource_adapter : ::cuda::mr::__copy_default_queries<_Resource> |
Contributor
There was a problem hiding this comment.
Q: should the adapter own the resource?
Contributor
Author
There was a problem hiding this comment.
I think it should, otherwise you run into lifetime issues
cudax/include/cuda/experimental/__memory_resource/synchronous_resource_adapter.cuh
Outdated
Show resolved
Hide resolved
…resource_adapter.cuh Co-authored-by: David Bayer <[email protected]>
This comment has been minimized.
This comment has been minimized.
…ource_adapter_and_use_it_in_async_buffer
pciolkosz
force-pushed
the
add_synchronous_resource_adapter_and_use_it_in_async_buffer
branch
from
8e34cb5 to
8b1b323
Compare
This comment has been minimized.
This comment has been minimized.
Contributor
|
pre-commit.ci autofix |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
NVIDIA#6352) * Add pointer attributes fallback to async buffer initialization * Add else branch instead of return because of MSVC * Fix format * Update cudax/include/cuda/experimental/__container/async_buffer.cuh Co-authored-by: David Bayer <[email protected]> * Fix format --------- Co-authored-by: David Bayer <[email protected]>
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Contributor
🥳 CI Workflow Results🟩 Finished in 2h 14m: Pass: 100%/120 | Total: 2d 15h | Max: 1h 33m | Hits: 87%/229477See results here. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We have the legacy non-stream ordered memory resources, but we don't have a way to use them in a buffer type.
While we could add a
synchronous_buffertype along withasync_bufferwe currently have, this approach has a number of issues:synchronous_bufferand most likely that would be a stream ordered initialization. It makes no difference if the allocation was stream ordered or not, after the initialization is submitted into a stream the buffer has a dependency on that stream and it would be nice if we could express that in the buffer type, like we do inasync_bufferasync_buffertosynchronous_buffer(and back?) on synchronization or launching new stream ordered work is not the most convenient. Instead [cudax] Make stream in async_buffer optional #6416 makes the stream inasync_bufferoptional, so you can add and remove stream dependency fromasync_buffereasily.The direction we would like to take instead is to rename
async_bufferto justbufferand make it work with legacy resources. This PR addssynchronous_resource_adapter, which is a generic utility wrapper that can extendsynchronous_resourceto a fullresource. It forwardsallocate_syncanddeallocate_syncto the wrapper resource, while forallocateanddeallocate, it forwards it if possible, otherwise uses the_syncvariants. In order to makedeallocatecorrect it also synchronizes the stream in case forwarding was not possible. The adapter is generic and will work for any customsynchronous_resource.This PR also makes
async_bufferautomatically apply the added adapter to a resource used to create a buffer if it does not have the stream ordered allocation/deallocation. This way legacy resources can be used with that type just like the memory pools.After this and #6416 is merged a future change will rename
async_bufferto justbuffer