Skip to content

CUDA: Provide stream in async_done result - #7453

Merged
sklam merged 2 commits into
numba:masterfrom
gmarkall:async_done_stream
Oct 5, 2021
Merged

sklam merged 2 commits into
numba:masterfrom
gmarkall:async_done_stream

Conversation

@gmarkall

@gmarkall gmarkall commented Oct 1, 2021

Copy link
Copy Markdown
Member

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

@stuartarchibald stuartarchibald left a comment

Copy link
Copy Markdown
Contributor

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.

Comment thread numba/cuda/cudadrv/driver.py
Comment thread numba/cuda/tests/cudadrv/test_streams.py Outdated
@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

@stuartarchibald stuartarchibald left a comment

Copy link
Copy Markdown
Contributor

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
Copy Markdown
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
Copy Markdown
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
Copy Markdown

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.

4 participants