Skip to content

Conversation

JacobSzwejbka
Copy link
Contributor

@JacobSzwejbka JacobSzwejbka commented Apr 10, 2025

Summary:
Config option to not memory plan mutable buffers.

This when paired with a future runtime PR will allow users to retrieve buffers by name in the runtime and then set their dataptr.

Differential Revision: D72749868

cc @angelayi

Copy link

pytorch-bot bot commented Apr 10, 2025

🔗 Helpful Links

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

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

✅ No Failures

As of commit d8fa917 with merge base d42676f (image):
💚 Looks good so far! There are no failures yet. 💚

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

@facebook-github-bot facebook-github-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 Apr 10, 2025
@facebook-github-bot
Copy link
Contributor

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

JacobSzwejbka added a commit to JacobSzwejbka/executorch-1 that referenced this pull request Apr 10, 2025
Summary:

Config option to not memory plan mutable buffers.

This when paired with a future runtime PR will allow users to retrieve buffers by name in the runtime and then set their dataptr.

Differential Revision: D72749868
JacobSzwejbka added a commit to JacobSzwejbka/executorch-1 that referenced this pull request Apr 10, 2025
Summary:

Config option to not memory plan mutable buffers.

This when paired with a future runtime PR will allow users to retrieve buffers by name in the runtime and then set their dataptr.

Differential Revision: D72749868
@facebook-github-bot
Copy link
Contributor

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

@JacobSzwejbka JacobSzwejbka added release notes: api Changes to public facing apis (any interfaces, pybinded runtime methods, etc.) module: exir Issues related to Export IR and the code under exir/ labels Apr 10, 2025
@facebook-github-bot
Copy link
Contributor

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

JacobSzwejbka added a commit to JacobSzwejbka/executorch-1 that referenced this pull request Apr 10, 2025
Summary:
Pull Request resolved: pytorch#10071

Config option to not memory plan mutable buffers.

This when paired with a future runtime PR will allow users to retrieve buffers by name in the runtime and then set their dataptr.

Differential Revision: D72749868
JacobSzwejbka added a commit to JacobSzwejbka/executorch-1 that referenced this pull request Apr 10, 2025
Summary:

Config option to not memory plan mutable buffers.

This when paired with a future runtime PR will allow users to retrieve buffers by name in the runtime and then set their dataptr.

Differential Revision: D72749868
@facebook-github-bot
Copy link
Contributor

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

Summary:

Config option to not memory plan mutable buffers.

This when paired with a future runtime PR will allow users to retrieve buffers by name in the runtime and then set their dataptr.

Differential Revision: D72749868
@facebook-github-bot
Copy link
Contributor

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

spec.extra_tensor_info = ExtraTensorInfo(
fully_qualified_name=fqn, location=TensorDataLocation.SEGMENT

if is_mutable_buffer:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the actual core logic change, the rest of the changes are mostly piping a flag around

# Lower the graph to executorch.
ep = ep.to_executorch(
config=ExecutorchBackendConfig(emit_mutable_buffer_names=True)
config=ExecutorchBackendConfig(
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe also add a post_init in the ExecutorchBackendConfig dataclass that asserts if emit_mutable_buffer_names is False and alloc_mutable_buffers is also False.

Copy link
Contributor Author

@JacobSzwejbka JacobSzwejbka Apr 11, 2025

Choose a reason for hiding this comment

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

I considered this but since the MemoryPlanningPass is user configureable there is no guarantee memory_planning_pass.alloc_mutable_buffers exists. So I just check the end result in the emitter.

if is_mutable_buffer:
# Emit names if we are supposed to.
if self.emitter_state.emit_mutable_buffer_names:
if spec.extra_tensor_info is None:
Copy link
Contributor

Choose a reason for hiding this comment

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

A little confused by this as i haven't kept track of the tensor info changes, what does it mean if extra_tensor_info is None and if it isn't we overwrite the fqn again? Maybe add a small comment here too.

Copy link
Contributor Author

@JacobSzwejbka JacobSzwejbka Apr 11, 2025

Choose a reason for hiding this comment

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

None just means no one has created one, if its not none they might not have populated the fqn so I populate it. I guess I could check what it is before overwriting it but the name is unique so it should always be safe to do this.

extra_tensor_info is where we put optional info in the flatbuffer to not regress the size of every tensor by too much in embedded cases that dont need it.

@facebook-github-bot facebook-github-bot merged commit d9c31fa into pytorch:main Apr 11, 2025
82 of 84 checks passed
keyprocedure pushed a commit to keyprocedure/executorch that referenced this pull request Apr 21, 2025
Differential Revision: D72749868

Pull Request resolved: pytorch#10071
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 module: exir Issues related to Export IR and the code under exir/ release notes: api Changes to public facing apis (any interfaces, pybinded runtime methods, etc.)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants