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

Revert PR 17767 for fixing GPU memory usage regression - #18283

Merged
leezu merged 2 commits into
apache:masterfrom
rondogency:revert-17767
May 13, 2020
Merged

Revert PR 17767 for fixing GPU memory usage regression#18283
leezu merged 2 commits into
apache:masterfrom
rondogency:revert-17767

Conversation

@rondogency

Copy link
Copy Markdown
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
Copy Markdown

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

apeforest commented May 12, 2020

Copy link
Copy Markdown
Contributor

@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
Copy Markdown
Contributor

@ptrendx Are you okay with this revert? Thanks!

@ptrendx

ptrendx commented May 12, 2020

Copy link
Copy Markdown
Member

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
Copy Markdown
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
Copy Markdown
Contributor Author

@mxnet-bot run ci [all]

@mxnet-bot

Copy link
Copy Markdown

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

@rondogency

Copy link
Copy Markdown
Contributor Author

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

@mxnet-bot

Copy link
Copy Markdown

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

@josephevans

Copy link
Copy Markdown
Contributor

@mxnet-bot run ci [centos-gpu]

@mxnet-bot

Copy link
Copy Markdown

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

@leezu

leezu commented May 12, 2020

Copy link
Copy Markdown
Contributor

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
Copy Markdown
Contributor Author

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

@mxnet-bot

Copy link
Copy Markdown

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

@rondogency

Copy link
Copy Markdown
Contributor Author

@mxnet-bot run ci [centos-cpu]

@mxnet-bot

Copy link
Copy Markdown

Jenkins CI successfully triggered : [centos-cpu]

@rondogency

Copy link
Copy Markdown
Contributor Author

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

@rondogency

Copy link
Copy Markdown
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
Copy Markdown
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
* Revert "Fix and optimize handling of vectorized memory accesses (apache#17767)"

This reverts commit 5542d03.

* add license to reverted file
@ciyongch

Copy link
Copy Markdown
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
Copy Markdown
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
* 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
* 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
* 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
* 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
…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.

Labels

None yet

7 participants