-
Notifications
You must be signed in to change notification settings - Fork 607
[ET-VK] Clean up api::vTensor class #3149
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
Conversation
Differential Revision: [D55811279](https://our.internmc.facebook.com/intern/diff/D55811279/) [ghstack-poisoned]
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/3149
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit 1863fa6 with merge base 269b6ad ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
This pull request was exported from Phabricator. Differential Revision: D55811279 |
## Context Now that we have forked the `api/` directory from PyTorch Vulkan, we can clean up the `vTensor` class and remove functionality that is not necessary for the ExecuTorch Vulkan delegate. The following changes are made: * Remove unused member variables and member functions from `vTensor` and `vTensorStorage` * Remove all quantization related member variables, member functions, and the `vTensor` constructor for quantized tensors. The Quantization API will be reworked from the ground up. * Rename `view_` (which is an instance of `vTensorStorage`) to `storage_` Finally, the critical change that is introduced is that we now store `storage_` as a direct `vTensorStorage` member variable in `vTensor` instead of storing it as a `std::shared_ptr<vTensorStorage>`. For context, the reason `storage_` was stored as a shared pointer is to be compliant with ATen Tensors, which needs to enable copy construction to enable the following: ``` at::Tensor b = at::rand(...); // Oftentimes this will create a "view" of the tensor. a and b will point the the same underlying storage, but with different metadata. at::Tensor a = b; ``` However, in the ExecuTorch delegate this is no longer necessary. Each Tensor is associated with it's own independent storage and is responsible for managing it's own memory. **By getting rid of `std::shared_ptr`, we can avoid a heap allocation and avoid chasing pointers whenever we need to access the resources of a `vTensor`.** Differential Revision: [D55811279](https://our.internmc.facebook.com/intern/diff/D55811279/) [ghstack-poisoned]
This pull request was exported from Phabricator. Differential Revision: D55811279 |
This pull request has been merged in bf5093a. |
Stack from ghstack (oldest at bottom):
ParamsBindList
to prevent needing to passshared_ptr
to bind parameter UBOs #3150Value
#3148Context
Now that we have forked the
api/
directory from PyTorch Vulkan, we can clean up thevTensor
class and remove functionality that is not necessary for the ExecuTorch Vulkan delegate.The following changes are made:
vTensor
andvTensorStorage
vTensor
constructor for quantized tensors. The Quantization API will be reworked from the ground up.view_
(which is an instance ofvTensorStorage
) tostorage_
Finally, the critical change that is introduced is that we now store
storage_
as a directvTensorStorage
member variable invTensor
instead of storing it as astd::shared_ptr<vTensorStorage>
.For context, the reason
storage_
was stored as a shared pointer is to be compliant with ATen Tensors, which needs to enable copy construction to enable the following:However, in the ExecuTorch delegate this is no longer necessary. Each Tensor is associated with it's own independent storage and is responsible for managing it's own memory. By getting rid of
std::shared_ptr
, we can avoid a heap allocation and avoid chasing pointers whenever we need to access the resources of avTensor
.Differential Revision: D55811279