[GitHub] [incubator-mxnet] leezu commented on a change in pull request #18570: Reducing the number of calls of the NDArray constructors

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

leezu commented on a change in pull request #18570:
URL: https://github.com/apache/incubator-mxnet/pull/18570#discussion_r470240442



##########
File path: src/imperative/imperative_utils.h
##########
@@ -35,6 +35,59 @@
 #define MXNET_IMPERATIVE_IMPERATIVE_UTILS_H_
 
 namespace mxnet {
+
+#if MXNET_USE_MKLDNN == 1
+template<typename T>T *pntr(T &obj)           { return &obj; }  // NOLINT
+template<typename T>T *pntr(T *obj)           { return obj; }
+
+template<typename T>
+void InvalidateOutputs(const std::vector<T> *pArrs,
+                        const std::vector<OpReqType> &reqs) {
+  auto arrs = *pArrs;
+  for (size_t i = 0; i < arrs.size(); i++) {
+    if (reqs[i] == kWriteTo || reqs[i] == kNullOp)
+      pntr(arrs[i])->InvalidateMKLDNNData();
+  }
+}
+
+static inline void CreateDefaultInputs(const std::vector<NDArray> &arrs,
+                                       std::vector<NDArray> *out_arrs) {
+  out_arrs->clear();
+  for (size_t i = 0; i < arrs.size(); ++i) {
+    if (arrs[i].IsMKLDNNData())
+      out_arrs->push_back(arrs[i].Reorder2Default());
+    else
+      out_arrs->push_back(arrs[i]);
+  }
+}
+
+static inline void CreateDefaultInputs(std::vector<NDArray> *pArrs) {
+  auto &&arrs = *pArrs;
+  for (size_t i = 0; i < arrs.size(); ++i)
+    arrs[i].SelfReorder2Default();
+}

Review comment:
       Making it private addresses my concern. Thank you.

##########
File path: src/imperative/imperative_utils.h
##########
@@ -35,6 +35,59 @@
 #define MXNET_IMPERATIVE_IMPERATIVE_UTILS_H_
 
 namespace mxnet {
+
+#if MXNET_USE_MKLDNN == 1
+template<typename T>T *pntr(T &obj)           { return &obj; }  // NOLINT
+template<typename T>T *pntr(T *obj)           { return obj; }
+
+template<typename T>
+void InvalidateOutputs(const std::vector<T> *pArrs,
+                        const std::vector<OpReqType> &reqs) {
+  auto arrs = *pArrs;
+  for (size_t i = 0; i < arrs.size(); i++) {
+    if (reqs[i] == kWriteTo || reqs[i] == kNullOp)
+      pntr(arrs[i])->InvalidateMKLDNNData();
+  }
+}
+
+static inline void CreateDefaultInputs(const std::vector<NDArray> &arrs,
+                                       std::vector<NDArray> *out_arrs) {
+  out_arrs->clear();
+  for (size_t i = 0; i < arrs.size(); ++i) {
+    if (arrs[i].IsMKLDNNData())
+      out_arrs->push_back(arrs[i].Reorder2Default());
+    else
+      out_arrs->push_back(arrs[i]);
+  }
+}
+
+static inline void CreateDefaultInputs(std::vector<NDArray> *pArrs) {
+  auto &&arrs = *pArrs;
+  for (size_t i = 0; i < arrs.size(); ++i)
+    arrs[i].SelfReorder2Default();
+}
+
+#define INVALIDATE_OUTPUTS(outputs, req) InvalidateOutputs(&outputs, req)

Review comment:
       > My guess is: the compiler is not smart enough to DO NOT generate the functions IsDefaultData, InvalidateOutputs, ... when they are empty. Therefore, they will be defined, called, and do nothing!
   
   My guess was that such small (empty) functions would be inlined. But it's fine to keep the empty macro trick




----------------------------------------------------------------
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