yuxihu commented on a change in pull request #14615: Add PushAsyncPtr and
PushSyncPtr APIs in engine
URL: https://github.com/apache/incubator-mxnet/pull/14615#discussion_r272042076
##########
File path: include/mxnet/engine.h
##########
@@ -120,6 +120,10 @@ class MXNET_API Engine {
typedef std::function<void(RunContext)> SyncFn;
/*! \brief Asynchronous operation to pass to engine. */
typedef std::function<void(RunContext, CallbackOnComplete)> AsyncFn;
+ /*! \brief Synchronous operation (function pointer) to pass to engine. */
+ typedef void (*SyncFnPtr)(RunContext, const std::shared_ptr<void>&);
Review comment:
I wanted to use reference to avoid unnecessary copy of shared_ptr which is
an expensive operation. I did some simple experiment and observed that passing
shared_ptr by value can be 20x slower compared to passing by reference. In
addition, in the implementation of PushAsyncPtr/PushSyncPtr functions, when we
create a [lambda
function](https://github.com/apache/incubator-mxnet/pull/14615/files#diff-86dd2a82be9dffd682cbdd5c13490ac9R76),
we captures the param, which is a copy operation. This is why I decide to use
reference here. Then using const to prevent it from being modified.
----------------------------------------------------------------
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:
us...@infra.apache.org
With regards,
Apache Git Services
