Skip to content

Conversation

@lukekarrys
Copy link
Contributor

@lukekarrys lukekarrys commented Apr 3, 2024

Closes #7314

This does the same thing as #7314 except using a new shared package for the log cleaning @npmcli/redact.

Here are the perf numbers from my local machine after setting a script with npm pkg set scripts.echo="echo". Note that the before seems slower on my machine than previous benchmarks, I think due to some security software that affects filesystem IO performance.

Before

❯ hyperfine --warmup 3 "node ./bin/npm-cli.js run echo" Benchmark 1: node ./bin/npm-cli.js run echo Time (mean ± σ): 349.4 ms ± 20.2 ms [User: 172.5 ms, System: 42.7 ms] Range (min … max): 319.0 ms … 376.1 ms 10 runs

After

❯ hyperfine --warmup 3 "node ./bin/npm-cli.js run echo" Benchmark 1: node ./bin/npm-cli.js run echo Time (mean ± σ): 170.7 ms ± 4.7 ms [User: 114.7 ms, System: 22.6 ms] Range (min … max): 163.6 ms … 184.9 ms 16 runs
@lukekarrys lukekarrys requested a review from a team as a code owner April 3, 2024 12:45
@lukekarrys
Copy link
Contributor Author

cc @H4ad

Copy link
Contributor

@H4ad H4ad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for promoting this, great PR and great improvements, also kudos to @sirenkovladd for finding this first!

@lukekarrys
Copy link
Contributor Author

@H4ad curious what benchmarks show in this PR for you compared to #7314?

@H4ad
Copy link
Contributor

H4ad commented Apr 3, 2024

Before:

Benchmark 1: node ./bin/npm-cli.js run echo Time (mean ± σ): 232.8 ms ± 3.2 ms [User: 235.4 ms, System: 54.8 ms] Range (min … max): 226.4 ms … 237.3 ms 12 runs 

After:

Benchmark 1: node ./bin/npm-cli.js run echo Time (mean ± σ): 196.0 ms ± 2.5 ms [User: 179.1 ms, System: 53.8 ms] Range (min … max): 192.0 ms … 199.5 ms 14 runs 

The new load time for each dependency (top N of most expensive):

MODULE_CJS 60649 [/home/h4ad/Projects/opensource/performance-analysis/npm-cli/node_modules/semver/functions/satisfies.js] [../classes/range]: 6.436ms MODULE_CJS 60649 [/home/h4ad/Projects/opensource/performance-analysis/npm-cli/node_modules/validate-npm-package-name/lib/index.js] [builtins]: 6.441ms MODULE_CJS 60649 [/home/h4ad/Projects/opensource/performance-analysis/npm-cli/lib/utils/error-message.js] [validate-npm-package-name]: 6.657ms MODULE_CJS 60649 [/home/h4ad/Projects/opensource/performance-analysis/npm-cli/lib/cli-entry.js] [semver/functions/satisfies]: 6.811ms MODULE_CJS 60649 [/home/h4ad/Projects/opensource/performance-analysis/npm-cli/node_modules/npmlog/lib/log.js] [gauge]: 7.489ms MODULE_CJS 60649 [/home/h4ad/Projects/opensource/performance-analysis/npm-cli/node_modules/normalize-package-data/lib/fixer.js] [validate-npm-package-license]: 7.668ms MODULE_CJS 60649 [/home/h4ad/Projects/opensource/performance-analysis/npm-cli/lib/utils/exit-handler.js] [./error-message.js]: 7.684ms MODULE_CJS 60649 [/home/h4ad/Projects/opensource/performance-analysis/npm-cli/lib/utils/log-shim.js] [npmlog]: 9.684ms MODULE_CJS 60649 [/home/h4ad/Projects/opensource/performance-analysis/npm-cli/lib/utils/exit-handler.js] [./log-shim.js]: 10.266ms MODULE_CJS 60649 [/home/h4ad/Projects/opensource/performance-analysis/npm-cli/workspaces/config/lib/index.js] [@npmcli/map-workspaces]: 10.349ms MODULE_CJS 60649 [/home/h4ad/Projects/opensource/performance-analysis/npm-cli/node_modules/@npmcli/package-json/lib/normalize.js] [normalize-package-data/lib/fixer.js]: 10.578ms MODULE_CJS 60649 [/home/h4ad/Projects/opensource/performance-analysis/npm-cli/lib/npm.js] [@npmcli/config]: 15.221ms MODULE_CJS 60649 [/home/h4ad/Projects/opensource/performance-analysis/npm-cli/node_modules/@npmcli/package-json/lib/index.js] [./normalize.js]: 17.558ms MODULE_CJS 60649 [/home/h4ad/Projects/opensource/performance-analysis/npm-cli/node_modules/@npmcli/run-script/lib/run-script.js] [@npmcli/package-json]: 18.414ms MODULE_CJS 60649 [/home/h4ad/Projects/opensource/performance-analysis/npm-cli/lib/cli-entry.js] [./utils/exit-handler.js]: 18.496ms MODULE_CJS 60649 [/home/h4ad/Projects/opensource/performance-analysis/npm-cli/lib/commands/run-script.js] [@npmcli/run-script]: 20.343ms MODULE_CJS 60649 [/home/h4ad/Projects/opensource/performance-analysis/npm-cli/lib/npm.js] [./commands/run-script.js]: 22.623ms MODULE_CJS 60649 [/home/h4ad/Projects/opensource/performance-analysis/npm-cli/lib/cli-entry.js] [./npm.js]: 25.942ms MODULE_CJS 60649 [] [/home/h4ad/Projects/opensource/performance-analysis/npm-cli/bin/npm-cli.js]: 60.411ms 
@lukekarrys lukekarrys merged commit 87a61fc into latest Apr 3, 2024
@lukekarrys lukekarrys deleted the lk/redact branch April 3, 2024 14:24
@github-actions github-actions bot mentioned this pull request Apr 3, 2024
@wraithgar wraithgar mentioned this pull request Apr 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants