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

[Guide] Speeding up your Android/iOS Build #3027

Merged
merged 12 commits into from
Mar 29, 2022
Merged

Conversation

cortinico
Copy link
Contributor

I'm adding a short guide on how to use single arch ABI and ccache to speedup android builds.
Feedbacks are more than welcome 🙏

@netlify
Copy link

netlify bot commented Mar 24, 2022

Deploy Preview for react-native ready!

Name Link
🔨 Latest commit a7c55a5
🔍 Latest deploy log https://app.netlify.com/sites/react-native/deploys/6242f926d9c15e000823ff4d
😎 Deploy Preview https://deploy-preview-3027--react-native.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

Copy link
Contributor

@mikehardy mikehardy left a comment

Choose a reason for hiding this comment

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

My only comment is, I guess, that this is maybe a missed opportunity to have it be "Speeding up your build" in general, with a mostly-Android section, but then also mentioning you can symlink clang/clang++ in the same way, and prefix xcodebuild with env vars for CLANG/CLANG++/LD/LD++ that point to clang/clang++ respectively or use a Podfile stanza so clang/clang++ are discovered via relative names vs absolute paths, and then your Objective-C/C++ builds are also blazing. If you're open to this I could make a formal proposal for the verbiage? Seems so obvious if people are already setting up ccache

@mikehardy
Copy link
Contributor

Actually there is the obligatory other comment which should be first: this is fantastic

@cortinico
Copy link
Contributor Author

If you're open to this I could make a formal proposal for the verbiage? Seems so obvious if people are already setting up ccache

Please go ahead 👍 I'd be more than happy to have iOS & Android instructions here. I was aware of benefits that ccache could give to iOS builds as well, but I haven't tried ccache for iOS, therefore I haven't added section with untested comments.

If you have a series of instructions that are concise and works well on iOS, we can totally add them.

@mikehardy
Copy link
Contributor

Great, I'll do that. I have it thoroughly tested + working locally and CI. Requires some special ccache "sloppiness" config settings for iOS to get cache hits so the testing is critical yes


However, there are a couple of things to be aware:

1. On CI, we recommend to do a full clean build, to avoid poisoned cache problems. If you follow the approach mentioned in the previous paragraph, you should be able to parallelize the native build on 4 different ABIs and you will most likely not need `ccache` on CI.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggest speedup might still be worth it, there is a ccache-action that works great, and you can specify a versioned cache key to wipe out the cache if you suspect a poisoned cache. All of this in action:

https://github.com/invertase/react-native-firebase/blob/ef7895cd49eed3a4d103df624e52315250c68ab3/.github/workflows/tests_e2e_ios.yml#L70-L74

website/sidebars.json Outdated Show resolved Hide resolved
@mikehardy
Copy link
Contributor

mikehardy commented Mar 24, 2022

Alright - I offered up the text for the three main steps to make ccache work for Xcode. Take them and mash them around, re-purpose, re-organize however needed. It is really a magic trick for react-native, I think it will impact people's lives more even then the NDK optimizations you started here with - my iOS builds now take just a few seconds normally despite build testing everything all the time in a huge matrix on many projects. Life altering and I don't say that lightly

@mikehardy
Copy link
Contributor

The formatting is terrible though, triple-backticks pop the "suggestion" from github web UI out of scope. I trust you to integrate well though, that's a trivial issue since you'll have the file up on your fork

@danilobuerger
Copy link
Contributor

Maybe also mention to install a jdk that matches your cpus arch.

@sektr63a
Copy link

Maybe also mention to install a jdk that matches your cpus arch.

I think it should be mentioned on environment setup page.

docs/build-speed-android.md Outdated Show resolved Hide resolved
@leotm
Copy link
Contributor

leotm commented Mar 25, 2022

Maybe also mention to install a jdk that matches your cpus arch.

I think it should be mentioned on environment setup page.

i mentioned here but unsure how to proceed, happy to add a diff suggestion, jus unsure where

@mganandraj
Copy link

@cortinico Is there any plans (or does it make sense) to integrate ccache into the build scripts to avoid developers setting it up manually ?
And, how well does ccache work on Windows ?

@mganandraj
Copy link

Is there any plans (or does it make sense) to integrate ccache into the build scripts to avoid developers setting it up manually ?

Nope there are none at the moment. ccache is an external tool as the Android SDK is an external tool. We can't convert the RN tooling to install you all sorts of tools. We can work on making our setup easier to plug-in and extend, but we're not looking into encapsulating all sorts of tools which might also not be wanted by our users

That makes a lot of sense.. it would be great to make RN builds more extendable .. for instance, I would like to try buildxl with RN builds..

By the way, big thanks for adding documentation to improve local c++ development workflow. Is there any documentation for debugging c++ code in react native locally ? My setup is a bit complicated, hence curious to know how others do it ?

Copy link
Contributor

@mikehardy mikehardy left a comment

Choose a reason for hiding this comment

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

This is going to make a lot of people enjoy the react-native DX a lot more I think!

Comment on lines 149 to 150
ln -s ccache /usr/local/bin/clang
ln -s ccache /usr/local/bin/clang++
Copy link
Contributor

Choose a reason for hiding this comment

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

We should also note that this may break compilation in general. Even Homebrew warns of this when installing ccache:

% brew info ccache
ccache: stable 4.6 (bottled), HEAD
Object-file caching compiler wrapper
https://ccache.dev/
/usr/local/Cellar/ccache/4.5.1 (68 files, 1.3MB) *
  Poured from bottle on 2022-02-14 at 11:55:31
From: https://github.com/Homebrew/homebrew-core/blob/HEAD/Formula/ccache.rb
License: GPL-3.0-or-later
==> Dependencies
Build: asciidoctor ✘, cmake ✔, pkg-config ✔
Required: hiredis ✔, zstd ✔
==> Options
--HEAD
	Install HEAD version
==> Caveats
To install symlinks for compilers that will automatically use
ccache, prepend this directory to your PATH:
  /usr/local/opt/ccache/libexec

If this is an upgrade and you have previously added the symlinks to
your PATH, you may need to modify it to the path specified above so
it points to the current version.

NOTE: ccache can prevent some software from compiling.
ALSO NOTE: The brew command, by design, will never use ccache.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree, though this is it's a user's risk. The brew problem is out of scope in this document. If we're afraid of users creating symlinks, we can update the .mk/CMake files to offer users to prepend ccache to all the compilation commands.

That solution however, would be android-only.

Copy link
Contributor

@tido64 tido64 Mar 28, 2022

Choose a reason for hiding this comment

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

I wonder if it's enough to leave a sentence saying something like "this may break other projects, use at own risk". My worry is that we'll start seeing issues around this. But maybe that'll happen regardless… 😛

Comment on lines +180 to +183
config.build_settings["CC"] = "clang"
config.build_settings["LD"] = "clang"
config.build_settings["CXX"] = "clang++"
config.build_settings["LDPLUSPLUS"] = "clang++"
Copy link
Contributor

Choose a reason for hiding this comment

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

We should note here that Clang static analysis may break when these variables are set. We've seen issues and had to disable ccache here: microsoft/react-native-macos#1043 (comment)

docs/build-speed.md Outdated Show resolved Hide resolved
docs/build-speed.md Outdated Show resolved Hide resolved
Co-authored-by: Tommy Nguyen <4123478+tido64@users.noreply.github.com>
Copy link
Contributor

@kelset kelset left a comment

Choose a reason for hiding this comment

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

my comments are mostly nits and reorderings to have a more streamlined document - overall great stuff, thanks for putting it together!

docs/build-speed.md Outdated Show resolved Hide resolved
docs/build-speed.md Outdated Show resolved Hide resolved
docs/build-speed.md Show resolved Hide resolved
docs/build-speed.md Show resolved Hide resolved
docs/build-speed.md Outdated Show resolved Hide resolved
docs/build-speed.md Outdated Show resolved Hide resolved
website/sidebars.json Outdated Show resolved Hide resolved
Co-authored-by: Tommy Nguyen <4123478+tido64@users.noreply.github.com>
@cortinico cortinico changed the title [Guide] Speeding up your Android Build [Guide] Speeding up your Android/iOS Build Mar 28, 2022
@cortinico
Copy link
Contributor Author

I should have addressed all the comments by now 👍 Let me know if you're fine with the restructure @kelset

docs/build-speed.md Outdated Show resolved Hide resolved
Co-authored-by: Lorenzo Sciandra <notkelset@kelset.dev>
@kelset
Copy link
Contributor

kelset commented Mar 29, 2022

Looks solid 👍

Copy link
Collaborator

@Simek Simek left a comment

Choose a reason for hiding this comment

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

Looks like a great addition to the website! 👍

Also thanks to all of the people involved in the review of this PR, I have seen bunch of great suggestions! 🙏

One thing that we can address in the future could be adding npm commands and wrapping them with yarn ones in tabs, like on the other pages.

Co-authored-by: Bartosz Kaszubowski <gosimek@gmail.com>
@cortinico
Copy link
Contributor Author

Also thanks to all of the people involved in the review of this PR, I have seen bunch of great suggestions! 🙏

Echoing @Simek here, thank you all for making this a x-platform build improvement page 🙌

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.