Skip to content

Conversation

@SS-JIA
Copy link
Contributor

@SS-JIA SS-JIA commented Aug 18, 2025

Stack from ghstack (oldest at bottom):

Summary:

Allow VulkanBackend to load constant tensors from the NamedDataMap instead of the constant data section of the delegate blob.

Motivation

This enables several key results:

  • Unblocks delegate retargetability with other backends
  • Allows reducing peak memory usage when loading models by freeing constant weight data as it gets moved to the GPU

Changes

  • Allow TensorRef to be constructed with a FreeableBuffer rvalue
  • Add ability to load constant data from NamedDataMap in VulkanBackend.cpp
  • When prepacking, free the constant data pointer once it's been copied to the staging buffer

Test Plan:
CI

Validate results by collecting memory measurements in the next few diffs.

Differential Revision: D80460035

Summary:

Allow VulkanBackend to load constant tensors from the NamedDataMap instead of the constant data section of the delegate blob.

## Motivation

This enables several key results:

* Unblocks delegate retargetability with other backends
* Allows reducing peak memory usage when loading models by freeing constant weight data as it gets moved to the GPU

## Changes

* Allow `TensorRef` to be constructed with a `FreeableBuffer` rvalue
* Add ability to load constant data from `NamedDataMap` in `VulkanBackend.cpp`
* When prepacking, free the constant data pointer once it's been copied to the staging buffer

Test Plan:
CI

Validate results by collecting memory measurements in the next few diffs.

[ghstack-poisoned]
@pytorch-bot
Copy link

pytorch-bot bot commented Aug 18, 2025

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/13472

Note: Links to docs will display an error until the docs builds have been completed.

❌ 1 New Failure, 1 Unrelated Failure

As of commit 491055c with merge base 8ef9595 (image):

NEW FAILURE - The following job has failed:

BROKEN TRUNK - The following job failed but were present on the merge base:

👉 Rebase onto the `viable/strict` branch to avoid these failures

  • pull / test-binary-size-linux-gcc / linux-job (gh) (trunk failure)
    /pytorch/executorch/kernels/portable/cpu/op_stack.cpp:129:26: error: comparison of integer expressions of different signedness: ‘size_t’ {aka ‘long unsigned int’} and ‘ssize_t’ {aka ‘long int’} [-Werror=sign-compare]

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@meta-cla meta-cla bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Aug 18, 2025
@github-actions
Copy link

This PR needs a release notes: label

If your change should be included in the release notes (i.e. would users of this library care about this change?), please use a label starting with release notes:. This helps us keep track and include your important work in the next release notes.

To add a label, you can comment to pytorchbot, for example
@pytorchbot label "release notes: none"

For more information, see
https://github.com/pytorch/pytorch/wiki/PyTorch-AutoLabel-Bot#why-categorize-for-release-notes-and-how-does-it-work.

@SS-JIA
Copy link
Contributor Author

SS-JIA commented Aug 18, 2025

@SS-JIA has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

Summary:

Allow VulkanBackend to load constant tensors from the NamedDataMap instead of the constant data section of the delegate blob.

## Motivation

This enables several key results:

* Unblocks delegate retargetability with other backends
* Allows reducing peak memory usage when loading models by freeing constant weight data as it gets moved to the GPU

## Changes

* Allow `TensorRef` to be constructed with a `FreeableBuffer` rvalue
* Add ability to load constant data from `NamedDataMap` in `VulkanBackend.cpp`
* When prepacking, free the constant data pointer once it's been copied to the staging buffer

Test Plan:
CI

Validate results by collecting memory measurements in the next few diffs.

Differential Revision: [D80460035](https://our.internmc.facebook.com/intern/diff/D80460035)

[ghstack-poisoned]
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D80460035

Summary:

Allow VulkanBackend to load constant tensors from the NamedDataMap instead of the constant data section of the delegate blob.

## Motivation

This enables several key results:

* Unblocks delegate retargetability with other backends
* Allows reducing peak memory usage when loading models by freeing constant weight data as it gets moved to the GPU

## Changes

* Allow `TensorRef` to be constructed with a `FreeableBuffer` rvalue
* Add ability to load constant data from `NamedDataMap` in `VulkanBackend.cpp`
* When prepacking, free the constant data pointer once it's been copied to the staging buffer

Test Plan:
CI

Validate results by collecting memory measurements in the next few diffs.

Differential Revision: [D80460035](https://our.internmc.facebook.com/intern/diff/D80460035)

[ghstack-poisoned]
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D80460035

@facebook-github-bot facebook-github-bot merged commit 5bf624b into gh/SS-JIA/290/base Aug 19, 2025
102 of 106 checks passed
@facebook-github-bot facebook-github-bot deleted the gh/SS-JIA/290/head branch August 19, 2025 02:23
ref = compute_graph_->add_tensorref(
dims_vector, dtype, std::move(buffer.get()));
} else {
const uint8_t* tensor_data = constant_data_ + constant_bytes->offset();
Copy link
Contributor

Choose a reason for hiding this comment

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

nit assert offset != UINT64_MAX for debugging

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants