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

[$1000] DEV Env - App performance on Android is extremely slow - reported by @parasharrajat #10345

Closed
mvtglobally opened this issue Aug 11, 2022 · 51 comments
Assignees

Comments

@mvtglobally
Copy link

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Action Performed:

  1. Upgrade Android app build in Dev to latest version
  2. Navigate to chats
  3. Create group chat
  4. try other typical user actions

Expected Result:

App should load and respond quickly

Actual Result:

Button clicks takes about 2-3 second to react.
List rendering is very slow.

Workaround:

unknown

Platform:

Where is this issue occurring?

  • Android

Version Number:
Reproducible in staging?: N
Reproducible in production?: N
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation

screen-2022-08-11_01.00.42.mp4

Expensify/Expensify Issue URL:
Issue reported by: @parasharrajat
Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1660159264911759

View all open jobs on GitHub

@melvin-bot
Copy link

melvin-bot bot commented Aug 11, 2022

Triggered auto assignment to @PauloGasparSv (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

@PauloGasparSv
Copy link
Contributor

Double checking if the ReactNative version upgrade done lately has affected the performance as it was mentioned in the slack thread multiple times

@PauloGasparSv
Copy link
Contributor

PauloGasparSv commented Aug 11, 2022

Problem was replicated successfully locally on main and goes to back to normal if tested on a commit before the RN version upgrade. Solving the issue requires a bigger investigation, since this can be worked by internal and external people I'm adding the external label. Also keeping as daily since this may be a big problem that increases qa and test time a lot.

cc @roryabraham and @AndrewGable

Useful links

https://reactnative.dev/blog/2022/06/21/version-069
https://reactnative.dev/docs/next/react-18-and-react-native
https://github.com/facebook/react-native/blob/main/CHANGELOG.md

@PauloGasparSv PauloGasparSv added the External Added to denote the issue can be worked on by a contributor label Aug 11, 2022
@melvin-bot
Copy link

melvin-bot bot commented Aug 11, 2022

Triggered auto assignment to @adelekennedy (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

@PauloGasparSv PauloGasparSv removed their assignment Aug 11, 2022
@adelekennedy
Copy link

internal
external

@melvin-bot
Copy link

melvin-bot bot commented Aug 11, 2022

Triggered auto assignment to Contributor-plus team member for initial proposal review - @Santhosh-Sellavel (Exported)

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Aug 11, 2022
@melvin-bot
Copy link

melvin-bot bot commented Aug 11, 2022

Triggered auto assignment to @marcochavezf (Exported), see https://stackoverflow.com/c/expensify/questions/7972 for more details.

@melvin-bot melvin-bot bot changed the title DEV Env - App performance on Android is extremely slow - reported by @parasharrajat [$250] DEV Env - App performance on Android is extremely slow - reported by @parasharrajat Aug 11, 2022
@marcaaron
Copy link
Contributor

Just curious if there is a more specific deliverable for this issue. In the past, when we have issues like "make x faster" the discussion tends not to land anywhere or a variety of solutions are provided. How much faster is fast enough? Are people using Android Emulator or actual devices when they experience the slowness? Am I reading this correctly and the slowness doesn't affect the release build only the dev build?

@dhairyasenjaliya
Copy link
Contributor

dhairyasenjaliya commented Aug 16, 2022

Until any permanent fix, the current workaround is to disable Hermes from the android build.gradle

  • I suspect the latest react version includes Hermes by default at an early stage than it was intended in the 0.70 version now making 0.69 an unnecessary mess into android

0.70 - https://reactnative.dev/blog/2022/07/08/hermes-as-the-default
0.69 - https://reactnative.dev/architecture/bundled-hermes#android-users-on-new-architecture-building-on-windows

project.ext.react = [
enableHermes: true, // clean and rebuild if changing
]

enableHermes: false,

Result after disabling Hermes on android

android_performance.mp4

Note: clear android build and rebuild to change effect

@parasharrajat
Copy link
Member

parasharrajat commented Aug 16, 2022

We are using Hermes for two years.

Note: if you are adding a proposal, please use Proposal title and try to follow the Contribution guidelines.

@dhairyasenjaliya
Copy link
Contributor

hm @parasharrajat this is not any proposal this is just temp fix for those who need to test android until the permanent fix

@dhairyasenjaliya
Copy link
Contributor

yes, Hermes was working fine, but in the latest 0.69 they include by default as an engine previously it was external so I believe something goes wrong with android

@parasharrajat
Copy link
Member

Am I reading this correctly and the slowness doesn't affect the release build only the dev build?

Correct. Currently is not usable on the Android dev build. It is very hard to test anything. It is not about performance but bug which is making it impossible to use the app on Android.

@ntdiary
Copy link
Contributor

ntdiary commented Aug 19, 2022

Proposal

Solution

OMG, I noticed this issue eight days ago, it is so hard to investigate. 😅
Some progress now:

  1. please try to build react-native with NDK 21
  2. use this version of hermes-engine in the project
  3. and then app will work fine.

0

0.mp4

Extra

This is the content of libhermes.so in the react-native and @expensify/react-native

1

it can be simply verified.
download react-native-0.69.4
extract the hermes-engine-0.69.4-debug.aar file
replace it in the node_modules/react-native/android/com/facebook/react/hermes-engine/0.69.4/ directory
exec npm run android to launch app and it will work fine.

@melvin-bot melvin-bot bot removed the Weekly KSv2 label Aug 19, 2022
@roryabraham
Copy link
Contributor

This seems to have been resolved by #10860

@roryabraham
Copy link
Contributor

Reopening to pay @ntdiary a bounty for figuring out the NDK issue

@roryabraham roryabraham removed Help Wanted Apply this label when an issue is open to proposals by contributors External Added to denote the issue can be worked on by a contributor labels Sep 23, 2022
@roryabraham
Copy link
Contributor

@ntdiary Please apply to the upwork job so that @adelekennedy can pay you the bounty. No C+ bounty on this one

@melvin-bot melvin-bot bot added the Overdue label Sep 26, 2022
@ntdiary
Copy link
Contributor

ntdiary commented Sep 26, 2022

Hi @roryabraham, thanks for reminding 🙂
Just curious if it is possible to re-evaluate this bounty or if this issue is difficult in your opinion.
I encountered this issue after testing the image file size issue, and then it took me a week to finally figure out the NDK version was the cause of this issue, as I thought it seemed to be more urgent at the time. 😂

Actually, it's a pity I didn't do more work with you because of other stuffs.
And I really like the atmosphere here. So even if the bounty won't increase, I'm also happy to pick up more issue here. 😄

@roryabraham
Copy link
Contributor

Yes, I think it's fair to bump this up to $1000. This issue was urgent and I'm not sure how you even figured out the NDK issue 😂 Were you building your own fork of React Native locally?

Anyways, it's possible I might have accidentally fixed this issue while working on something unrelated, but:

  1. I might not have realized that the NDK version was important
  2. We might not have quickly realized what PR made the difference and fixed this issue
  3. We'll also never know because you had already figured out the NDK issue before I republished our fork

@melvin-bot melvin-bot bot removed the Overdue label Sep 26, 2022
@ntdiary
Copy link
Contributor

ntdiary commented Sep 27, 2022

Thanks for your opinion. 😄
I don't have my own fork of react-native. I have also tried to find the cause of this issue from many aspects, but all failed. As a last attempt, I had to carefully compare some differences of hermes engine. At that time, I noticed the huge difference in file size. Then I checked the contents of binary file and found the NDK issue. Finally I submitted the proposal above after rebuilding the @expensify/react-native and verifying it locally. 😂

Could I apply to the upwork job for $1000 now? Or need to wait for some other confirmation?
(This job is no longer available. 😅)

@adelekennedy adelekennedy changed the title [$250] DEV Env - App performance on Android is extremely slow - reported by @parasharrajat [$1000] DEV Env - App performance on Android is extremely slow - reported by @parasharrajat Sep 27, 2022
@adelekennedy
Copy link

Upped the pay in Upwork and here - @ntdiary do you mind applying here?

@ntdiary
Copy link
Contributor

ntdiary commented Sep 27, 2022

@adelekennedy, Thanks, I have applied. 😄

@parasharrajat
Copy link
Member

Can I apply for reporting as well?

@adelekennedy
Copy link

@parasharrajat please do!

@melvin-bot
Copy link

melvin-bot bot commented Oct 3, 2022

@roryabraham, @adelekennedy Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

@melvin-bot melvin-bot bot added the Overdue label Oct 3, 2022
@adelekennedy
Copy link

@parasharrajat did you apply for the bonus?

@melvin-bot melvin-bot bot removed the Overdue label Oct 4, 2022
@parasharrajat
Copy link
Member

@adelekennedy Done.

@roryabraham
Copy link
Contributor

@adelekennedy Let's pay this out and close it

@melvin-bot
Copy link

melvin-bot bot commented Oct 10, 2022

@roryabraham, @adelekennedy Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

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

No branches or pull requests