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

Stop swapping in Storages of the wrong device for Tensors. (#18831) #18900

Closed
wants to merge 1 commit into from

Conversation

gchanan
Copy link
Contributor

@gchanan gchanan commented Apr 4, 2019

Stack from ghstack:

  • (to be filled)

Summary:
Pull Request resolved: #18831
ghimport-source-id: 2741e0d

Stack from ghstack:

This is necessary to support device caching, see #18751 and #18578.

In library code, we potentially swap in Storages with the wrong device when device_guard is False. This happens as follows with "view-like" operations.

  1. We allocate a tensor on the 'wrong' device (because device_guard is false).
  2. We swap out the 'wrong' storage with the 'right' storage using e.g. THCTensor_setStorage.

Instead, we can just construct the Tensor with the correct Storage from the beginning. This is what we do with 'view'.

Note there are two other "view-like" cases where this happens:

  1. unfold
  2. set_()

Because these aren't performance critical, I just added the device_guard instead of applying the above correction.

For completeness, this also includes a test that all device_guard: false functions behave properly under these conditions.

Reviewed By: dzhulgakov

Differential Revision: D14766232

fbshipit-source-id: 0865c3ddae3f415df5da7a9869b1ea9f210e81bc

Summary:
Pull Request resolved: #18831
ghimport-source-id: 2741e0d

Stack from [ghstack](https://github.com/ezyang/ghstack):
* #18833 [STACK] Cache device on TensorImpl; clean up TensorImpl constructors.
* #18832 [STACK] Disallow changing the device of a tensor via set_.
* **#18831 [STACK] Stop swapping in Storages of the wrong device for Tensors.**

This is necessary to support device caching, see #18751 and #18578.

In library code, we potentially swap in Storages with the wrong device when device_guard is False.  This happens as follows with "view-like" operations.
1) We allocate a tensor on the 'wrong' device (because device_guard is false).
2) We swap out the 'wrong' storage with the 'right' storage using e.g. THCTensor_setStorage.

Instead, we can just construct the Tensor with the correct Storage from the beginning.  This is what we do with 'view'.

Note there are two other "view-like" cases where this happens:
1) unfold
2) set_()

Because these aren't performance critical, I just added the device_guard instead of applying the above correction.

For completeness, this also includes a test that all `device_guard: false` functions behave properly under these conditions.

Reviewed By: dzhulgakov

Differential Revision: D14766232

fbshipit-source-id: 0865c3ddae3f415df5da7a9869b1ea9f210e81bc
@facebook-github-bot facebook-github-bot added the oncall: jit Add this issue/PR to JIT oncall triage queue label Apr 4, 2019
@gchanan gchanan closed this Apr 5, 2019
@facebook-github-bot facebook-github-bot deleted the gh/gchanan/15/head branch October 28, 2019 22:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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

2 participants