-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
Make Android app start 14%+ faster 🚀 #56930
Make Android app start 14%+ faster 🚀 #56930
Conversation
@MarioExpensify Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
Wow, this is nice. I'm gonna trigger an ad-hoc build to test it locally but otherwise, looks pretty good! |
🚧 @MarioExpensify has triggered a test build. You can view the workflow run here. |
🧪🧪 Use the links below to test this adhoc build on Android, iOS, Desktop, and Web. Happy testing! 🧪🧪 |
Reviewer Checklist
Screenshots/VideosAndroid: NativeWhatsApp.Video.2025-02-17.at.12.24.34.mp4Android: mWeb ChromeiOS: NativeiOS: mWeb SafariMacOS: Chrome / SafariMacOS: Desktop |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice, App is feeling snappier and seems everything is working as expected (Android native quick login video attached to checklist)
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
🚀 Deployed to staging by https://github.com/MarioExpensify in version: 9.1.0-0 🚀
|
Hi @perunt, are there any QA steps for this PR? |
🚀 Deployed to production by https://github.com/francoisl in version: 9.1.0-2 🚀
|
…Native apps start 12% faster) (#49449) Summary: Okay the title is a bit clickbaity, but this is actually true. (on Android) We (Janic, Szymon, Ruby and Me) discovered something interesting. React Native uses `mmap` for mapping the JS bundle to RAM, to avoid having to load the entire thing instantly at app startup. Ruby doubted that this was true - so we investigated. Apparently on Android, resources are **compressed**. And if the JS bundle is stored compressed, it has to be uncompressed before it can be loaded into RAM, hence not allowing it to be mmapp'ed! (see [`_CompressedAsset::getBuffer`](https://cs.android.com/android/platform/superproject/+/master:frameworks/base/libs/androidfw/Asset.cpp;l=903?q=Asset.cpp)) So with this PR, we now add `.bundle` files to `noCompress` in the react-native gradle plugin, which disables compression for the JS bundle. We discovered while improving the performance of one of our clients: **Discord**. In our tests, **this improved the TTI of the Discord app by 400ms!! (or 12%)** 🤯🚀 NOTE: Yes, the .apk will now be bigger. But; Google Play compresses it anyways, so the **download size** of your .apk will likely not increase by much. It will be bigger on disk though. ## Changelog: [ANDROID] [CHANGED] Add option to disable bundle compression to improve startup time <!-- Help reviewers and the release process by writing your own changelog entry. Pick one each for the category and type tags: [ANDROID|GENERAL|IOS|INTERNAL] [BREAKING|ADDED|CHANGED|DEPRECATED|REMOVED|FIXED|SECURITY] - Message For more details, see: https://reactnative.dev/contributing/changelogs-in-pull-requests Pull Request resolved: #49449 Test Plan: ### 1. Verify compression is disabled Build two apps, one with this patch and one without. When I did this using the RN community template, the one without this patch was 47,6 MB, and the one with this patch was 48 MB in size. So the .apk got bigger, which is what we expected ### 2. Verify app startup is faster Use tools like react-native-performance or custom markers to measure TTI. In our tests, we shaved off 400ms from the startup time, which was about 12% of Discord's total TTI. (on a low-end Android device) In Expensify, we improved the TTI by 14-20% with this change (source: Expensify/App#56930) Reviewed By: javache, cipolleschi Differential Revision: D69742221 Pulled By: cortinico fbshipit-source-id: bd59d77662bd30a3acdbb2e9f8d8f23db922c3f2
facebook/react-native#49449
Apparently on Android, resources are compressed. And if the JS bundle is stored compressed, it has to be uncompressed before it can be loaded into RAM, hence not allowing it to be mmapp'ed! (see _CompressedAsset::getBuffer)
So with this PR, we now add .bundle files to noCompress in the react-native gradle plugin, which disables compression for the JS bundle.
This PR improves TTI by 180-260ms (14-20%)
Explanation of Change
Fixed Issues
$
PROPOSAL:
Tests
Offline tests
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
PR Author Checklist
### Fixed Issues
section aboveTests
sectionOffline steps
sectionQA steps
sectiontoggleReport
and notonIconClick
)src/languages/*
files and using the translation methodSTYLE.md
) were followedAvatar
, I verified the components usingAvatar
are working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)Design
label and/or tagged@Expensify/design
so the design team can review the changes.ScrollView
component to make it scrollable when more elements are added to the page.main
branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTest
steps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
MacOS: Desktop