You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mxnet.apache.org by GitBox <gi...@apache.org> on 2021/07/15 08:11:07 UTC

[GitHub] [incubator-mxnet] bgawrych opened a new pull request #20450: [v1.x][Feature] Add flag for disabling oneDNN BRGEMM implementation of FC

bgawrych opened a new pull request #20450:
URL: https://github.com/apache/incubator-mxnet/pull/20450


   ## Description ##
   In new oneDNN version there are BRGEMM kernels for FullyConnected - it require special memory format of weights.
   This PR let user to decide if BRGEMM implementation should be used by flag - it can significantly speedup FC execution for large tensors (got 42% speedup on BERT with 64 batch size ) - feature disabled by default as it's not so efficient on small tensors.
   
   ## Checklist ##
   ### Essentials ###
   - [x] PR's title starts with a category (e.g. [BUGFIX], [MODEL], [TUTORIAL], [FEATURE], [DOC], etc)
   - [x] Changes are complete (i.e. I finished coding on this PR)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-mxnet] bgawrych commented on pull request #20450: [v1.x][Feature] Add flag for disabling oneDNN BRGEMM implementation of FC

Posted by GitBox <gi...@apache.org>.
bgawrych commented on pull request #20450:
URL: https://github.com/apache/incubator-mxnet/pull/20450#issuecomment-880603171


   @mxnet-bot run ci [unix-cpu]


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-mxnet] bgawrych commented on pull request #20450: [v1.x][Feature] Add flag for disabling oneDNN BRGEMM implementation of FC

Posted by GitBox <gi...@apache.org>.
bgawrych commented on pull request #20450:
URL: https://github.com/apache/incubator-mxnet/pull/20450#issuecomment-881611945


   @mxnet-bot run ci [unix-gpu]


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-mxnet] TaoLv commented on a change in pull request #20450: [v1.x][Feature] Add flag for disabling oneDNN BRGEMM implementation of FC

Posted by GitBox <gi...@apache.org>.
TaoLv commented on a change in pull request #20450:
URL: https://github.com/apache/incubator-mxnet/pull/20450#discussion_r685801003



##########
File path: src/operator/nn/mkldnn/mkldnn_base-inl.h
##########
@@ -312,7 +312,8 @@ inline static mkldnn::memory::desc GetFCWeightDesc(const NDArray &arr, int dtype
   for (size_t i = 0; i < dims.size(); i++) dims[i] = arr.shape()[i];
   auto format = mkldnn::memory::format_tag::any;
   // for batch 256 alexnet benchmark test
-  if (dims.size() == 2) {
+  const bool brgemm_disabled = dmlc::GetEnv("MXNET_MKLDNN_DISABLE_BRGEMM_FC", true);
+  if (dims.size() == 2 && brgemm_disabled) {

Review comment:
       Could you please provide more benchmarking data with different m/n/k and formats?
   
   BTW, actually `brgemm_disabled` looks misleading to me. According to the code change, i would rather call the flag `force_plain_format` or `force_ab_format`.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-mxnet] anko-intel commented on pull request #20450: [v1.x][Feature] Add flag for disabling oneDNN BRGEMM implementation of FC

Posted by GitBox <gi...@apache.org>.
anko-intel commented on pull request #20450:
URL: https://github.com/apache/incubator-mxnet/pull/20450#issuecomment-881594410


   @mxnet-bot run ci [unix-gpu]


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #20450: [v1.x][Feature] Add flag for disabling oneDNN BRGEMM implementation of FC

Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #20450:
URL: https://github.com/apache/incubator-mxnet/pull/20450#issuecomment-881594445


   Unauthorized access detected. 
   Only following 3 categories can trigger CI : 
   PR Author, MXNet Committer, Jenkins Admin.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #20450: [v1.x][Feature] Add flag for disabling oneDNN BRGEMM implementation of FC

Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #20450:
URL: https://github.com/apache/incubator-mxnet/pull/20450#issuecomment-880603245


   Jenkins CI successfully triggered : [unix-cpu]


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-mxnet] bgawrych commented on pull request #20450: [v1.x][Feature] Add flag for disabling oneDNN BRGEMM implementation of FC

Posted by GitBox <gi...@apache.org>.
bgawrych commented on pull request #20450:
URL: https://github.com/apache/incubator-mxnet/pull/20450#issuecomment-881193523


   @szha  Can you help with merge?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-mxnet] szha merged pull request #20450: [v1.x][Feature] Add flag for disabling oneDNN BRGEMM implementation of FC

Posted by GitBox <gi...@apache.org>.
szha merged pull request #20450:
URL: https://github.com/apache/incubator-mxnet/pull/20450


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-mxnet] anko-intel commented on a change in pull request #20450: [v1.x][Feature] Add flag for disabling oneDNN BRGEMM implementation of FC

Posted by GitBox <gi...@apache.org>.
anko-intel commented on a change in pull request #20450:
URL: https://github.com/apache/incubator-mxnet/pull/20450#discussion_r670376928



##########
File path: src/operator/nn/mkldnn/mkldnn_base-inl.h
##########
@@ -312,7 +312,8 @@ inline static mkldnn::memory::desc GetFCWeightDesc(const NDArray &arr, int dtype
   for (size_t i = 0; i < dims.size(); i++) dims[i] = arr.shape()[i];
   auto format = mkldnn::memory::format_tag::any;
   // for batch 256 alexnet benchmark test
-  if (dims.size() == 2) {
+  const bool brgemm_disabled = dmlc::GetEnv("MXNET_DISABLE_ONEDNN_BRGEMM_FC", true);

Review comment:
       It will be good to add description to docs/static_site/src/pages/api/faq/env_var.md
   Also I am not sure if for 1.x branch the name have to include ONEDNN ?
   so maybe MXNET_MKLDNN_DISABLE_BRGEMM_FC




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #20450: [v1.x][Feature] Add flag for disabling oneDNN BRGEMM implementation of FC

Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #20450:
URL: https://github.com/apache/incubator-mxnet/pull/20450#issuecomment-881612010


   Jenkins CI successfully triggered : [unix-gpu]


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #20450: [v1.x][Feature] Add flag for disabling oneDNN BRGEMM implementation of FC

Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #20450:
URL: https://github.com/apache/incubator-mxnet/pull/20450#issuecomment-880491032


   Hey @bgawrych , Thanks for submitting the PR 
   All tests are already queued to run once. If tests fail, you can trigger one or more tests again with the following commands: 
   - To trigger all jobs: @mxnet-bot run ci [all] 
   - To trigger specific jobs: @mxnet-bot run ci [job1, job2] 
   *** 
   **CI supported jobs**: [edge, sanity, centos-cpu, centos-gpu, windows-gpu, clang, miscellaneous, unix-gpu, unix-cpu, website, windows-cpu]
   *** 
   _Note_: 
    Only following 3 categories can trigger CI :PR Author, MXNet Committer, Jenkins Admin. 
   All CI tests must pass before the PR can be merged. 
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org