[pytorch][mobile] add NO_EXPORT macro to unset __visibility__ attribute #25816
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Summary: On Android we will release a small set of native APIs designed for mobile use cases. All of needed libtorch c++ APIs are called from inside this JNI bridge: android/pytorch_android/src/main/cpp/pytorch_jni.cpp With NO_EXPORT set for android static library build, it will hide all original TORCH, CAFFE2, TH/ATen APIs, which will allow linker to strip out unused ones from mobile library when producing DSO. If people choose to directly build libtorch DSO then it will still keep all c++ APIs as the mobile API layer is not part of libtorch build (yet). Test Plan: - build libtorch statically and link into demo app; - confirm that linker can strip out unused APIs;
pytorchbot
added
module: build
This was referenced Sep 7, 2019
Closed
ezyang
approved these changes
Sep 9, 2019
Contributor
ezyang
left a comment
ezyang
left a comment
There was a problem hiding this comment.
I'm not sure this is exhaustive, but it's fine as a start.
Will we add some sort of test that will audit the number of visible symbols, to make sure they are what we expect?
Contributor
Author
Looks like torch/* all use TORCH_API, caffe2/* all use CAFFE2_API - both are now defined as C10_API / C10_EXPORT. There are a few cases directly exported using C10_EXPORT. So redefining C10_EXPORT should cover most of them.
I was thinking of tracking the android gradle build size directly. Will see how hard it is to count visible symbols. |
…y__ attribute" Summary: On Android we will release a small set of native APIs designed for mobile use cases. All of needed libtorch c++ APIs are called from inside this JNI bridge: android/pytorch_android/src/main/cpp/pytorch_jni.cpp With NO_EXPORT set for android static library build, it will hide all original TORCH, CAFFE2, TH/ATen APIs, which will allow linker to strip out unused ones from mobile library when producing DSO. If people choose to directly build libtorch DSO then it will still keep all c++ APIs as the mobile API layer is not part of libtorch build (yet). Test Plan: - build libtorch statically and link into demo app; - confirm that linker can strip out unused APIs; Pull Request resolved: #25816 Differential Revision: [D17247237](https://our.internmc.facebook.com/intern/diff/D17247237)
…y__ attribute" Summary: On Android we will release a small set of native APIs designed for mobile use cases. All of needed libtorch c++ APIs are called from inside this JNI bridge: android/pytorch_android/src/main/cpp/pytorch_jni.cpp With NO_EXPORT set for android static library build, it will hide all original TORCH, CAFFE2, TH/ATen APIs, which will allow linker to strip out unused ones from mobile library when producing DSO. If people choose to directly build libtorch DSO then it will still keep all c++ APIs as the mobile API layer is not part of libtorch build (yet). Test Plan: - build libtorch statically and link into demo app; - confirm that linker can strip out unused APIs; Pull Request resolved: #25816 Differential Revision: [D17247237](https://our.internmc.facebook.com/intern/diff/D17247237)
This was referenced Sep 10, 2019
…y__ attribute" Summary: On Android we will release a small set of native APIs designed for mobile use cases. All of needed libtorch c++ APIs are called from inside this JNI bridge: android/pytorch_android/src/main/cpp/pytorch_jni.cpp With NO_EXPORT set for android static library build, it will hide all original TORCH, CAFFE2, TH/ATen APIs, which will allow linker to strip out unused ones from mobile library when producing DSO. If people choose to directly build libtorch DSO then it will still keep all c++ APIs as the mobile API layer is not part of libtorch build (yet). Test Plan: - build libtorch statically and link into demo app; - confirm that linker can strip out unused APIs; Pull Request resolved: #25816 Differential Revision: [D17247237](https://our.internmc.facebook.com/intern/diff/D17247237)
Contributor
That sounds good. @mingbowan has recently put our binary sizes in scuba, and might know what we should do here. |
Contributor
zdevito
pushed a commit
to zdevito/ATen
that referenced
this pull request
Sep 10, 2019
Summary: Pull Request resolved: pytorch/pytorch#25816 On Android we will release a small set of native APIs designed for mobile use cases. All of needed libtorch c++ APIs are called from inside this JNI bridge: android/pytorch_android/src/main/cpp/pytorch_jni.cpp With NO_EXPORT set for android static library build, it will hide all original TORCH, CAFFE2, TH/ATen APIs, which will allow linker to strip out unused ones from mobile library when producing DSO. If people choose to directly build libtorch DSO then it will still keep all c++ APIs as the mobile API layer is not part of libtorch build (yet). Test Plan: - build libtorch statically and link into demo app; - confirm that linker can strip out unused APIs; Differential Revision: D17247237 Pulled By: ljk53 fbshipit-source-id: de668216b5f2130da0d6988937f98770de571c7a
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Stack from ghstack:
Summary:
On Android we will release a small set of native APIs designed for mobile use
cases. All of needed libtorch c++ APIs are called from inside this JNI bridge:
android/pytorch_android/src/main/cpp/pytorch_jni.cpp
With NO_EXPORT set for android static library build, it will hide all
original TORCH, CAFFE2, TH/ATen APIs, which will allow linker to strip
out unused ones from mobile library when producing DSO.
If people choose to directly build libtorch DSO then it will still keep
all c++ APIs as the mobile API layer is not part of libtorch build (yet).
Test Plan:
Pull Request resolved: #25816
Differential Revision: D17247237