Skip to content

Conversation

@H4ad
Copy link
Contributor

@H4ad H4ad commented Apr 6, 2024

I know we avoid lazy loading but I think this case is worthy:

Before:

$ hyperfine --warmup 3 "node ./bin/npm-cli.js run empty"
Benchmark 1: node ./bin/npm-cli.js run empty
  Time (mean ± σ):     197.3 ms ±   3.5 ms    [User: 183.1 ms, System: 51.9 ms]
  Range (min … max):   193.2 ms … 207.5 ms    15 runs

After:

$ hyperfine --warmup 3 "node ./bin/npm-cli.js run empty"
Benchmark 1: node ./bin/npm-cli.js run empty
  Time (mean ± σ):     184.6 ms ±   2.3 ms    [User: 170.1 ms, System: 51.1 ms]
  Range (min … max):   181.6 ms … 190.5 ms    16 runs

@H4ad H4ad requested a review from a team as a code owner April 6, 2024 02:54
@lukekarrys lukekarrys changed the title perf: lazy load un-common dependencies for npm run fix(perf): lazy load un-common dependencies for npm run Apr 6, 2024
@wraithgar
Copy link
Member

We need to be 100% sure this code path isn't hit when running npm i -g npm

@wraithgar
Copy link
Member

normalizeData and gitHead are run as part of the prepare process. This is used by npm when preparing a package.json for publishing. That is not done during an install so I think this is safe for npm i -g npm

@wraithgar
Copy link
Member

Additionally, thinking of potential future problems. We are moving in the direction of doing fewer normalizations, not more. So the likelihood of us adding either of those steps during install is very low.

@wraithgar
Copy link
Member

Eventually normalize-package-data needs to be brought under the umbrella of this package. Eventually.

@wraithgar wraithgar merged commit fda5722 into npm:main Apr 7, 2024
@github-actions github-actions bot mentioned this pull request Apr 6, 2024
@H4ad H4ad deleted the perf/lazy-load-require branch April 7, 2024 16:06
wraithgar pushed a commit that referenced this pull request Apr 9, 2024
🤖 I have created a release *beep* *boop*
---


## [5.0.1](v5.0.0...v5.0.1)
(2024-04-07)

### Bug Fixes

*
[`fda5722`](fda5722)
[#87](#87) perf: lazy load
un-common dependencies for npm run (#87) (@H4ad)
*
[`71f09d6`](71f09d6)
[#88](#88) perf: only import
necessary functions from semver (#88) (@H4ad)

### Documentation

*
[`6ebb3c9`](6ebb3c9)
[#85](#85) readme: fix broken
badge URL (#85) (@10xLaCroixDrinker)

### Chores

*
[`66e0c23`](66e0c23)
[#80](#80) postinstall for
dependabot template-oss PR (@lukekarrys)
*
[`00e4bbb`](00e4bbb)
[#80](#80) bump
@npmcli/template-oss from 4.21.1 to 4.21.3 (@dependabot[bot])
*
[`d784aa8`](d784aa8)
[#77](#77) postinstall for
dependabot template-oss PR (@lukekarrys)
*
[`efeee22`](efeee22)
[#77](#77) bump
@npmcli/template-oss from 4.19.0 to 4.21.1 (@dependabot[bot])
*
[`a4df4cf`](a4df4cf)
[#56](#56) bump
read-package-json from 6.0.4 to 7.0.0 (@dependabot[bot])
*
[`f7c048a`](f7c048a)
[#58](#58) postinstall for
dependabot template-oss PR (@lukekarrys)
*
[`6240313`](6240313)
[#58](#58) bump
@npmcli/template-oss from 4.18.1 to 4.19.0 (@dependabot[bot])
*
[`5ab117c`](5ab117c)
[#57](#57) postinstall for
dependabot template-oss PR (@lukekarrys)
*
[`f56390e`](f56390e)
[#57](#57) bump
@npmcli/template-oss from 4.18.0 to 4.18.1 (@dependabot[bot])

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
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

Successfully merging this pull request may close these issues.

2 participants