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.

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

Enable Bitcode on libmobilecoin, reduces downloadable size by ~25% #1124

Conversation

the-real-adammork
Copy link
Contributor

@the-real-adammork the-real-adammork commented Oct 22, 2021

Soundtrack of this PR: SW2 & Friends - For the People

Motivation

Add support for Apple's Bitcode and reduce the size of our SDK & its dependencies. With this enabled the compressed "downloadable" size shrinks by ~25%.

ENABLE_BITCODE = YES

App Thinning Size Report for All Variants of Example
App + On Demand Resources size: 7.2 MB compressed, 19.7 MB uncompressed
App size: 7.2 MB compressed, 19.7 MB uncompressed

ENABLE_BITCODE = NO

App Thinning Size Report for All Variants of Example
App + On Demand Resources size: 9.6 MB compressed, 35.6 MB uncompressed
App size: 9.6 MB compressed, 35.6 MB uncompressed

In this PR

rust-bitcode

Add a submodule rust-bitcode to compile rustup toolchains that include LLVM compatible with Apple's Xcode build system.

No more symbol stripping

Remove "symbol" stripping step from the Makefile because it's not compatible with enabling Bitcode. While this step had large impact on the size of the archive .a (10mb vs 55mb) it has a small impact on the "downloadable" size (~0.2mb). This is because un-linked symbols are not included in final binary regardless of whether or not they get stripped.

Remove build-target x86_64-apple-darwin

To my knowledge MobileCoin and Signal are not using this target, and it does not support Bitcode.

Future Work

Technically there may be a way to strip and keep the Bitcode but the size savings would be small ~0.2mb compressed. This can be left as a future task if needed.

Linked PRs

libmobilecoin-ios-artifacts - mobilecoinofficial/libmobilecoin#4
MobileCoin-Swift - mobilecoinofficial/MobileCoin-Swift#80

Copy link
Contributor

@cbeck88 cbeck88 left a comment

Choose a reason for hiding this comment

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

that is pretty cool!

so our CI is now testing this thing with the toolchain being installed from the makefile?

@the-real-adammork
Copy link
Contributor Author

that is pretty cool!

so our CI is now testing this thing with the toolchain being installed from the makefile?

Not sure about the CI, Ill look into that.

@the-real-adammork
Copy link
Contributor Author

that is pretty cool!

so our CI is now testing this thing with the toolchain being installed from the makefile?

Update on CI

The CI workflow uses this cargo command: cargo build --frozen --target "$HOST_TARGET_TRIPLE" -p libmobilecoin which won't hit the Makefile or my new changes.

I have a WIP branch, using the Makefile, but it takes ~2hrs to complete (rust + llvm = slow). It can be refined to be faster but would take me some time. Im gonna make it as a future work ticket.

Legacy Build process

I added a recipe make legacy that uses the previous rustup toolchain setup & build flow.

…in for the all recipe and it works now with a fix to rust-bitcode (doesnt add the -fembed-bitcode flag if target is x86_64-apple-darwin)
@cbeck88
Copy link
Contributor

cbeck88 commented Oct 26, 2021

that sounds good -- i'm happy with either approach to CI, whatever you are comfortable with. i would hate to break your build by accident but if it's not practical to test it in CI that's too bad. ideally we could put the custom toolchain in a docker container for CI to just fetch and run or something, but that might not work on an apple target?

@the-real-adammork
Copy link
Contributor Author

the-real-adammork commented Oct 26, 2021

that sounds good -- i'm happy with either approach to CI, whatever you are comfortable with. i would hate to break your build by accident but if it's not practical to test it in CI that's too bad. ideally we could put the custom toolchain in a docker container for CI to just fetch and run or something, but that might not work on an apple target?

ya this is the type of refinement I was looking at. We're already doing cache-ing in a few places I could implement that for the rust-toolchains. When we update the rust nightly, we update the caches

@the-real-adammork
Copy link
Contributor Author

@jcape changed to using a shallow clone of rust-bitcode, and added the stripping code back into the legacy recipe.

@the-real-adammork the-real-adammork merged commit a9a25b0 into mobilecoinfoundation:master Oct 29, 2021
the-real-adammork added a commit to the-real-adammork/mobilecoin that referenced this pull request Oct 29, 2021
mobilecoinfoundation#1124)

### Motivation

Add support for Apple's `Bitcode` and reduce the size of our SDK & its dependencies. With this enabled the compressed "downloadable" size **shrinks by ~25%.**  

#### `ENABLE_BITCODE = YES`

```
App Thinning Size Report for All Variants of Example
App + On Demand Resources size: 7.2 MB compressed, 19.7 MB uncompressed
App size: 7.2 MB compressed, 19.7 MB uncompressed
```

#### `ENABLE_BITCODE = NO`

```
App Thinning Size Report for All Variants of Example
App + On Demand Resources size: 9.6 MB compressed, 35.6 MB uncompressed
App size: 9.6 MB compressed, 35.6 MB uncompressed
```

### In this PR

#### `rust-bitcode`

Shallow clone [`rust-bitcode`](https://github.com/mobilecoinofficial/rust-bitcode) to compile `rustup` toolchains that include LLVM compatible with Apple's Xcode build system.

#### No more symbol stripping

Remove "symbol" stripping step from the `Makefile` because it's not compatible with enabling `Bitcode`. While this step had large impact on the size of the archive `.a` (10mb vs 55mb) it has a small impact on the "downloadable" size (~0.2mb). This is because un-linked symbols are not included in final binary regardless of whether or not they get stripped.

#### Remove build-target `x86_64-apple-darwin`

To my knowledge MobileCoin and Signal are not using this target, and it does not support `Bitcode`.
the-real-adammork added a commit that referenced this pull request Nov 2, 2021
#1124) (#1140)

### Motivation

Add support for Apple's `Bitcode` and reduce the size of our SDK & its dependencies. With this enabled the compressed "downloadable" size **shrinks by ~25%.**  

#### `ENABLE_BITCODE = YES`

```
App Thinning Size Report for All Variants of Example
App + On Demand Resources size: 7.2 MB compressed, 19.7 MB uncompressed
App size: 7.2 MB compressed, 19.7 MB uncompressed
```

#### `ENABLE_BITCODE = NO`

```
App Thinning Size Report for All Variants of Example
App + On Demand Resources size: 9.6 MB compressed, 35.6 MB uncompressed
App size: 9.6 MB compressed, 35.6 MB uncompressed
```

### In this PR

#### `rust-bitcode`

Shallow clone [`rust-bitcode`](https://github.com/mobilecoinofficial/rust-bitcode) to compile `rustup` toolchains that include LLVM compatible with Apple's Xcode build system.

#### No more symbol stripping

Remove "symbol" stripping step from the `Makefile` because it's not compatible with enabling `Bitcode`. While this step had large impact on the size of the archive `.a` (10mb vs 55mb) it has a small impact on the "downloadable" size (~0.2mb). This is because un-linked symbols are not included in final binary regardless of whether or not they get stripped.

#### Remove build-target `x86_64-apple-darwin`

To my knowledge MobileCoin and Signal are not using this target, and it does not support `Bitcode`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants