-
Notifications
You must be signed in to change notification settings - Fork 24.7k
Setup a Macrobenchmark for RNTester #49482
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
Conversation
…Native apps start 12% faster) (facebook#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. In our tests, **this improved TTI 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: facebook#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%. (on a low-end Android device) Reviewed By: javache, cipolleschi Differential Revision: D69742221 Pulled By: cortinico
Summary: Tests have been executed with Android Studio: **Startup** ``` # BEFORE timeToInitialDisplayMs min 216.9, median 222.9, max 245.3 Traces: Iteration 0 1 2 3 4 5 6 7 8 9 # AFTER timeToInitialDisplayMs min 213.8, median 220.4, max 237.9 Traces: Iteration 0 1 2 3 4 5 6 7 8 9 ``` **APK size** ``` -rw-r--r-- 1 ncor staff 20087367 Feb 17 17:37 before.apk -rw-r--r-- 1 ncor staff 20087399 Feb 17 18:43 after.apk ``` Changelog: [Internal] [Changed] - Differential Revision: D69753053
This pull request was exported from Phabricator. Differential Revision: D69753053 |
iterations = 10, | ||
startupMode = StartupMode.COLD) { | ||
pressHome() | ||
startActivityAndWait() |
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.
Are you sure this waits for the react root view to be displayed, and not just a blank activity before react rendered?
Also are these numbers from running on emulator? They look a little bit low, hence why I am suspicious.
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.
The numbers are from running on a Pixel 8 Pro. If you run it locally you can see the RNTester home screen appearing on device 10x times
Summary:
Tests have been executed with Android Studio:
Startup
APK size
Changelog:
[Internal] [Changed] -
Differential Revision: D69753053