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

Add test coverage for per-thread default streams #732

Open
brycelelbach opened this issue May 1, 2020 · 5 comments
Open

Add test coverage for per-thread default streams #732

brycelelbach opened this issue May 1, 2020 · 5 comments
Assignees
Labels
thrust For all items related to Thrust.

Comments

@brycelelbach
Copy link
Collaborator

PR NVIDIA/thrust#1128 added support for using the per-thread default stream in Thrust if it has been requested via CUDA_API_PER_THREAD_DEFAULT_STREAM. We don't have any tests for this codepath, or in fact any multi-threaded host tests. We should probably add some.

@rongou
Copy link

rongou commented May 16, 2020

Great idea! I'm seeing some synchronization issues in spark with per-thread default stream enabled, but not sure if the problem is with thrust or cudf (or both). Having some test coverage would definitely help.

@rongou
Copy link

rongou commented Jun 18, 2020

@brycelelbach I think I might have some time to work on this. Do you have any idea on how to test this effectively?

@brycelelbach
Copy link
Collaborator Author

Spawn a thread with std::thread, get the stream in the parent thread and the child thread by calling thrust::cuda_cub::stream, then compare them for equality?

We should probably also make this a CMake option so we can run tests with and without this.

@rongou
Copy link

rongou commented Jun 18, 2020

Ah yes I think we can easily test that we are using the correct streams. I'm also wondering if we can test the multi-threaded behavior, checking for race conditions, etc.

@alliepiper
Copy link
Collaborator

As discussed in today's meeting, we want to update both the per-thread stream and RDC options as follows:

  • Add a small number of dedicated tests that always enable / always disable these features (small, quick sanity checks)
  • Add a global option that enables / disables these features in all targets, when applicable

This will give each build some coverage, while letting us add wider coverage with CI.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
thrust For all items related to Thrust.
Projects
Status: Todo
Development

No branches or pull requests

5 participants