Make mkldnn Stream object thread_local and enable mkldnn thread-safe #17022
Conversation
bdaskalov
force-pushed
the
thread-safe-mkldnn
branch
from
db9eb25 to
40d6333
Compare
There was a problem hiding this comment.
@ezyang is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
@ezyang Looking at the MKLDNN source code it seems like the only place this is used is in scratchpad.cpp where it changes the underlying implementation. I don't think there is a way to know how a prebuild implementation is configured since it is not exposed anywhere in the public interface or the headers. Can you retry the failing CI runs? They all seem to be broken for unreleated reasons but I don't have the rights to rerun them. Also I already backported this change to 1.0.1. I can make a PR for that too if you guys plan on doing a bugfix release? |
…(#17022) Summary: This PR fixes following issue: pytorch/pytorch#16828 It is a combination of two things: 1) MKLDNN streams are not thread-safe but are currently shared between different threads. This change makes them thread_local 2) By default MKLDNN primitives can share global memory and can't be invoked from multiple threads. This PR enables the MKLDNN_ENABLE_CONCURRENT_EXEC cmake configuration option that makes them thread-safe. Pull Request resolved: pytorch/pytorch#17022 Differential Revision: D14069052 Pulled By: ezyang fbshipit-source-id: f8f7fcb86c40f5d751fb35dfccc2f802b6e137c6
|
@bdaskalov we dont plan to do v1.0.2, the next will be v1.1 which will be cut off of master. |
|
OK great. Will wait for 1.1 to come out then.
…On Wed, Feb 13, 2019 at 10:00 PM Soumith Chintala ***@***.***> wrote:
@bdaskalov <https://github.com/bdaskalov> we dont plan to do v1.0.2, the
next will be v1.1 which will be cut off of master.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#17022 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ACs8kEoCzQNp-RX4ldIUEf-A4AFEua3jks5vNNE_gaJpZM4a376V>
.
|
4 participants
This PR fixes following issue: #16828
It is a combination of two things: