Make cuDNN use the current stream #2984
Conversation
| "WARNING: Mismatch between user stream and the cuDNN handle context\n"; | ||
| } else if (status != CUDNN_STATUS_SUCCESS) { | ||
| std::cerr << "WARNING: Could not set cuDNN to use current stream\n"; | ||
| } |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
|
It might break in a multi-threaded situation where multiple threads work on the same device, use the same cudnn handle, release GIL and race to set different streams. Multi-threaded + single cudnn handle is a fragile configuration. |
|
I agree. Our threading model for THCState is very fragile in general. I think we've been good so far only because we usually use at most one thread per device. However I think this is no longer true in |
|
it breaks (remember nccl deadlocks that i'm fixing :) ) |
|
It's completely unrelated to nccl deadlocks |
Summary
cudnnSetStream calls were missing in the cudnn bindings, resulting in cudnn not using the current stream. This makes it so that the cudnn functions use the current stream. #2523
Test plan
Create a test file (adopted from here)
Ran it through
nvprof. Asserted that the cudnn functions did not use the current stream before changes. Asserted that the cudnn functions did use the current stream after changes.Before:
After:
