[GitHub] [incubator-mxnet] anirudh2290 commented on a change in pull request #14363: Support multi-threading for Custom Operator

[GitBox <gi...@apache.org>]

anirudh2290 commented on a change in pull request #14363: Support multi-threading for Custom Operator
URL: https://github.com/apache/incubator-mxnet/pull/14363#discussion_r263976175
 
 

 ##########
 File path: src/operator/custom/custom-inl.h
 ##########
 @@ -139,38 +143,51 @@ class CustomOperator {
       destructing_ = true;
       cv_.notify_all();
     }
-    worker_.join();
+    for (auto &worker : workers_)
+      worker.join();
 
 Review comment:
   Have you evaluated this when the custom op is part of a bigger graph and if there is any performance impact ? Since the CustomOperator is static its lifetime is till the end of the program and destructor of customoperator gets called at the end. This means there is one thread that is waiting on the condition variable while other 11 threads tryign to obtain the lock and in a blocked state. Since these are idle threads, I am not sure if the impact will be significant but good to verify. Will also help us come up with a good default for MXNET_CUSTOM_OP_NUM_THREADS

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services