metro icon indicating copy to clipboard operation
metro copied to clipboard
verified publisher icon facebook

Default metro resolver slower than MetroSymlinksResolver in a monorepo

Open abejfehr opened this issue 1 year ago • 4 comments

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

I'm not sure if this counts as a bug exactly, but it's a performance issue for monorepos in cases where most of the app's code (including node_modules) is outside the directory that the app is in.

What is the current behavior?

When building an app bundle with metro using the default metro resolver in a monorepo, the build times are a lot longer than if you use @rnx-kit/metro-resolver-symlinks.

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.

To reproduce this somewhat effectively, you'd need a really large monorepo that looks something like this:

apps/
├── mobile # this is the RN app, including metro.config.js
└── web
libs/
├── mobile/
│   ├── screens /
│   │   ├── screen-1
│   │   ├── ...
│   │   └── screen-n
│   └── packages/
│       ├── package-1
│       ├── ...
│       └── package-n
└── shared/
    └── packages/
        ├── package-1
        ├── ...
        └── package-n
node_modules/

and in your metro config, something like this:

const path = require('path');
const { getDefaultConfig, mergeConfig } = require('@react-native/metro-config');

const defaultConfig = getDefaultConfig(__dirname);

const config = {
  watchFolders: [
    path.resolve(workspaceRoot, 'node_modules'),
    path.join(__dirname, '../../')
  ],
  projectRoot: __dirname,
};

module.exports = mergeConfig(defaultConfig, config);

If you bundle the app using a command like:

node --cpu-prof ./node_modules/.bin/react-native bundle --platform ios --dev true --minify false --entry-file index.js --reset-cache --bundle-output /tmp/bundle.js --max-workers 1

and analyze the generated .cpuprofile file in speedscope.net in the "Left Heavy" view, you can see that a non-trivial amount of time is spent in here:

image

It appears to be spending a lot of time (in this case over a minute because I'm using only 1 worker thread) in relative in (ironically) fast_path.js here, called by TreeFS here.

I think that happens because the files being imported exist outside of #rootDir, which in this example is /apps/mobile, whereas most of the code is in /libs/... in this repo.

Even though we have no need for resolving symlinks, we use MetroSymlinksResolver because it's just more performant in this scenario.

Possibly related: if you add a console.log inside the relative function and log the args, you see that it seems to run with every permutation of extension, e.g.

relative /Users/abejfehr/Development/some-monorepo/apps/mobile /Users/abejfehr/Development/some-monorepo/node_modules/react.native/package.json
relative /Users/abejfehr/Development/some-monorepo/apps/mobile /Users/abejfehr/Development/some-monorepo/node_modules/react.native
relative /Users/abejfehr/Development/some-monorepo/apps/mobile /Users/abejfehr/Development/some-monorepo/node_modules/react
relative /Users/abejfehr/Development/some-monorepo/apps/mobile /Users/abejfehr/Development/some-monorepo/node_modules/react.ios.ts/package.json
relative /Users/abejfehr/Development/some-monorepo/apps/mobile /Users/abejfehr/Development/some-monorepo/node_modules/react.ios.ts
relative /Users/abejfehr/Development/some-monorepo/apps/mobile /Users/abejfehr/Development/some-monorepo/node_modules/react.native.ts/package.json
relative /Users/abejfehr/Development/some-monorepo/apps/mobile /Users/abejfehr/Development/some-monorepo/node_modules/react.native.ts
relative /Users/abejfehr/Development/some-monorepo/apps/mobile /Users/abejfehr/Development/some-monorepo/node_modules/react.ts/package.json
relative /Users/abejfehr/Development/some-monorepo/apps/mobile /Users/abejfehr/Development/some-monorepo/node_modules/react.ts
relative /Users/abejfehr/Development/some-monorepo/apps/mobile /Users/abejfehr/Development/some-monorepo/node_modules/react.ios.js/package.json
relative /Users/abejfehr/Development/some-monorepo/apps/mobile /Users/abejfehr/Development/some-monorepo/node_modules/react.ios.js
relative /Users/abejfehr/Development/some-monorepo/apps/mobile /Users/abejfehr/Development/some-monorepo/node_modules/react.native.js/package.json
relative /Users/abejfehr/Development/some-monorepo/apps/mobile /Users/abejfehr/Development/some-monorepo/node_modules/react.native.js
relative /Users/abejfehr/Development/some-monorepo/apps/mobile /Users/abejfehr/Development/some-monorepo/node_modules/react.js/package.json
relative /Users/abejfehr/Development/some-monorepo/apps/mobile /Users/abejfehr/Development/some-monorepo/node_modules/react.js
relative /Users/abejfehr/Development/some-monorepo/apps/mobile /Users/abejfehr/Development/some-monorepo/node_modules/react.ios.tsx/package.json
relative /Users/abejfehr/Development/some-monorepo/apps/mobile /Users/abejfehr/Development/some-monorepo/node_modules/react.ios.tsx
relative /Users/abejfehr/Development/some-monorepo/apps/mobile /Users/abejfehr/Development/some-monorepo/node_modules/react.native.tsx/package.json
relative /Users/abejfehr/Development/some-monorepo/apps/mobile /Users/abejfehr/Development/some-monorepo/node_modules/react.native.tsx
relative /Users/abejfehr/Development/some-monorepo/apps/mobile /Users/abejfehr/Development/some-monorepo/node_modules/react.tsx/package.json
relative /Users/abejfehr/Development/some-monorepo/apps/mobile /Users/abejfehr/Development/some-monorepo/node_modules/react.tsx
relative /Users/abejfehr/Development/some-monorepo/apps/mobile /Users/abejfehr/Development/some-monorepo/node_modules/react.ios.graphql/package.json
relative /Users/abejfehr/Development/some-monorepo/apps/mobile /Users/abejfehr/Development/some-monorepo/node_modules/react.ios.graphql
relative /Users/abejfehr/Development/some-monorepo/apps/mobile /Users/abejfehr/Development/some-monorepo/node_modules/react.native.graphql/package.json
relative /Users/abejfehr/Development/some-monorepo/apps/mobile /Users/abejfehr/Development/some-monorepo/node_modules/react.native.graphql
relative /Users/abejfehr/Development/some-monorepo/apps/mobile /Users/abejfehr/Development/some-monorepo/node_modules/react.graphql/package.json
relative /Users/abejfehr/Development/some-monorepo/apps/mobile /Users/abejfehr/Development/some-monorepo/node_modules/react.graphql
relative /Users/abejfehr/Development/some-monorepo/apps/mobile /Users/abejfehr/Development/some-monorepo/node_modules/react.ios.mjs/package.json
relative /Users/abejfehr/Development/some-monorepo/apps/mobile /Users/abejfehr/Development/some-monorepo/node_modules/react.ios.mjs
relative /Users/abejfehr/Development/some-monorepo/apps/mobile /Users/abejfehr/Development/some-monorepo/node_modules/react.native.mjs/package.json
relative /Users/abejfehr/Development/some-monorepo/apps/mobile /Users/abejfehr/Development/some-monorepo/node_modules/react.native.mjs
relative /Users/abejfehr/Development/some-monorepo/apps/mobile /Users/abejfehr/Development/some-monorepo/node_modules/react.mjs/package.json
relative /Users/abejfehr/Development/some-monorepo/apps/mobile /Users/abejfehr/Development/some-monorepo/node_modules/react.mjs
relative /Users/abejfehr/Development/some-monorepo/apps/mobile /Users/abejfehr/Development/some-monorepo/node_modules/react.ios.json/package.json
relative /Users/abejfehr/Development/some-monorepo/apps/mobile /Users/abejfehr/Development/some-monorepo/node_modules/react.ios.json
relative /Users/abejfehr/Development/some-monorepo/apps/mobile /Users/abejfehr/Development/some-monorepo/node_modules/react.native.json/package.json
relative /Users/abejfehr/Development/some-monorepo/apps/mobile /Users/abejfehr/Development/some-monorepo/node_modules/react.native.json
relative /Users/abejfehr/Development/some-monorepo/apps/mobile /Users/abejfehr/Development/some-monorepo/node_modules/react.json/package.json
relative /Users/abejfehr/Development/some-monorepo/apps/mobile /Users/abejfehr/Development/some-monorepo/node_modules/react.json
relative /Users/abejfehr/Development/some-monorepo/apps/mobile /Users/abejfehr/Development/some-monorepo/node_modules/react/package.json
relative /Users/abejfehr/Development/some-monorepo/apps/mobile /Users/abejfehr/Development/some-monorepo/node_modules/react/index.js.native
relative /Users/abejfehr/Development/some-monorepo/apps/mobile /Users/abejfehr/Development/some-monorepo/node_modules/react/index.js

^ this is an actual portion (with some path redactions) of that log of it resolving react

It doesn't feel very effiicent to me, and it might be nice if there was a way to call relative once on the path and append the path with every permutation of extension on the already-relative'd version of the path, but I don't how involved a change like that might be.

What is the expected behavior?

Ideally, the default resolver would be closer to the same speed as MetroSymlinksResolver in the case of a monorepo where most of the app's code is outside of #rootDir.

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

I've given an abridged version of the metro configuration above.

OS and package manager doesn't matter much, but I use macOS and yarn 1.

abejfehr avatar Feb 04 '24 21:02 abejfehr

Hi @abejfehr - thanks for taking the time to deep-dive into this - it's always good to find out how Metro is performing in setups "in the wild" outside Meta, and particularly in this detail.

A couple of high level points -

  • You've hit on two known perf/efficiency issues with the resolver here

    1. That we repeat a lot of work in typical sequences of existence checks - eg for multiple candidate extensions on the same basename (also finding the nearest package.json). This is possible to address with the addition of some fine-tuned APIs wired from TreeFS through to ResolutionContext. I plan to do this as part of the incremental resolution improvements underway at the moment (https://github.com/facebook/metro/pull/1161), which will likely land around RN 0.74.
    2. That fast_path.relative is built to optimise for resolutions being under the project root, but is actually slower than path.relative for anything else. This is the target of https://github.com/facebook/metro/pull/1210, which should improve the case of path under a parent or grandparent of the project root about 10x. I had this in the works already but you've reminded me to revive it :). https://github.com/facebook/metro/pull/1209 should help too (fast_path.resolve may be the next entry down in your CPU profile?). This will be in the next Metro patch release for RN 0.73 - it'd be great to know how it works for you.

  • Secondly, I am surprised @rnx-kit/metro-resolver-symlinks is faster - we don't use it and I'm not very close to its internals, but my understanding was that it augments Metro's existence checks with an IO fallback, so I'd expect it to be doing more work if anything. Since you have a like-for-like comparison with your project, might you be able to check what work Metro isn't doing as a result of using @rnx-kit/metro-resolver-symlinks? Maybe it's not using Metro's existence check for anything under a node_modules package?

robhogan avatar Feb 05 '24 21:02 robhogan

Hi @abejfehr - Metro 0.80.6 was released yesterday with the two path manipulation performance improvements I mentioned.

If you're using RN 0.73 you should get that update by refreshing your lockfile - it'd be interesting to see whether that helps.

robhogan avatar Feb 16 '24 12:02 robhogan

Unfortunately we're using 0.72 at this time, but thank you for letting me know!

I'll also see if I can get the information you asked for previously about "what work Metro isn't doing as a result of using @rnx-kit/metro-resolver-symlinks"

abejfehr avatar Feb 22 '24 17:02 abejfehr