Resolver perf: Weakly cache normalisation of exports field (4/n)
#1334
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
facebook-github-bot
added
the
CLA Signed
|
This pull request was exported from Phabricator. Differential Revision: D61841586 |
|
This pull request was exported from Phabricator. Differential Revision: D61841586 |
robhogan
force-pushed
the
export-D61841586
branch
from
6215e65 to
c50efaf
Compare
robhogan
added a commit
to robhogan/metro
that referenced
this pull request
Aug 27, 2024
…acebook#1334) Summary: Pull Request resolved: facebook#1334 Typically, a given package is required multiple times within a project, but currently we normalise the same `exports` JSON for every dependency. In RNTester, only three of its dependencies have non-primitive `exports` fields: ``` /node_modules/babel/runtime /node_modules/react /packages/react-native-test-library ``` But these are referenced 760 times, meaning 760 calls to the internal `normalizeExportsField`. This diff uses the property of Metro's upstream `ModuleCache._packageCache`, which avoids reading/parsing `package.json` unless it has changed. This makes `exports` stable, and (when it is non-primitive), weakly referenceable. By caching these values we reduce time spent in `normalizeExportsField` by ~10x and resolution time overall by 10% for RNTester, though the impact is likely to be larger for larger projects. By using a `WeakMap`, the cache can be GCed as corresponding packages are deleted from `ModuleCache`. ``` - **[Performance]**: Cache normalisation of `exports` fields for improved resolution performance. ``` Differential Revision: D61841586
…acebook#1334) Summary: Pull Request resolved: facebook#1334 Typically, a given package is required multiple times within a project, but currently we normalise the same `exports` JSON for every dependency. In RNTester, only three of its dependencies have non-primitive `exports` fields: ``` /node_modules/babel/runtime /node_modules/react /packages/react-native-test-library ``` But these are referenced 760 times, meaning 760 calls to the internal `normalizeExportsField`. This diff uses the property of Metro's upstream `ModuleCache._packageCache`, which avoids reading/parsing `package.json` unless it has changed. This makes `exports` stable, and (when it is non-primitive), weakly referenceable. By caching these values we reduce time spent in `normalizeExportsField` by ~10x and resolution time overall by 10% for RNTester, though the impact is likely to be larger for larger projects. By using a `WeakMap`, the cache can be GCed as corresponding packages are deleted from `ModuleCache`. ``` - **[Performance]**: Cache normalisation of `exports` fields for improved resolution performance. ``` Reviewed By: huntie Differential Revision: D61841586
|
This pull request was exported from Phabricator. Differential Revision: D61841586 |
robhogan
force-pushed
the
export-D61841586
branch
from
c50efaf to
1498f4d
Compare
facebook-github-bot
pushed a commit
that referenced
this pull request
Aug 27, 2024
…1334) Summary: Typically, a given package is required multiple times within a project, but currently we normalise the same `exports` JSON for every dependency. In RNTester, only three of its dependencies have non-primitive `exports` fields: ``` /node_modules/babel/runtime /node_modules/react /packages/react-native-test-library ``` But these are referenced 760 times, meaning 760 calls to the internal `normalizeExportsField`. This diff uses the property of Metro's upstream `ModuleCache._packageCache`, which avoids reading/parsing `package.json` unless it has changed. This makes `exports` stable, and (when it is non-primitive), weakly referenceable. By caching these values we reduce time spent in `normalizeExportsField` by ~10x and resolution time overall by 10% for RNTester, though the impact is likely to be larger for larger projects. By using a `WeakMap`, the cache can be GCed as corresponding packages are deleted from `ModuleCache`. ``` - **[Performance]**: Cache normalisation of `exports` fields for improved resolution performance. ``` Reviewed By: huntie Differential Revision: D61841586
|
This pull request has been merged in 16d2205. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary:
Typically, a given package is required multiple times within a project, but currently we normalise the same
exportsJSON for every dependency.In RNTester, only three of its dependencies have non-primitive
exportsfields:But these are referenced 760 times, meaning 760 calls to the internal
normalizeExportsField.This diff uses the property of Metro's upstream
ModuleCache._packageCache, which avoids reading/parsingpackage.jsonunless it has changed. This makesexportsstable, and (when it is non-primitive), weakly referenceable.By caching these values we reduce time spent in
normalizeExportsFieldby ~10x and resolution time overall by 10% for RNTester, though the impact is likely to be larger for larger projects. By using aWeakMap, the cache can be GCed as corresponding packages are deleted fromModuleCache.Differential Revision: D61841586