-
Notifications
You must be signed in to change notification settings - Fork 3.5k
[NO QA] perf: Increase initial Hermes heap size for faster startup and delay Young-Gen GC #76154
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
base: main
Are you sure you want to change the base?
[NO QA] perf: Increase initial Hermes heap size for faster startup and delay Young-Gen GC #76154
Conversation
|
|
530f898 to
97b551e
Compare
|
@chrispader What is your ETA for this one? |
|
@mountiny @Julesssss i'm finishing up this PR right now. |
|
The changes in this PR are finished, right now i'm only running some more android build to get reliable numbers. These build are quite time-consuming, therefore i wasn't able to find time for this earlier. I'm currently testing 4MB, 50MB, 150MB, 300MB initial heap size. I think in general 150mb should be a good value for low-end Android devices, but we can fine-grain this even more. If we want to get this pushed sooner than later, we can also merge this PR right away and we can adjust the initial heap size value after i have more reliable numbers. @mountiny @Julesssss what do you think? |
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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d708a4f9ef
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
patches/react-native/react-native+0.81.4+026+perf-increase-initial-heap-size.patch
Show resolved
Hide resolved
No worries, happy to add a small delay to help get it right first time. What devices models are you testing with out of interest? |
|
Fell free to find the best heap size |
I'm benchmarking mainly on a Samsung Galaxy A14 5G (SM-A146DP). This is our baseline low-end Android device at Margelo, which we use for most performance metrics. I've also tested on other low-end devices, which yield similar results. |
|
@Julesssss @mountiny i'll continue my benchmarks tomorrow, but these are the first reliable numbers that i can provide: 20 iterations each, outliers were ignored
As mentioned, i'll also try 50MB and 100MB. As you can see, there is not such a big diff between 150MB and 300MB initial heap size, therefore i would stick with 150MB or lower, depending on my final results. 300MB should still be easily handled by most low-end Android devices, but there's no need for putting that much pressure on memory right from the startup, if the gains are minimal |
|
Thanks for sharing, happy with your suggestion of 150 👍 |
|
finishing this up right now! |
@mountiny
Explanation of Change
By default, the Hermes JS engine uses an initial heap size of 4MB, which can be used for allocating memory for JS values. Once the heap is fully allocated, Hermes will run garbage collection, which takes time and can slow down other work, which is a problem especially during app startup.
With a small tweak in the creation of the Hermes instance (HermesInstance.cpp) we can increase the initial heap size to a higher value (e.g. 150MB) and delay Young-Gen GC on app start until Old-Gen GC is triggered for the first time. Based on my testing, this saves up around 150-300ms in startup time on low-end Android devices (potentially even more!). The saved startup time heavily depends on the device, the available RAM and memory pressure. The good thing about this change is that it does not introduce any downsides, except for higher RAM usage on startup.
Fixed Issues
$ #76859
PROPOSAL:
Tests
Profile and benchmark Android app start.
Offline tests
None needed.
QA Steps
Make sure the app behaves expectedly the same as before.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native