Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add an option to getWriteableTensorData to avoid copy CUDA tensor to CPU #46524

Closed
wants to merge 5 commits into from

Conversation

mrshenli
Copy link
Contributor

@mrshenli mrshenli commented Oct 19, 2020

Stack from ghstack:

Differential Revision: D24392794

@facebook-github-bot facebook-github-bot added the oncall: jit Add this issue/PR to JIT oncall triage queue label Oct 19, 2020
@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Oct 19, 2020

💊 CI failures summary and remediations

As of commit 39396f4 (more details on the Dr. CI page):


  • 1/1 failures possibly* introduced in this PR
    • 1/1 non-CircleCI failure(s)---

Extra GitHub checks: 1 failed


This comment was automatically generated by Dr. CI (expand for details).Follow this link to opt-out of these comments for your Pull Requests.

Please report bugs/suggestions on the GitHub issue tracker or post in the (internal) Dr. CI Users group.

See how this bot performed.

This comment has been revised 3 times.

@dr-ci
Copy link

dr-ci bot commented Oct 19, 2020

💊 CI failures summary and remediations

As of commit ad3f662 (more details on the Dr. CI page):


None of the CI failures appear to be your fault 💚



🚧 2 fixed upstream failures:

These were probably caused by upstream breakages that were already fixed.

Please rebase on the viable/strict branch (expand for instructions)

Since your merge base is older than viable/strict, run these commands:

git fetch https://github.com/pytorch/pytorch viable/strict
git rebase FETCH_HEAD

Check out the recency history of this "viable master" tracking branch.


This comment was automatically generated by Dr. CI (expand for details).Follow this link to opt-out of these comments for your Pull Requests.

Please report bugs/suggestions on the GitHub issue tracker or post in the (internal) Dr. CI Users group.

See how this bot performed.

This comment has been revised 14 times.

Copy link
Contributor

@lw lw left a comment

Choose a reason for hiding this comment

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

Looks good to me, but perhaps someone from the JIT team should also take a look?

@mrshenli
Copy link
Contributor Author

Hey @wanchaol, I added this because we need to access WritableTensorData in RPC and would like to avoid the cpu-to-gpu copy. Please let me know if this looks OK to you. If not, we can also try expose WritableTensorData struct from JIT or replicate the logic in RPC.

Copy link
Contributor

@wanchaol wanchaol left a comment

Choose a reason for hiding this comment

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

This looks good to me.

torch/csrc/jit/serialization/pickler.cpp Outdated Show resolved Hide resolved
@facebook-github-bot
Copy link
Contributor

@mrshenli merged this pull request in 5003fd1.

@facebook-github-bot facebook-github-bot deleted the gh/mrshenli/256/head branch October 24, 2020 14:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Merged oncall: jit Add this issue/PR to JIT oncall triage queue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants