-
Notifications
You must be signed in to change notification settings - Fork 632
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
Optimise the size of iOS artifacts #368
Comments
We definitely do not need bitcode for the simulator. IIRC, Xcode doesn’t even actually build bitcode until an archive release is built. Are you saying Xcode won’t allow a simulator release build without the slice present?
Symbols should be made available, at the very least for release builds in order to symbolicate crash reports. These could potentially be made available separately for when needed, but we have to take into account the possibility that RN users will not know/remember to do so and in turn struggle with unhelpful crash reports.
I am in favour of not using npm. At first it seemed like it might be good to stay consistent for now, but it’s becoming clear that it’s only adding friction, with eg mow needing to decide when to install it.
Strong yes on this from me. FWIW I believe we also shouldn’t install the Android runtime by default and perhaps also not via npm.
This would indeed solve the artefact size issue, but the build isn’t trivial to convert (although everything is doable) and would increase the build time of RN projects. |
I believe when bitcode is turned on and simulator slices don't contain bitcode, the build will fail, unless you turn that off for that particular build. I am fine with that, but I am trying to think for the users as well. My current preferred solution is to release Hermes to the CocoaPods repository with artifacts and have I think this is totally fine, given it's what |
2gb! This sounds painful for everyone without the biggest network connections. How does the CocoaPods repository release process work? |
@mhorowitz CocoaPods isn’t a package manager, so it doesn’t host packages; this means we just need to serve the artefacts from somewhere, e.g. a GitHub release, and then point the CocoaPods podspec to that location. CocoaPods also supports tarballs compressed with xz, so I’m interested in seeing how large the archive will be when compressed with that algorithm. |
I will experiment with that and post results at the beginning of next week. |
I am back after holidays, resuming my work on this one. Will keep you posted. The plan here is to not build bitcode for non-release targets (simulator) and to ship to CocoaPods. That should offload the installation of Hermes until it is turned on via a Podfile. |
To publish a podspec to the centralised spec repo, you need to follow the steps outlined here: https://guides.cocoapods.org/making/getting-setup-with-trunk.html An important thing to note here is that, as we’re planning to distribute prebuilt binaries, the spec.source = ENV['hermes-artefact-url'] ? ENV['hermes-artefact-url'] : { git: "https://github.com/facebook/hermes.git", tag: "v#{spec.version}" }
unless ENV['hermes-artefact-url']
spec.prepare_command = '…'
end And then do: env hermes-artefact-url='…' pod trunk push hermes.podspec |
Thanks, @alloy, a couple more questions.
|
|
Summary: Bitcode is not needed for `iphonesimulator` - this PR makes bitcode to be included with iOS only. Reduces the package size by ~1GB. Related to #368 Pull Request resolved: #376 Reviewed By: willholen Differential Revision: D24031684 Pulled By: Huxpro fbshipit-source-id: 342b0139d208cdea6c2af2983c894b758a7d7789
Summary: Bitcode is not needed for `iphonesimulator` - this PR makes bitcode to be included with iOS only. Reduces the package size by ~1GB. Related to #368 Pull Request resolved: #376 Reviewed By: willholen Differential Revision: D24031684 Pulled By: Huxpro fbshipit-source-id: 342b0139d208cdea6c2af2983c894b758a7d7789
This PR did two things. 1. rename to `hermes-engine` since <https://cocoapods.org/pods/Hermes> already exists 2. conditionalize `spec.source` so it can distribute prebuilt binaries as @alloy suggested at #368 (comment)
This PR did two things. 1. rename to `hermes-engine` since <https://cocoapods.org/pods/Hermes> already exists 2. conditionalize `spec.source` so it can distribute prebuilt binaries as @alloy suggested at #368 (comment)
Summary: This PR did two things. 1. rename everything to `hermes-engine` since <https://cocoapods.org/pods/Hermes> already exists 2. conditionalize `spec.source` so it can distribute prebuilt binaries as alloy suggested at #368 (comment) Pull Request resolved: #385 Test Plan: CircleCI doing fine Reviewed By: tmikov Differential Revision: D24206180 Pulled By: Huxpro fbshipit-source-id: ec46350e00099c61a7de8cc9eb99991af72abdb4
Summary: I cherry-picked this PR which: 1. rename everything to `hermes-engine` since <https://cocoapods.org/pods/Hermes> already exists 2. conditionalize `spec.source` so it can distribute prebuilt binaries as alloy suggested at #368 (comment) Noted that the version in the `podspec` had been updated to 0.7.1 correctly. Pull Request resolved: #385 Test Plan: CircleCI doing fine Reviewed By: tmikov Differential Revision: D24206180 Pulled By: Huxpro fbshipit-source-id: ec46350e00099c61a7de8cc9eb99991af72abdb4
@alloy it seems like
Btw, I realized |
Ah yes indeed, my bad. See the doc on it here https://guides.cocoapods.org/syntax/podspec.html#source. The hash can also include a checksum for the artefact. |
As for the name, I think |
@alloy the current document seem to use a different syntax (
then redo the push. But this is what I got:
Despite the warnings, there are another 4 errors. |
As discussed on Discord, you may wanna use |
Yay!! Looks good to me? Need to verify if it actually works! |
I think this is good for now. Specifically we no longer install this dependency by default with new apps, people have to opt-in and then install it. |
Problem
Right now, the artifact itself is rather large after all architectures, dSYMs, and bitcode were included - 2GB. We need to find a way to reduce this to a reasonably small size, so that it doesn't affect React Native users.
The goal is to reduce the size so that it comes w/o penalty for new React Native applications.
Solution
hermes-engine-darwin
npm packagebitcode
, which comes "on" by default. Potential change in the default template possible as long as it makes sense to do so. We can also disablebitcode
foriphonesimulator
architectures as I think it makes no sense to ship that for non-release architectures (won't be able to build for simulator in the release tho)dSYM
weights 200mbhermes-engine-darwin
npm package on demand (unlike for Android) - size constraint doesn't affect the initial setup time. Potential degraded DX as it requires many actions to be done to enable Hermesnpm
forhermes-engine-darwin
, but useCocoaPods
dependency management mechanism. We can publish full Hermes to CocoaPods repository and it will be automatically installed as soon as React Native users change the flag in a Podfile toenable_hermes
. This is done by removingpath
to aHermes
podspec that points tonode_modules
.Opening this issue to discuss potential ways going forward and options that we have.
The text was updated successfully, but these errors were encountered: