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.

Sign up for GitHub

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

Jump to bottom

Performance for npm could be even more faster? #703

Open
dalisoft opened this issue Apr 13, 2024 · 4 comments · May be fixed by #705
Open

Performance for npm could be even more faster? #703

dalisoft opened this issue Apr 13, 2024 · 4 comments · May be fixed by #705
Labels
feature request A new lefthook feature description

Comments

@dalisoft
Copy link

⚡ Summary

Hi @evilmartians team/contributors

Tuning npm package could lead into performance improvements and reducing startup execution.

Opened similar PR/Issue on other projects but impossible to check locally due of hard running local or lacking of running locally easier script.

As these PR not merged into neither of them, so first decided to ask from you

Value

Command Mean [ms] Min [ms] Max [ms] Relative
../lint-staged.sh 154.3 ± 2.8 151.1 159.3 1.00
lint-staged 306.9 ± 6.1 300.3 320.8 1.99 ± 0.05
lefthook run pre-commit 202.8 ± 4.4 197.3 210.1 1.31 ± 0.04
./lefthook run pre-commit 162.7 ± 3.2 160.4 170.7 1.05 ± 0.03

Behavior and configuration changes

If npx is not important, can be tuned for using lefthook binary from optionalDependencies or prepare script can download and change binary for main npm package bin field.

Test script locally for you

postinstall.js 
import { prepare, maps } from "binary2npm";

maps.os = {
  linux: "Linux",
  freebsd: "Freebsd",
  darwin: "MacOS",
  win32: "Windows",
};
maps.arch = {
  arm64: "arm64",
  x64: "x86_64",
  x86: "x86_64",
};

await prepare({
  remote: "github",
  author: "evilmartians",
  repository: "lefthook",
  remoteToken: process.env.GITHUB_TOKEN,
  binary: "lefthook",
  orders: ["binary", "version", "os", "arch"],
});
  • Try run node postinstall.js
  • Then benchmark direct running versus using custom bin.js from npm package
@dalisoft dalisoft added the feature request A new lefthook feature description label Apr 13, 2024
@mrexox
Copy link
Member

mrexox commented Apr 15, 2024

Hey! Lefthook actually implements different ways of distribution. We have lefthook package that utilized optionalDepdencies, we have @evilmartians/lefthook-installer which downloads the binary in a postinstall step and we have @evilmartians/lefthook which simply contains all the binaries.

So, what is your suggestion for improvement? And what is the problem with current npm packages you see?

@dalisoft
Copy link
Author

I looked into other ways of distribution and it is actually 30% slower than suggested improvements.

I am suggesting using direct binary for package.json like {"bin":"lefthook.exe"} or {"bin":"lefthook"} so no additional overhead when executing lefthook from CLI
Currently no problem but only 30% performance loss and after improvement CLI speed for npm could be improved by 30%

@mrexox
Copy link
Member

mrexox commented Apr 15, 2024

Oh, I see. Ok, I agree, it would be great to have this improvement.

@dalisoft dalisoft linked a pull request Apr 15, 2024 that will close this issue
Open
3 tasks
@dalisoft
Copy link
Author

@mrexox See #705 please

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
feature request A new lefthook feature description
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

perf(npm): startup time reduce dalisoft/lefthook
2 participants
@dalisoft @mrexox