Optimize cached op static memory allocation #11658

piiswrong · 2018-07-11T22:50:21Z

Description

(Brief description on what this PR is about)

Checklist

Essentials

Please feel free to remove inapplicable items for your PR.

The PR title starts with [MXNET-$JIRA_ID], where $JIRA_ID refers to the relevant JIRA issue created (except PRs with tiny changes)
Changes are complete (i.e. I finished coding on this PR)
All changes have test coverage:
Unit tests are added for small changes to verify correctness (e.g. adding a new operator)
Nightly tests are added for complicated/long-running ones (e.g. changing distributed kvstore)
Build tests will be added for build configuration changes (e.g. adding a new build option with NCCL)
Code is well-documented:
For user-facing API changes, API doc string has been updated.
For new C++ functions in header files, their functionalities and arguments are documented.
For new examples, README.md is added to explain the what the example does, the source of the dataset, expected performance on test set and reference to the original paper if applicable
Check the API doc at http://mxnet-ci-doc.s3-accelerate.dualstack.amazonaws.com/PR-$PR_ID/$BUILD_ID/index.html
To the my best knowledge, examples are either not affected by this change, or have been fixed to be compatible with this change

Changes

Feature1, tests, (and when applicable, API doc)
Feature2, tests, (and when applicable, API doc)

Comments

If this change is a backward incompatible change, why must this change be made.
Interesting edge cases to note here

piiswrong · 2018-07-11T23:15:24Z

what API doc?

zheng-da · 2018-07-12T03:07:07Z

include/mxnet/ndarray.h

@@ -513,6 +515,7 @@ class NDArray {
    // We can't reuse memory in a view.
    CHECK(!IsView());
    NDArray ret = *this;
+    ret.byte_offset_ = byte_offset_ + byte_offset;


this might be problematic. The way of testing if an array is a view uses both reuse_ and byte_offset_. /~https://github.com/apache/incubator-mxnet/blob/master/include/mxnet/ndarray.h#L147
if reuse_ is true, the array can't be a view. I wonder if setting byte_offset_ to non-zero can cause problems.

zheng-da · 2018-07-12T03:11:51Z

I think you should test on models with static memory alloc when mkldnn is enabled.

lupesko · 2018-09-11T08:35:57Z

@piiswrong looks like this is ready for merge?

stu1130 · 2018-09-21T22:39:34Z

@piiswrong bounce again

vandanavk · 2018-09-24T22:01:58Z

@haojin2 @apeforest for review

vrakesh · 2018-10-09T07:20:11Z

@haojin2 is this good to merge?

Roshrini · 2018-10-22T17:07:03Z

@piiswrong @eric-haibin-lin Is this PR ready to be merged?

ankkhedia · 2018-10-30T18:59:36Z

@piiswrong @eric-haibin-lin ping!
It seems like this PR is good to go. Could you please look into it?

kalyc · 2018-11-12T22:13:51Z

@eric-haibin-lin @piiswrong could you please take a look?

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

stu1130 · 2018-11-21T18:31:03Z

@mxnet-label-bot update [backend, pr-awaiting-merge]
@eric-haibin-lin feel free to review and change the label

pengzhao-intel · 2018-11-25T02:55:34Z

@ZhennanQin to aware the changes since we are WIP for Cached OP too.

KellenSunderland · 2018-11-26T02:35:40Z

@piiswrong When you have a second I believe this may need a rebase. Has some refs to a non-existing TVM commit, 290226e1c9a.

ZhennanQin · 2018-11-26T02:56:38Z

@pengzhao-intel I just reviewed this. It has nothing changed with cpu backend, but an optimization for GPU memory pool. So I think it's OK.

stu1130 · 2018-11-26T17:42:20Z

@mxnet-label-bot update [pr-awaiting-response]

roywei · 2018-12-11T00:44:34Z

@piiswrong ping to rebase and trigger ci, thanks!

sandeep-krishnamurthy · 2018-12-26T21:15:02Z

@piiswrong - Can you please suggest next steps in this PR? How can we take this forward?

Roshrini · 2019-01-08T00:28:18Z

@piiswrong Can you please look into failing CI builds? Thanks

stu1130 · 2019-01-16T21:31:49Z

@piiswrong could you please rebase the PR thanks!

sandeep-krishnamurthy · 2019-01-25T19:33:10Z

@zheng-da @pengzhao-intel - Any suggestions on next step with this optimization? How can we take this forward?

pengzhao-intel · 2019-01-26T12:36:42Z

@sandeep-krishnamurthy This PR is not related to CPU backend now.
Feel free to ping us if anything needs our help. Thanks a lot.

pinaraws · 2019-03-19T23:02:32Z

@mxnet-label-bot remove[pr-work-in-progress]

pinaraws · 2019-03-19T23:03:03Z

@nswamy @sandeep-krishnamurthy @anirudh2290 - Please consider closing this PR since there is no follow up from the authors since January.

abhinavs95 · 2019-03-27T22:51:40Z

@piiswrong Could you provide an update on the PR? Thanks

lupesko · 2019-04-09T00:17:24Z

This PR has been open with no follow up for a long time, more than 6 months now.
I suggest we close it, it can always be re-opened if deem appropriate.
@sandeep-krishnamurthy @nswamy - can you please look into the suggestion?

sandeep-krishnamurthy · 2019-04-09T00:28:47Z

To make sure we don't lose the work done in the PR, I have linked this PR in the issue about Gluon Memory optimization - #12226

Closing the PR due to inactivity.

piiswrong requested a review from anirudh2290 as a code owner July 11, 2018 22:50

zheng-da reviewed Jul 12, 2018

View reviewed changes

marcoabreu mentioned this pull request Jul 12, 2018

CI Problem: Build status not reflected on PR #11654

Open

sandeep-krishnamurthy added Backend Issues related to the backend of MXNet pr-awaiting-response PR is reviewed and waiting for contributor to respond labels Aug 8, 2018

piiswrong force-pushed the pool branch from 4c699da to 71bb8f2 Compare August 13, 2018 18:42

piiswrong added 5 commits August 23, 2018 11:34

fix memory allocation

9c37821

fix

e86b4a5

fix

72756aa

fix

8a15bd4

fix

1f1e29c

piiswrong force-pushed the pool branch from b148cf6 to 1f1e29c Compare August 23, 2018 18:34

zheng-da approved these changes Aug 23, 2018

View reviewed changes

marcoabreu added pr-awaiting-merge Review and CI is complete. Ready to Merge Backend Issues related to the backend of MXNet and removed Backend Issues related to the backend of MXNet pr-awaiting-response PR is reviewed and waiting for contributor to respond labels Nov 21, 2018

marcoabreu added pr-awaiting-response PR is reviewed and waiting for contributor to respond and removed Backend Issues related to the backend of MXNet pr-awaiting-merge Review and CI is complete. Ready to Merge labels Nov 26, 2018

sandeep-krishnamurthy added the pr-work-in-progress PR is still work in progress label Jan 25, 2019

marcoabreu removed the pr-work-in-progress PR is still work in progress label Mar 19, 2019

sandeep-krishnamurthy mentioned this pull request Apr 9, 2019

Memory optimization in GLUON #12226

Open

sandeep-krishnamurthy closed this Apr 9, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Optimize cached op static memory allocation #11658

Optimize cached op static memory allocation #11658

piiswrong commented Jul 11, 2018

piiswrong commented Jul 11, 2018

zheng-da Jul 12, 2018

zheng-da commented Jul 12, 2018

lupesko commented Sep 11, 2018

stu1130 commented Sep 21, 2018

vandanavk commented Sep 24, 2018

vrakesh commented Oct 9, 2018

Roshrini commented Oct 22, 2018

ankkhedia commented Oct 30, 2018

kalyc commented Nov 12, 2018 •

edited

Loading

stu1130 commented Nov 21, 2018 •

edited

Loading

pengzhao-intel commented Nov 25, 2018

KellenSunderland commented Nov 26, 2018

ZhennanQin commented Nov 26, 2018

stu1130 commented Nov 26, 2018

roywei commented Dec 11, 2018

sandeep-krishnamurthy commented Dec 26, 2018

Roshrini commented Jan 8, 2019

stu1130 commented Jan 16, 2019 •

edited

Loading

sandeep-krishnamurthy commented Jan 25, 2019

pengzhao-intel commented Jan 26, 2019

pinaraws commented Mar 19, 2019

pinaraws commented Mar 19, 2019

abhinavs95 commented Mar 27, 2019

lupesko commented Apr 9, 2019

sandeep-krishnamurthy commented Apr 9, 2019

Optimize cached op static memory allocation #11658

Optimize cached op static memory allocation #11658

Conversation

piiswrong commented Jul 11, 2018

Description

Checklist

Essentials

Changes

Comments

piiswrong commented Jul 11, 2018

zheng-da Jul 12, 2018

Choose a reason for hiding this comment

zheng-da commented Jul 12, 2018

lupesko commented Sep 11, 2018

stu1130 commented Sep 21, 2018

vandanavk commented Sep 24, 2018

vrakesh commented Oct 9, 2018

Roshrini commented Oct 22, 2018

ankkhedia commented Oct 30, 2018

kalyc commented Nov 12, 2018 • edited Loading

stu1130 commented Nov 21, 2018 • edited Loading

pengzhao-intel commented Nov 25, 2018

KellenSunderland commented Nov 26, 2018

ZhennanQin commented Nov 26, 2018

stu1130 commented Nov 26, 2018

roywei commented Dec 11, 2018

sandeep-krishnamurthy commented Dec 26, 2018

Roshrini commented Jan 8, 2019

stu1130 commented Jan 16, 2019 • edited Loading

sandeep-krishnamurthy commented Jan 25, 2019

pengzhao-intel commented Jan 26, 2019

pinaraws commented Mar 19, 2019

pinaraws commented Mar 19, 2019

abhinavs95 commented Mar 27, 2019

lupesko commented Apr 9, 2019

sandeep-krishnamurthy commented Apr 9, 2019

kalyc commented Nov 12, 2018 •

edited

Loading

stu1130 commented Nov 21, 2018 •

edited

Loading

stu1130 commented Jan 16, 2019 •

edited

Loading