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.

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

CUDA: Provide stream in async_done result #7453

Merged
merged 2 commits into from Oct 5, 2021

Conversation

gmarkall
Copy link
Member

@gmarkall gmarkall commented Oct 1, 2021

It's awkward to await on a Stream.async_done() for the purpose of enqueuing something new onto the stream, because there is no way to tell which stream finished from the completed future - this forces the user to maintain a mapping between futures and streams.

This PR makes the result of the future be the stream on which operations completed, so that the user can obtain the stream from the completed future.

It's awkward to await on a `Stream.async_done()` for the purpose of
enqueuing something new onto the stream, because there is no way to tell
which stream finished from the completed future - this forces the user
to maintain a mapping between futures and streams.

This commit makes the result of the future be the stream on which
operations completed, so that the user can obtain the stream from the
completed future.
@gmarkall gmarkall added CUDA CUDA related issue/PR 3 - Ready for Review labels Oct 1, 2021
@gmarkall gmarkall added this to the Numba 0.55 RC milestone Oct 1, 2021
Copy link
Contributor

@stuartarchibald stuartarchibald left a comment

Choose a reason for hiding this comment

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

Thanks for the patch, couple of minor things to look at else looks good. This new capability will be useful for e.g. data streams with non-uniform workloads.

numba/cuda/cudadrv/driver.py Show resolved Hide resolved
numba/cuda/tests/cudadrv/test_streams.py Outdated Show resolved Hide resolved
@stuartarchibald stuartarchibald added 4 - Waiting on author Waiting for author to respond to review Effort - short Short size effort needed and removed 3 - Ready for Review labels Oct 5, 2021
Update numba/cuda/tests/cudadrv/test_streams.py

Co-authored-by: stuartarchibald <stuartarchibald@users.noreply.github.com>
@gmarkall gmarkall added 4 - Waiting on reviewer Waiting for reviewer to respond to author and removed 4 - Waiting on author Waiting for author to respond to review labels Oct 5, 2021
Copy link
Contributor

@stuartarchibald stuartarchibald left a comment

Choose a reason for hiding this comment

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

Thanks for the patch!

@stuartarchibald stuartarchibald added 4 - Waiting on CI Review etc done, waiting for CI to finish Pending BuildFarm For PRs that have been reviewed but pending a push through our buildfarm and removed 4 - Waiting on reviewer Waiting for reviewer to respond to author labels Oct 5, 2021
@stuartarchibald
Copy link
Contributor

Buildfarm ID: numba_smoketest_cuda_yaml_97.

@stuartarchibald stuartarchibald removed Pending BuildFarm For PRs that have been reviewed but pending a push through our buildfarm 4 - Waiting on CI Review etc done, waiting for CI to finish labels Oct 5, 2021
@stuartarchibald
Copy link
Contributor

Buildfarm ID: numba_smoketest_cuda_yaml_97.

Passed.

@stuartarchibald stuartarchibald added 5 - Ready to merge Review and testing done, is ready to merge BuildFarm Passed For PRs that have been through the buildfarm and passed labels Oct 5, 2021
@sklam sklam merged commit fd6a059 into numba:master Oct 5, 2021
@MurrayData
Copy link

This is great, thank you @gmarkall, really useful for streams with non-uniform workloads, as @stuartarchibald said, such as the satellite data feeds I work with.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
5 - Ready to merge Review and testing done, is ready to merge BuildFarm Passed For PRs that have been through the buildfarm and passed CUDA CUDA related issue/PR Effort - short Short size effort needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants