Add support for fission on android #14765
Closed
Conversation
This file contains 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
keith
added a commit
to envoyproxy/envoy-mobile
that referenced
this pull request
Feb 9, 2022
When building the jni dylib for android, we previously stripped all debug info to decrease the artifact size. With this change we now produce the same stripped binary as before, but before stripping it we create a dump of the debug info suitable for crash reporting. This is made overly difficult for a few reasons: 1. Bazel doesn't support fission for Android bazelbuild/bazel#14765 2. Extra outputs from rules are not propagated up the dependency tree, so just building `android_dist` at the top level, isn't enough to get the extra outputs built as well 3. Building the library manually alongside the android artifact on the command line results in 2 separate builds, one for android as a transitive dependency of `android_dist` and one for the host platform This change avoids #1 fission for now, but the same approach could be used once that change makes its way to a bazel release. This change ignores #2 but fixes #3 so it requires you to explicitly build the library as part of the command line invocation if you want debug info, like: ``` ./bazelw build --fat_apk_cpu=... android_dist //library/common/jni:libenvoy_jni.so.debug_info ``` Theoretically we could probably shove this artifact into the aar to make sure it was correctly produced, but that risks us accidentally shipping that in the aar. Signed-off-by: Keith Smiley <keithbsmiley@gmail.com>
keith
added a commit
to envoyproxy/envoy-mobile
that referenced
this pull request
Feb 9, 2022
When building the jni dylib for android, we previously stripped all debug info to decrease the artifact size. With this change we now produce the same stripped binary as before, but before stripping it we create a dump of the debug info suitable for crash reporting. This is made overly difficult for a few reasons: 1. Bazel doesn't support fission for Android bazelbuild/bazel#14765 2. Extra outputs from rules are not propagated up the dependency tree, so just building `android_dist` at the top level, isn't enough to get the extra outputs built as well 3. Building the library manually alongside the android artifact on the command line results in 2 separate builds, one for android as a transitive dependency of `android_dist` and one for the host platform This change avoids #1 fission for now, but the same approach could be used once that change makes its way to a bazel release. This change fixes #2 by using a separate output group that can be depended on by the genrule that writes to dist while avoiding #3 because the custom rule producing these uses the android transition. Signed-off-by: Keith Smiley <keithbsmiley@gmail.com>
4 tasks
keith
force-pushed
the
ks/add-support-for-fission-on-android
branch
from
114a18c
to
258c60e
Compare
This change adds the per_object_debug_info feature to the android cc toolchain to support fission. This also removes this same feature from the macOS crosstool because clang targeting darwin does nothing when passed this flag, which leads to failures if you enable this for android, but are on a macOS host because host tools fail to produce the dwo files.
keith
force-pushed
the
ks/add-support-for-fission-on-android
branch
from
258c60e
to
e5e3fdd
Compare
|
cc: @ahumesky |
|
This one is also supported by the new NDK rules https://github.com/bazelbuild/rules_android_ndk/blob/40e17b3af75406ab10219313f16a404937f09874/ndk_cc_toolchain_config.bzl#L886-L894 so I will close this once it's more clear those are the future |
jpsim
pushed a commit
to envoyproxy/envoy
that referenced
this pull request
Nov 29, 2022
When building the jni dylib for android, we previously stripped all debug info to decrease the artifact size. With this change we now produce the same stripped binary as before, but before stripping it we create a dump of the debug info suitable for crash reporting. This is made overly difficult for a few reasons: 1. Bazel doesn't support fission for Android bazelbuild/bazel#14765 2. Extra outputs from rules are not propagated up the dependency tree, so just building `android_dist` at the top level, isn't enough to get the extra outputs built as well 3. Building the library manually alongside the android artifact on the command line results in 2 separate builds, one for android as a transitive dependency of `android_dist` and one for the host platform This change avoids #1 fission for now, but the same approach could be used once that change makes its way to a bazel release. This change fixes #2 by using a separate output group that can be depended on by the genrule that writes to dist while avoiding #3 because the custom rule producing these uses the android transition. Signed-off-by: Keith Smiley <keithbsmiley@gmail.com> Signed-off-by: JP Simard <jp@jpsim.com>
|
dropping for the new ndk rules |
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.
This change adds the per_object_debug_info feature to the android cc
toolchain to support fission.
This also removes this same feature from the macOS crosstool because
clang targeting darwin does nothing when passed this flag, which leads
to failures if you enable this for android, but are on a macOS host
because host tools fail to produce the dwo files.