[REVIEW] Explicitly set legacy or per-thread default stream in JNI [skip ci] #6690
Conversation
|
Please update the changelog in order to start CI tests. View the gpuCI docs here. |
| @@ -39,9 +45,9 @@ | |||
| protected boolean cleanImpl(boolean logErrorIfNotClean) { | |||
| boolean neededCleanup = false; | |||
| long origAddress = stream; | |||
| if (stream != 0) { | |||
| if (stream != defaultStream()) { | |||
There was a problem hiding this comment.
This should check for any stream that shouldn't be cleaned up which includes implicit default stream (0), legacy default stream (1), and per-thread default stream (2).
| destroyStream(stream); | ||
| stream = 0; | ||
| stream = defaultStream(); |
There was a problem hiding this comment.
Probably simplest to keep using 0 as an indicator the stream has been cleaned, that way we don't have to compute anything and it's consistent how custom streams look cleaned regardless of PTDS setting.
| private static long defaultStream() { | ||
| return isPtdsEnabled() ? CUDA_STREAM_PER_THREAD : CUDA_STREAM_LEGACY; | ||
| } |
There was a problem hiding this comment.
This doesn't need to be a function, since it's never going to change value. This should be computed once so we avoid the JNI call each time.
| private static long defaultStream() { | |
| return isPtdsEnabled() ? CUDA_STREAM_PER_THREAD : CUDA_STREAM_LEGACY; | |
| } | |
| private static long DEFAULT_STREAM_ID = isPtdsEnabled() ? CUDA_STREAM_PER_THREAD : CUDA_STREAM_LEGACY; |
|
@rongou I am going to test your change with my setup, since I saw this issue originally. |
|
@rongou I have tried your change and I have not seen the same issue with nvcomp. Thanks for doing this. |
This should make
nvcompuse the correct cuda stream.@jlowe @abellina