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.

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

perf(npm): startup time reduce #705

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

dalisoft
Copy link

@dalisoft dalisoft commented Apr 15, 2024

Closes #703

⚡ Summary

This PR should improve startup performance to get even more out of your performant library

☑️ Checklist

  • Check locally
  • Add tests
  • Add documentation

@dalisoft
Copy link
Author

Make test i'm getting these results

❯ make test
go test -cpu 24 -race -count=1 -timeout=30s ./...
?       github.com/evilmartians/lefthook        [no test files]
?       github.com/evilmartians/lefthook/cmd    [no test files]
ok      github.com/evilmartians/lefthook/internal/config        1.318s
ok      github.com/evilmartians/lefthook/internal/git   1.504s
?       github.com/evilmartians/lefthook/internal/lefthook/runner/exec  [no test files]
?       github.com/evilmartians/lefthook/internal/templates     [no test files]
?       github.com/evilmartians/lefthook/internal/system        [no test files]
?       github.com/evilmartians/lefthook/internal/version       [no test files]
ok      github.com/evilmartians/lefthook/internal/lefthook      1.737s
ok      github.com/evilmartians/lefthook/internal/lefthook/runner       1.287s
ok      github.com/evilmartians/lefthook/internal/lefthook/runner/filters       1.730s
ok      github.com/evilmartians/lefthook/internal/log   1.483s
lefthook on  perf-cli-improvement [?] via 🐹 v1.22.2 took 11s 
❯ 

It is okay to check check locally?

@dalisoft
Copy link
Author

Testing locally currently isn't possible because of locally it's failed. make prepare got these results on macOS

lefthook/packaging on  perf-cli-improvement [?] 
❯ make prepare
find npm/ -name 'README.md' -exec rm \{} \;
find npm/ -type f -name 'lefthook*' -exec rm \{} \;
git clean -fdX npm-installer/ npm-bundled/ npm-bundled/bin/
Removing npm-bundled/node_modules/
Removing npm-installer/node_modules/
git clean -fdX rubygems/libexec/ rubygems/pkg/
find npm -name 'package.json' -type f -print0 | xargs -0 sed -E -i "s/\"version\": \".+\"/\"version\": \"1.6.10\"/"
sed: 1: "npm/lefthook-linux-x64/ ...": extra characters at the end of n command
make: *** [set-version] Error 1
lefthook/packaging on  perf-cli-improvement [?] 
❯ 

@mrexox
Copy link
Member

mrexox commented Apr 15, 2024

There is no need to do this for npm-bundled and npm-installer packages because the binaries from these packages are executed directly in a git hook (with full paths).

So, only lefthook package which uses optionalDependencies is the target for this optimization.

@dalisoft
Copy link
Author

@mrexox I'm tried to optimize all of npm packages so any of type npm packages uses this performance boost

@dalisoft
Copy link
Author

@mrexox I am updated PR and now changes affects only lefthook package

@mrexox
Copy link
Member

mrexox commented Apr 15, 2024

I understand, but this optimization would work only if you run npx @evilmartians/lefthook run which is usually not the way lefthook is being used. Usually it gets executed implicitly from a git hook

@mrexox
Copy link
Member

mrexox commented Apr 15, 2024

Thank you! I will test the PR later this week 👍

@dalisoft
Copy link
Author

Thank you too for your time and looking into this PR

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.

Performance for npm could be even more faster?
2 participants