-
Notifications
You must be signed in to change notification settings - Fork 537
Enhance load_method to support optional planned memory allocator #8032
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
Enhance load_method to support optional planned memory allocator #8032
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/8032
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit 4956dac with merge base 2094659 ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
There was a problem hiding this 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 will let @shoumikhin take a look as well
@cptspacemanspiff thanks a lot for the idea and implementation, I think that's a great way to provide the customers yet another point of customization. I started leaving suggestings to the code, but then realized it may be more readable as a demo PR, please feel free to borrow and update yours. |
e943466
to
864f2cd
Compare
@shoumikhin |
- Updated the load_method signature to accept an optional runtime::HierarchicalAllocator parameter.
5d7be33
to
b55c86f
Compare
Can you look over this? |
@JacobSzwejbka has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
@JacobSzwejbka has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
@JacobSzwejbka has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
Summary
The module api for the executorch runtime is pretty nice, however the load_method function forces the user to use the heirarchical allocator auto-generated by load_method. This is less than ideal if we have to do anything special with our memory allocation.
My specific use-case is having the ability to share memory allocations between multiple methods. Specifically, I have a memory manager does all the bookwork, and the user can request a hierarchical allocator from the manager during the load method call.
This is related to issue #8030
That being said, this is probably useful in general for cases where a default 'buffer' is not desired, but where one would still like to use the Module extension.
Test plan
Currently testing has just been that it works, all it does is:
cc @JacobSzwejbka @dbort