-
Notifications
You must be signed in to change notification settings - Fork 712
[executorch][flat tensor] Store number of external tensors in flatbuffer #8483
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
base: gh/lucylq/42/base
Are you sure you want to change the base?
Conversation
Store the number of external tensors in each ExecutionPlan. This saves time during loading, as we can directly allocate memory for external tensors without doing a pass to count how many external tensors require allocation. Differential Revision: [D69618283](https://our.internmc.facebook.com/intern/diff/D69618283/) [ghstack-poisoned]
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/8483
Note: Links to docs will display an error until the docs builds have been completed. ❌ 5 New FailuresAs of commit cfd05d6 with merge base c8311e6 ( NEW FAILURES - The following jobs have failed:
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
|
This pull request was exported from Phabricator. Differential Revision: D69618283 |
Store the number of external tensors in each ExecutionPlan. This saves time during loading, as we can directly allocate memory for external tensors without doing a pass to count how many external tensors require allocation. Differential Revision: [D69618283](https://our.internmc.facebook.com/intern/diff/D69618283/) ghstack-source-id: 266371204 Pull Request resolved: #8483
This PR needs a
|
|
So this is an FC breaking change but thats fine since no one is using external constants yet. Otherwise adding things to the schema is a pretty high bar typically. I think this change overall is fine since the overhead is small, but I'd still prefer we icebox this change until its clear the overhead of the scan requires this. Since we only ever have to perform the scan if a .ptd was provided anyway. |
Oh just saw this - I would love to make things right before people start to use it.
Even if the scan is trivial, it is something not necessary at runtime and we probably want the runtime logic to be as simple as possible. |
|
Looks like this PR hasn't been updated in a while so we're going to go ahead and mark this as |
|
Looks like this PR hasn't been updated in a while so we're going to go ahead and mark this as |
Stack from ghstack (oldest at bottom):
Store the number of external tensors in each ExecutionPlan.
This saves time during loading, as we can directly allocate memory for external tensors without doing a pass to count how many external tensors require allocation.
Differential Revision: D69618283