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

Revert PR 17767 for fixing GPU memory usage regression #18283

Merged
merged 2 commits into from
May 13, 2020
Merged

Revert PR 17767 for fixing GPU memory usage regression #18283

merged 2 commits into from
May 13, 2020

Conversation

rondogency
Copy link
Contributor

Revert PR #17767 for fixing #18280 GPU memory usage regression

Tested locally and observed GPU memory decreasing after reverting

Resolved revert conflict in elemwise_binrary_ops.h and test_operators.py

@mxnet-bot
Copy link

Hey @rondogency , Thanks for submitting the PR
All tests are already queued to run once. If tests fail, you can trigger one or more tests again with the following commands:

  • To trigger all jobs: @mxnet-bot run ci [all]
  • To trigger specific jobs: @mxnet-bot run ci [job1, job2]

CI supported jobs: [sanity, centos-cpu, edge, miscellaneous, unix-gpu, clang, website, windows-gpu, windows-cpu, unix-cpu, centos-gpu]

Note:
Only following 3 categories can trigger CI :PR Author, MXNet Committer, Jenkins Admin.
All CI tests must pass before the PR can be merged.

@apeforest
Copy link
Contributor

apeforest commented May 12, 2020 •
edited
Loading

@rondogency could you change the description to "... fixes #18280". Only if you use the magic word "fix" or "fixes", it will auto close the issue. Thanks

@apeforest apeforest requested a review from ptrendx May 12, 2020 19:06
@apeforest
Copy link
Contributor

@ptrendx Are you okay with this revert? Thanks!

@ptrendx
Copy link
Member

ptrendx commented May 12, 2020

Yes, I'm fine with it, although I would not want this PR to close that issue, as there is discussion happening there on an actual fix.

@rondogency
Copy link
Contributor Author

I agree with @ptrendx that we should not close the issue, also I noticed that this one is picked into 1.7 release, we should revert it also

@rondogency
Copy link
Contributor Author

@mxnet-bot run ci [all]

@mxnet-bot
Copy link

Jenkins CI successfully triggered : [windows-cpu, centos-gpu, edge, sanity, miscellaneous, unix-cpu, website, unix-gpu, windows-gpu, centos-cpu, clang]

@rondogency
Copy link
Contributor Author

@mxnet-bot run ci [centos-cpu, miscellaneous, unix-gpu, windows-gpu, windows-cpu, unix-cpu, centos-gpu]

@mxnet-bot
Copy link

Jenkins CI successfully triggered : [miscellaneous, unix-gpu, centos-cpu, unix-cpu, windows-gpu, centos-gpu, windows-cpu]

@josephevans
Copy link
Contributor

@mxnet-bot run ci [centos-gpu]

@mxnet-bot
Copy link

Unauthorized access detected.
Only following 3 categories can trigger CI :
PR Author, MXNet Committer, Jenkins Admin.

@leezu
Copy link
Contributor

leezu commented May 12, 2020

As @ptrendx targets having runtime compilation for cuda kernels in 2.0, I wonder if this should be reverted on 1.x branch only?

@rondogency
Copy link
Contributor Author

@mxnet-bot run ci [centos-gpu, unix-cpu, unix-gpu, windows-cpu]

@mxnet-bot
Copy link

Jenkins CI successfully triggered : [windows-cpu, unix-cpu, centos-gpu, unix-gpu]

@rondogency
Copy link
Contributor Author

@mxnet-bot run ci [centos-cpu]

@mxnet-bot
Copy link

Jenkins CI successfully triggered : [centos-cpu]

@rondogency
Copy link
Contributor Author

@leezu we need to resolve the current regression first and ptrendx would work on a fix

@rondogency
Copy link
Contributor Author

@ptrendx can you help to merge it, thanks!

@leezu leezu merged commit 47a38d1 into apache:master May 13, 2020
@rondogency rondogency deleted the revert-17767 branch May 13, 2020 20:53
@rondogency
Copy link
Contributor Author

@ciyongch Can you help to cherry-pick this into 1.x and 1.7 release, as this addresses a memory regression fix

rondogency added a commit to rondogency/incubator-mxnet that referenced this pull request May 14, 2020
@rondogency
Revert PR 17767 for fixing GPU memory usage regression (apache#18283)
ac4fce1 
* Revert "Fix and optimize handling of vectorized memory accesses (apache#17767)"

This reverts commit 5542d03.

* add license to reverted file
@ciyongch
Copy link
Contributor

Thanks @rondogency , do you mind to create a PR to 1.7 as well, as it's already merge/created in master/v1.x now :)

@rondogency
Copy link
Contributor Author

@ciyongch yes I will do it shortly

rondogency added a commit to rondogency/incubator-mxnet that referenced this pull request May 14, 2020
@rondogency
Revert PR 17767 for fixing GPU memory usage regression (apache#18283)
ea301d3 
* Revert "Fix and optimize handling of vectorized memory accesses (apache#17767)"

This reverts commit 5542d03.

* add license to reverted file
ptrendx pushed a commit that referenced this pull request May 27, 2020
@rondogency
Revert PR 17767 for fixing GPU memory usage regression (#18283) (#18311)
4033a85 
* Revert "Fix and optimize handling of vectorized memory accesses (#17767)"

This reverts commit 5542d03.

* add license to reverted file
eric-haibin-lin pushed a commit that referenced this pull request May 29, 2020
@rondogency
Revert PR 17767 for fixing GPU memory usage regression (#18283) (#18309)
d621e50 
* Revert "Fix and optimize handling of vectorized memory accesses (#17767)"

This reverts commit 5542d03.

* add license to reverted file
AntiZpvoh pushed a commit to AntiZpvoh/incubator-mxnet that referenced this pull request Jul 6, 2020
@rondogency @AntiZpvoh
Revert PR 17767 for fixing GPU memory usage regression (apache#18283)
f603590 
* Revert "Fix and optimize handling of vectorized memory accesses (apache#17767)"

This reverts commit 5542d03.

* add license to reverted file
ChaiBapchya pushed a commit to ChaiBapchya/mxnet that referenced this pull request Aug 15, 2020
@rondogency @ChaiBapchya
Revert PR 17767 for fixing GPU memory usage regression (apache#18283) (…
47ea684 
…apache#18309)

* Revert "Fix and optimize handling of vectorized memory accesses (apache#17767)"

This reverts commit 5542d03.

* add license to reverted file
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@ptrendx ptrendx ptrendx approved these changes

@apeforest apeforest apeforest approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@rondogency @mxnet-bot @apeforest @ptrendx @josephevans @leezu @ciyongch