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

Android release converting js bundle to byte code is very slow #852

Closed
mmmoussa opened this issue Nov 16, 2022 · 20 comments
Closed

Android release converting js bundle to byte code is very slow #852

mmmoussa opened this issue Nov 16, 2022 · 20 comments
Labels
bug Something isn't working

Comments

@mmmoussa
Copy link

Bug Description

We recently added a bunch of json files containing (thousands of) nested arrays of numerical data to our RN app. Each of these files is around 1.2MB minified. We conditionally access the data from within our app through inline requires like this:
const data = require('./path/to/the/file.json');

Running our app in development mode is still working fine as is building a release bundle for iOS. However, with each json file that we added, our Android release build time increased by a few minutes, with our total build time exploding from ~5 minutes to ~50 minutes.

Hermes version: 0.70.4
React Native version (if any): 0.70.4
OS version (if any): MacOS Monterey 12.5.1
Platform (most likely one of arm64-v8a, armeabi-v7a, x86, x86_64): arm

Steps To Reproduce

  1. Add multiple very large json files to your project
  2. Try to build your app for release on Android
  3. Notice the enormous difference in total build time

The Expected Behavior

Android build time changes minimally like is the case with metro bundling and iOS release builds.

@mmmoussa mmmoussa added the bug Something isn't working label Nov 16, 2022
@tmikov
Copy link
Contributor

tmikov commented Nov 16, 2022

Possibly duplicate of #135

@tmikov
Copy link
Contributor

tmikov commented Nov 16, 2022

Can you please provide a file reproducing the behavior?

@tmikov
Copy link
Contributor

tmikov commented Nov 16, 2022

My first guess is that this is due to slow deduplication. We have something for that, but we would like to confirm it fixes the issue.

@mmmoussa
Copy link
Author

Sorry, I should've provided that upfront. Here's a sample file: https://drive.google.com/file/d/13Wa02PbEmipfmVPHxX2VklLnc-lP84-J/view?usp=sharing

@fbmal7
Copy link
Contributor

fbmal7 commented Nov 16, 2022

with each json file that we added

How many of these files are you using in your app?

@mmmoussa
Copy link
Author

Currently 14, and we may add more in the future

@fbmal7
Copy link
Contributor

fbmal7 commented Nov 16, 2022

Okay, and can you verify that you are using Hermes on iOS as well? There shouldn't be a cross-platform difference coming from Hermes with respect to this functionality, so I'm surprised that you aren't experiencing the same thing on both Android and iOS.

@mmmoussa
Copy link
Author

We are using hermes on iOS as well

@mmmoussa
Copy link
Author

Using codepush for the build, it gets stuck here:

Converting JS bundle to byte code via Hermes, running command:

node_modules/react-native/sdks/hermesc/osx-bin/hermesc -emit-binary -out build/CodePush/index.android.bundle.hbc build/CodePush/index.android.bundle -output-source-map -w

Using ./gradlew bundleRelease it gets stuck here:

> Task :app:bundleReleaseJsAndAssets

@fbmal7
Copy link
Contributor

fbmal7 commented Nov 16, 2022

One more question- are all those json files about the same size? The one you linked is 1.3Mb.

@fbmal7
Copy link
Contributor

fbmal7 commented Nov 16, 2022

I compiled that file you gave me on the command line and it took ~15 seconds. At 14 files, that would be ~210sec, or 3.5mins, which is in line with the rest of the times you are experiencing, correct? I think there might be another issue happening here at a layer above Hermes.

@mmmoussa
Copy link
Author

All of the json files are about the same size. Each file added ~3 minutes to the total build time, taking it from 5 minutes to 50 minutes.

@mmmoussa
Copy link
Author

Based on @tmikov's suggestion in #135, I changed all of our json files into js files that export the data as a string and then used JSON.parse in the code. Bundling is now back to being fast.

@tmikov
Copy link
Contributor

tmikov commented Nov 16, 2022

@fbmal7 the separate JSON files get combined into a single .js bundle by Metro, so if there are non-linear effects, performance would drop off sharply. We should try concatenating the provided file 14 times.

@fbmal7
Copy link
Contributor

fbmal7 commented Nov 16, 2022

@mmmoussa the one file you linked was solely a bunch of nested arrays with numbers inside of them. Is that type of data consistent with the rest of your files?

@mmmoussa
Copy link
Author

Yes, they are all exactly the same but with different numbers

@fbmal7
Copy link
Contributor

fbmal7 commented Nov 16, 2022

@mmmoussa Unfortunately I can't get this to repro. I built a fresh RN app and then downloaded the JSON file you linked and pasted it 6 times, changing some random values around. I then require'd the files inline in the App.js file and grabbed some random elements from each array, summed them up, and displayed the result. Running ./gradlew bundleRelease finished in 2mins.

@tmikov tmikov added the need-repro Potentially valid, but blocked on missing test case to reproduce label Nov 17, 2022
@tmikov
Copy link
Contributor

tmikov commented Nov 17, 2022

Closing, since we are not able to reproduce, and the submitter seems to have a work-around. Please, feel free to re-open if more data is available.

@tmikov tmikov closed this as completed Nov 17, 2022
@fbmal7
Copy link
Contributor

fbmal7 commented Nov 17, 2022

Reopening the task as the initial method we used to repro was flawed. We are now able to repro the issue, and have attributed the source to the slowdown @tmikov originally hypothesized: slow deduplication

@fbmal7 fbmal7 reopened this Nov 17, 2022
@tmikov tmikov removed the need-repro Potentially valid, but blocked on missing test case to reproduce label Nov 17, 2022
@fbmal7
Copy link
Contributor

fbmal7 commented Dec 14, 2022

The proper fix to this issue is finally landed in 221ce. This will not be available in RN until at least 0.72, though.

@fbmal7 fbmal7 closed this as completed Dec 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants