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

Introduce shallow gitignore parsing for better performance #66

Closed

Conversation

marionebl
Copy link
Contributor

@marionebl marionebl commented Jan 28, 2018

This changes the .gitignore handling to only scan for ignore files at $CWD/.gitignore, which results in better performance of globby runs.

Previously globby scanned for **/.gitignore, which increased execution time in relation to the number of files in `cwd.

This does not change the ignore filter to be applied before glob tasks are created.

Considerations

  • Should nested .gitignore files be supported?
  • Probably should move benchmarking for .gitignore handling here
  • Should ignore files be searched from ${gitRoot}/ instead? (How to handle non-git dirs then?)

Details

Tested via https://github.com/pixelass/ava-xo-test:

git clone https://github.com/pixelass/ava-xo-test.git
git clone https://github.com/sindresorhus/globby.git

cd globby 
git checkout perf-shallow-gitignore
npm install

cd ../ava-xo-test
npm install
# Prevent xo from finding the local (old) copy
mv node_modules/xo/cli.js node_modules/xo/_cli.js

# Local
# node --inspect --inspect-brk ./node_modules/xo/_cli.js 

# Development
# node --inspect --inspect-brk ../xo/cli.js 

Before

~/Documents/oss/ava-xo-test master*
λ time ./node_modules/.bin/xo

  4 errors
       58.69 real        52.60 user         7.43 sys

globby-slow-gitignore

The area with red borders signifies time spent in getGitIgnoreFilter

CPU profile before

After

~/Documents/oss/ava-xo-test master*
λ time node ../xo/cli.js

  5 errors
       42.90 real        39.16 user         5.81 sys

globby-simplified-gitignore

CPU profile after

@marionebl
Copy link
Contributor Author

Obsolete as per v8

@marionebl marionebl closed this Feb 13, 2018
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.

1 participant