cjolivier01 commented on a change in pull request #9862: Fix a race condition 
in converting data layouts in MKLDNN.
URL: https://github.com/apache/incubator-mxnet/pull/9862#discussion_r170161424

 File path: src/operator/nn/mkldnn/mkldnn_base.cc
 @@ -270,9 +270,24 @@ void FallBackCompute(FCompute fn, const nnvm::NodeAttrs 
                      const std::vector<OpReqType> &req,
                      const std::vector<NDArray> &outputs) {
   std::vector<TBlob> in_blobs(inputs.size());
+  std::vector<NDArray> in_bufs;
   for (size_t i = 0; i < in_blobs.size(); i++) {
+    // If the input data isn't stored in the default format, we shouldn't
+    // call data() directly, which will change the layout of the NDArray.
+    // Instead, we should save the converted data in another NDArray.
+    // TODO(zhengda) we should use temp space to save the converted data.
+    if (inputs[i].IsDefaultData()) {
       in_blobs[i] = inputs[i].data();
+    } else {
+      in_bufs.emplace_back(inputs[i].shape(), inputs[i].ctx(),
 Review comment:
   when you add items to a vector, when it runs out of space for the items 
(which generally happens on the second item sometimes), it allocates memory for 
2x the current number of items and then copies all of the items (copy 
constructor or move, depending on a number of things), into to the new space 
and then frees the old space. this can happen over and over again, and the 
whole reallocate and copy can be very expensive. they tend to even show up as 
hotspots in vtune. so if you know how many items a vector may have, it?s always 
a good idea to call reserve() because otherwise you are guaranteed a lot of 
extra reallocing And copying.

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:

With regards,
Apache Git Services

Reply via email to