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

Performance regression between 0.73.9 and 0.76.8 #1218

Open
asherLZR opened this issue Feb 9, 2024 · 4 comments
Open

Performance regression between 0.73.9 and 0.76.8 #1218

asherLZR opened this issue Feb 9, 2024 · 4 comments

Comments

@asherLZR
Copy link

asherLZR commented Feb 9, 2024

Do you want to request a feature or report a bug?
Bug

What is the current behavior?
Metro bundling in development mode takes approximately twice as long on 0.76.8 compared to 0.73.9, after upgrading RN from 0.71.7 to 0.72.9. As remediation we have had to downgrade metro dependencies.

We suspect this is related to the symlink support improvements. With or without the transform cache the degradation is significant. We measure the first bundle time with the built-in perf logger.

  • Metro 0.73.9
    • Without cache 261s
    • With cache 116s
  • Metro 0.76.8
    • Without cache 315s
    • With cache 216s

I'm reporting this as a separate issue to #1208 even though many of the fingerprints are similar, as we see a clear change between versions. Feel free to close in case of duplicate. We live in a monorepo where the directory structure is almost identical. However, we have always used the default Metro resolver and use Metro only for development.

my-app/
├─ node_modules/
├─ apps/
│  ├─ native
│  ├─ web
├─ packages/
│  ├─ ...screens/

Our command looks like

  node ./node_modules/.bin/react-native start \
    --watchFolders $PWD \ 
    --projectRoot apps/native \
    --config apps/native/metro.config.js

I will investigate further and check next week to see if we can upgrade to the latest metro despite being currently tied to RN72 for now.

If the current behavior is a bug, please provide the steps to reproduce and a minimal repository on GitHub that we can yarn install and yarn test.

I suspect this only happens in a large app.

What is the expected behavior?

No performance regressions.

Please provide your exact Metro configuration and mention your Metro, node, yarn/npm version and operating system.

Metro: 0.76.8
Node: 18.17.0
yarn: 1.22.10
OS: Sonoma 14.3

@robhogan
Copy link
Contributor

robhogan commented Feb 10, 2024

Hi @asherLZR - thanks for the report. The specific detail of a regression between those versions (and how clear your numbers make that) is new information for me, so I'm happy to investigate this as a separate issue until we've pinned down the point of regression. Indeed the fact that the regression is similar with and without cache suggests a main thread issue, and most of that is (or should be) module resolution.

We didn't knowingly regress while introducing symlink support (in fact we were pretty careful trying not to) but obviously something has slipped between those versions that we're not aware of.

I will investigate further and check next week to see if we can upgrade to the latest metro despite being currently tied to RN72 for now.

We don't normally recommend this (we don't guarantee new Metro supports older RN, especially around things like downlevelling for Hermes) but for the sake of experiment it can be done with eg Yarn resolutions, taking care around the breaking changes in Metro's release notes. For Metro 0.80 with RN 0.72 I'd recommend using the RN 0.73-style metro.config.js, extending @react-native/metro-config@0.73.4. I think that should work.

In the mean time I'm going to try to reproduce something with an artificial project of a similar structure - if I can't do that I'd love your help bisecting this down to at least a particular Metro release, if you're willing, or if (NDA) access to your repo is possible I'd be happy to take a look. I feel like this is pretty important to get to the bottom of!

Update
There's a detectable resolution perf regression on acbfe63 between 0.74.1 and 0.75.0 - I'd be interested to know whether that reproduces on your repo. Investigating further...

@asherLZR
Copy link
Author

I would be happy to assist with the bisect of this for now and we can explore further options if need be. My test method so far has been to run once with --reset-cache then 3 subsequent times without.

I've tried to keep things constant in the environment by not doing memory/cpu intensive tasks but there will be some uncertainty. Despite this, there is indisputable change between 0.75.0 and 0.76.8. Where possible, I will continue with narrowing down the versions this week.

  • Baseline 0.73.9
  • 0.74.1 no change relative to baseline
  • 0.75.0 +23% relative to baseline
  • 0.76.8 +80% relative to baseline

@robhogan
Copy link
Contributor

Ouch, that looks like two regressions.

The first one (in 0.75.0) is due to an increase in path.resolve and path.relative calls, which are maybe-surprisingly expensive. We're working on a fix for that.

The second is new to me - that'll take some more bisection. Hopefully it's reproducible in the sample projects I have.

Thanks again for your help with this.

@asherLZR
Copy link
Author

Thanks for the helpful insight. I've managed to run a new set of tests. The good news is that between 0.76.2 and 0.80.6 there appears to be little regression (on par with 0.76.8 as described in the original post).

My results tell me that something introduced between 0.76.1 and 0.76.2 caused the further slow down of ~20%.

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

No branches or pull requests

2 participants