Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Automatic pulling ExtraFileMaps without explicit mapping. #99747

Merged
merged 1 commit into from
May 1, 2023
Merged

Automatic pulling ExtraFileMaps without explicit mapping. #99747

merged 1 commit into from
May 1, 2023

Conversation

kwanghoon-meta
Copy link
Contributor

Summary:
It comes handy when we are trying to load a module with ExtraFileMaps after JITing since BytecodeDeserializer loops over extra maps entries that is given from end-user APIs.

We can simply uses getAllRecord so that we can easily pull all extra files embedded without explicitly mapping with high level API

Note :

  • Flatbuffer by nature already extracts all embedded files without explicit mapping. I'm simply adding extra unit test for future refence if someone also interested in knowing later on

  • only lite interpreter needs a change as diff does

Test Plan:
buck2 run //caffe2/test/cpp/jit:jit -- --gtest_filter=LiteInterpreterTest.ExtraFiles

buck2 run //caffe2/test/cpp/jit:jit -- --gtest_filter=FlatbufferTest.ExtraFiles

Differential Revision: D45170126

@pytorch-bot pytorch-bot bot added the release notes: mobile release notes category label Apr 21, 2023
@pytorch-bot
Copy link

pytorch-bot bot commented Apr 21, 2023 •
edited
Loading

🔗 Helpful Links

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

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

✅ No Failures

As of commit 60b16dc:
💚 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
Copy link
Contributor

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

1 similar comment
@facebook-github-bot
Copy link
Contributor

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

@kwanghoon-meta kwanghoon-meta force-pushed the export-D45170126 branch 2 times, most recently from 059fbea to 4aa7c62 Compare April 22, 2023 00:28
@facebook-github-bot
Copy link
Contributor

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

@facebook-github-bot
Copy link
Contributor

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

@facebook-github-bot
Copy link
Contributor

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

1 similar comment
@facebook-github-bot
Copy link
Contributor

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

@facebook-github-bot
Copy link
Contributor

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

davidberard98
davidberard98 approved these changes Apr 28, 2023
View reviewed changes
Copy link
Contributor

@davidberard98 davidberard98 left a comment

Choose a reason for hiding this comment

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

nit - address linter issues

otherwise LGTM!

@facebook-github-bot
Copy link
Contributor

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

@facebook-github-bot
Copy link
Contributor

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

1 similar comment
@facebook-github-bot
Copy link
Contributor

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

@facebook-github-bot
Copy link
Contributor

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

1 similar comment
@facebook-github-bot
Copy link
Contributor

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

@kwanghoon-meta @facebook-github-bot
Automatic pulling ExtraFileMaps without explicit mapping. (pytorch#99747
60b16dc 
)

Summary:
Pull Request resolved: pytorch#99747

It comes handy when we are trying to load a module with ExtraFileMaps after JITing since [*BytecodeDeserializer*](https://www.internalfb.com/code/aros/[144773a900f80d6bdb2cbc35f817532200b545be]/xros/third-party/caffe2/caffe2/torch/csrc/jit/mobile/import.cpp?lines=196%2C389%2C408%2C569%2C611) loops over extra maps entries that is given from end-user APIs.

We can simply uses [getAllRecord](https://www.internalfb.com/code/fbsource/[c8b0b670f4782345c954a292dc20f562dd95e263]/xplat/caffe2/caffe2/serialize/inline_container.cc?lines=31%2C67%2C72%2C79%2C86%2C92%2C174%2C178%2C222%2C243%2C275%2C283%2C304%2C322) so that we can easily pull all extra files embedded without explicitly mapping with high level [API ](https://www.internalfb.com/code/fbsource/fbcode/caffe2/torch/csrc/jit/mobile/import.cpp?lines=548-563)

Note :
- Flatbuffer by nature already extracts all embedded files without explicit mapping. I'm simply adding extra unit test for future refence if someone also interested in knowing later on

- only lite interpreter needs a change as diff does

Test Plan:
buck2 run //caffe2/test/cpp/jit:jit -- --gtest_filter=LiteInterpreterTest.ExtraFiles

buck2 run //caffe2/test/cpp/jit:jit -- --gtest_filter=FlatbufferTest.ExtraFiles

Reviewed By: davidberard98

Differential Revision: D45170126

fbshipit-source-id: 57c8a43cd64931f5dcca4b36d306b23769d4ce5b
@facebook-github-bot
Copy link
Contributor

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

@facebook-github-bot facebook-github-bot merged commit 3fb0bf4 into pytorch:main May 1, 2023
valentinandrei pushed a commit to valentinandrei/pytorch that referenced this pull request May 2, 2023
@kwanghoon-meta @valentinandrei
Automatic pulling ExtraFileMaps without explicit mapping.
9eaacca 
Differential Revision: D45170126nnPull Request resolved: pytorch#99747
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@davidberard98 davidberard98 davidberard98 approved these changes

Assignees
No one assigned
Labels
fb-exported release notes: mobile release notes category
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@kwanghoon-meta @facebook-github-bot @davidberard98