Skip to content
This repository has been archived by the owner on Nov 17, 2023. It is now read-only.

Optimize C++ API #13496

Merged
merged 2 commits into from
Dec 12, 2018
Merged

Optimize C++ API #13496

merged 2 commits into from
Dec 12, 2018

Conversation

zhaoyao73
Copy link
Contributor

Pass parameter with reference instead of value.
Add const as well as it is not changed.

Description

Optimize some functions to pass reference instead of value. This could avoid unnecessary construction/destruction and copy.

Checklist

Essentials

Please feel free to remove inapplicable items for your PR.

  • tiny changes
  • did a successful build
  • For user-facing API changes, API doc string has been updated.
    Didn't find the API doc in mxnet.io/doxygen
  • To the my best knowledge, examples are either not affected by this change, or have been fixed to be compatible with this change

Changes

Comments

I am not sure I could run a sanity test with it or how to run a unit test/sanity test against the change. But pass value -> pass reference, should be transparent to C++, although not like C's pass value->pass pointer.

Pass parameter with reference instead of value.
Add const as well as it is not changed.
@vandanavk
Copy link
Contributor

@mxnet-label-bot add [C++, pr-awaiting-review]

@leleamol for review

@marcoabreu marcoabreu added C++ Related to C++ pr-awaiting-review PR is waiting for code review labels Nov 30, 2018
Fix BinaryShapeFunction typedef
Add a right brace for SmoothL1Shape_
@zhaoyao73 zhaoyao73 requested a review from szha as a code owner December 4, 2018 19:00
Copy link
Contributor

@sandeep-krishnamurthy sandeep-krishnamurthy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.
@leleamol

@zhaoyao73
Copy link
Contributor Author

any idea why centos-cpu still not done?
Is it the only thing need to be done?

@zhaoyao73
Copy link
Contributor Author

@marcoabreu May I ask why the centos-cpu still not done?

@marcoabreu
Copy link
Contributor

Seems like it hasn't been kicked off. Please rebase to start another build

Copy link
Contributor

@larroy larroy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@lebeg lebeg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@zhaoyao73
Copy link
Contributor Author

do I need to do anything?

@larroy
Copy link
Contributor

larroy commented Dec 10, 2018

@marcoabreu are we missing statuses from github? Check centos-cpu

@marcoabreu
Copy link
Contributor

@zhaoyao73 I have kicked off a build on your behalf. Sorry for the inconveniences.

@zhaoyao73
Copy link
Contributor Author

@marcoabreu just a dummy question, can I start a CI build?

@marcoabreu
Copy link
Contributor

Yeah, just make a new commit and it will automatically start a build

@larroy
Copy link
Contributor

larroy commented Dec 11, 2018

@mxnet-label-bot [pr-awaiting-merge]

@zhaoyao73
Copy link
Contributor Author

weird, look at this PR it is still pr-awaiting-review, although I see @larrroy put the pr-awaiting-merge?

@marcoabreu
Copy link
Contributor

You received two binding approvals, no requested changes and there are no outstanding discussions, thus the PR is ready to be merged.

@marcoabreu marcoabreu merged commit 002e0bb into apache:master Dec 12, 2018
zhaoyao73 added a commit to zhaoyao73/incubator-mxnet that referenced this pull request Dec 13, 2018
* upstream/master: (38 commits)
  Feature/mkldnn static (apache#13628)
  Fix the bug of BidirectionalCell (apache#13575)
  Set install path for libmxnet.so dynamic lib on Mac OS (apache#13629)
  add batch norm test (apache#13625)
  Scripts for building dependency libraries of MXNet (apache#13282)
  fix quantize pass error when the quantization supported Op are excluded in the model (apache#13596)
  Optimize C++ API (apache#13496)
  Fix warning in waitall doc (apache#13618)
  [MXNET-1225] Always use config.mk in make install instructions (apache#13364)
  [MXNET-1224]: improve scala maven jni build and packing. (apache#13493)
  [MXNET-1155] Add scala packageTest utility (apache#13046)
  fix the Float not showing correctly problem (apache#13617)
  apache#13385 [Clojure] - Turn examples into integration tests (apache#13554)
  Add Intel MKL blas to Jenkins (apache#13607)
  Revert "[MXNET-1198] MXNet Java API (apache#13162)"
  Reducing the length of setup tutorial (apache#13306)
  [MXNET-1182] Predictor example (apache#13237)
  [MXNET-1187] Added Java SSD Inference Tutorial for website (apache#13201)
  add defaults and clean up the tests (apache#13295)
  [MXNET-1181] Added command line alternative to IntelliJ in install instructions (apache#13267)
  ...
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
C++ Related to C++ pr-awaiting-review PR is waiting for code review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants