[GitHub] [incubator-mxnet] larroy commented on issue #14095: Optimize move semantics of NodeEntry

2019-05-20 Thread GitBox
larroy commented on issue #14095: Optimize move semantics of NodeEntry URL: https://github.com/apache/incubator-mxnet/pull/14095#issuecomment-494157061 Would you be willing to merge since it simplifies creating graphs on operators if you don't find the performance improvement justified?

[GitHub] [incubator-mxnet] larroy commented on issue #14095: Optimize move semantics of NodeEntry

2019-05-20 Thread GitBox
larroy commented on issue #14095: Optimize move semantics of NodeEntry URL: https://github.com/apache/incubator-mxnet/pull/14095#issuecomment-494148298 I don't think is premature optimization, is generally agreed that is better to emplace_back rather than push_back and create additional cop

[GitHub] [incubator-mxnet] larroy commented on issue #14095: Optimize move semantics of NodeEntry

2019-05-20 Thread GitBox
larroy commented on issue #14095: Optimize move semantics of NodeEntry URL: https://github.com/apache/incubator-mxnet/pull/14095#issuecomment-494147738 I just provided a microbenchmark where you can see it's 10% faster. Usually in C++ we try to avoid doing more work than necessary such as c

[GitHub] [incubator-mxnet] larroy commented on issue #14095: Optimize move semantics of NodeEntry

2019-05-20 Thread GitBox
larroy commented on issue #14095: Optimize move semantics of NodeEntry URL: https://github.com/apache/incubator-mxnet/pull/14095#issuecomment-494144633 Moving a shared pointer is faster than copying. When using move in the microbenchmark is 10% faster. Overall is not much and in the big pi

[GitHub] [incubator-mxnet] larroy commented on issue #14095: Optimize move semantics of NodeEntry

2019-05-20 Thread GitBox
larroy commented on issue #14095: Optimize move semantics of NodeEntry URL: https://github.com/apache/incubator-mxnet/pull/14095#issuecomment-494129614 @szha Performance is the same, so motivation is correctness and ease of graph building: Tested in a tight loop: ``` T

[GitHub] [incubator-mxnet] larroy commented on issue #14095: Optimize move semantics of NodeEntry

2019-05-20 Thread GitBox
larroy commented on issue #14095: Optimize move semantics of NodeEntry URL: https://github.com/apache/incubator-mxnet/pull/14095#issuecomment-494106277 @szha the initial motivation is to lean out node creation and for correctness since there are difficult to catch bugs depending on Node in

[GitHub] [incubator-mxnet] larroy commented on issue #14095: Optimize move semantics of NodeEntry

2019-05-20 Thread GitBox
larroy commented on issue #14095: Optimize move semantics of NodeEntry URL: https://github.com/apache/incubator-mxnet/pull/14095#issuecomment-494102521 @szha what would you suggest? This is an automated message from the Apache

[GitHub] [incubator-mxnet] larroy commented on issue #14095: Optimize move semantics of NodeEntry

2019-05-09 Thread GitBox
larroy commented on issue #14095: Optimize move semantics of NodeEntry URL: https://github.com/apache/incubator-mxnet/pull/14095#issuecomment-490925743 @mxnet-label-bot update [Backend,pr-awaiting-review] This is an automated