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 2020/07/29 07:35:42 UTC
[GitHub] [incubator-mxnet] andrei5055 commented on a change in pull request #18570: Reducing the number of calls of the NDArray constructors
andrei5055 commented on a change in pull request #18570:
URL: https://github.com/apache/incubator-mxnet/pull/18570#discussion_r461745054
##########
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:
As I mentioned in the PR **Description** (see **Changes**), that and a couple of other macros are defined differently for different values of `MXNET_USE_MKLDNN` flag:
```
#if MXNET_USE_MKLDNN == 1
#define INVALIDATE_OUTPUTS(outputs, req) InvalidateOutputs(&outputs, req)
#else
#define INVALIDATE_OUTPUTS(outputs, ...) // empty macros
#endif
```
It allows us to eliminate the multiple usages of `#if MXNET_USE_MKLDNN == 1 / #endif lines and simplify the code.
##########
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:
As I mentioned in the PR **Description** (see **Changes**), that and a couple of other macros are defined differently for different values of `MXNET_USE_MKLDNN` flag:
```
#if MXNET_USE_MKLDNN == 1
#define INVALIDATE_OUTPUTS(outputs, req) InvalidateOutputs(&outputs, req)
#else
#define INVALIDATE_OUTPUTS(outputs, ...) // empty macros
#endif
```
This simple trick allows us to eliminate the multiple usages of `#if MXNET_USE_MKLDNN == 1 / #endif lines and simplify the code.
##########
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:
It looks like I lost this comment line when I was moving ```static inline void CreateDefaultInputs``` from one file into another. I just recreated it and rebaised this PR branch.
##########
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:
Frankly, I am not familiar with `constexpr if` notion. I will take a look at it, to see if it is possible to use it as effective as empty macros.
----------------------------------------------------------------
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