-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
[$250] [HOLD for payment 2024-10-22] Speed up Android builds #49567
Comments
There are a few things I'd like to try:
|
In order to be methodic and quantitative with these changes, I'll be using |
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
Build scan for a clean |
Another clean build scan, this time only one arch: https://scans.gradle.com/s/4apezfw7xtotm Build command was: |
A build scan with only this diff: diff --git a/android/gradle.properties b/android/gradle.properties
index 87333d20f74..1f60a7d335d 100644
--- a/android/gradle.properties
+++ b/android/gradle.properties
@@ -55,3 +55,6 @@ MYAPP_UPLOAD_KEY_ALIAS=ReactNativeChat-Key-Alias
disableFrameProcessors=true
android.nonTransitiveRClass=false
+
+org.gradle.parallel=true |
Adding the
|
Going to try the configuration cache field with: ./gradlew assembleDebug --configuration-cache -PreactNativeArchitectures=arm64-v8a --scan Edit: 2nd build w/ configuration cache enabled failed 😞 Logs
|
This branch enabling ccache for Android builds also does not seem to materially improve the configuration time. Not sure if it's all done correctly though |
Another simple thing we could try is providing more RAM to the gradle build. Pretty sure it gets only 512MB by default. We could try bumping that to 2048 (or even higher) and see if it helps |
Hey @roryabraham I did some investigation around ccache and build process I have M1 Pro chip with 32gb memory
What i noticed, App/src/libs/Fullstory/index.native.ts Lines 17 to 24 in 2c07b39
So maybe there is a way we could exclude it from development builds?
I was having the same errors when trying to use |
Nice investigation @TMisiukiewicz! Since ccache does seem to help, can you take a look at this draft PR? My goal with that was to:
and the idea with the script is to setup symlinks as per the instructions in the RN docs, but then clean them up before the script exits using |
I got the next message: So I run a check, I disabled jetifier and check if other libs need it enabled, this is the result:
All of the libs has the Jetifier disabled. Meaning that we can disable Jetifier doing
That would save 5m, from Before: https://scans.gradle.com/s/4apezfw7xtotm |
@Julesssss can you comment on whether disabling jetifier is safe? Looks like it's already disabled for all those libs? So why was it taking so long? 😕 |
Looks like Jetifier is also removed from the community template: react-native-community/template#9 |
Wow, these are looking like great potential improvements. It sounds like jetifier is removing deprecated support libraries and using shared AndroidX support libraries. I'm not sure why it added to build times, perhaps low level Android libraries also benefit from this 🤷 Anyway, the trade-off of packaging duplicate support libs seems worth the reduced build times. My worry though is that HybridApp might depend on Jetifier. As the app was closer to native code, it's possible that we'd have to replace our AndroidX libraries usages with quite outdated libraries. I think the next step would be to get HybridApp to build. |
@roryabraham I tried running your branch, here are the results: I changed
The time is kinda extreme here, usually it takes up to 15 minutes. CMake tasks were really long here.
One thing that bothers me is the fact that I have to remember to run the script with |
Not sure if there's a good way to avoid that since I guess |
The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.47-4 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue: If no regressions arise, payment will be issued on 2024-10-17. 🎊 For reference, here are some details about the assignees on this issue:
|
The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.48-2 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue: If no regressions arise, payment will be issued on 2024-10-22. 🎊 For reference, here are some details about the assignees on this issue:
|
@Julesssss can you please reopen? C+ payment is remaining |
Job added to Upwork: https://www.upwork.com/jobs/~021856375406194339171 |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @dukenv0307 ( |
Triggered auto assignment to @garrettmknight ( |
Sorry @aimane-chnaif, we never had a member of the Bug Zero team assigned so this was missed! @garrettmknight could you please pay out @aimane-chnaif for C+ review of this PR. Thanks! |
📣 @aimane-chnaif 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app! Offer link |
@aimane-chnaif mind accepting that offer? |
Problem
Android builds typically take 10-30 minutes on my M2 Mac, which is a long time.
Solution
Research ways to speed them up
Upwork Automation - Do Not Edit
Issue Owner
Current Issue Owner: @dukenv0307The text was updated successfully, but these errors were encountered: