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

Slower than node-glob when ignoring a directory with dir/** #45

Closed
pvdlg opened this issue Jan 30, 2018 · 3 comments
Closed

Slower than node-glob when ignoring a directory with dir/** #45

pvdlg opened this issue Jan 30, 2018 · 3 comments
Assignees

Comments

@pvdlg
Copy link

pvdlg commented Jan 30, 2018

See benchmark file: https://gist.github.com/pvdlg/8f72824d81263efc71b30fd443aa8b0e

npm install matcha, fast-glob, glob
./node_modules/.bin/matcha fast-blog-bench.js
patterns: 'a/*', ignore: ['a']

231 op/s » fast-glob sync
136 op/s » glob sync

patterns: 'a/*', ignore: ['a/**']

302 op/s » fast-glob sync
31,096 op/s » glob sync

It's quite an edge case as it seems the performance difference happens only when the the ignore pattern make so that no pattern will match. It seems that node-glob is able to figure out that nothing will ever with pattern 'a/*' and ignore ['a/**'], while fast-glob still goes through the directory.

With pattern 'a/*' and ignore ['a'] it seems neither fast-glob nor node-glob figure out that nothing would match and both attempt to go through the directory.
I thought the directory would be ignored completely in such case.

I'm not sure it's a big issue as it concerns a pattern/ignore config that doesn't make much sense, but I though it worth mentioning in case the perf improvement is really simple to do. Maybe ignoring patterns that are included within ignore pattern can bring other more useful performance improvements.

@mrmlnc
Copy link
Owner

mrmlnc commented Jan 31, 2018

Hello, @pvdlg,

Thanks for good catch. Yeap, I know about this problem. The current behavior is described in «How to exclude directory from reading?» section.

Unfortunately, I don't remember why I decided to only describe the current behavior. It seems that were borderline cases when the directory still need to read the case even if you specify a pattern that excludes this directory. I will try to find artifacts notes of these conversations.

@mrmlnc
Copy link
Owner

mrmlnc commented Feb 6, 2018

test
├── a
│   ├── a.css
│   ├── a.js
│   └── a.txt
└── b
    ├── b.css
    ├── b.js
    └── b.txt
const globby = require('globby');

const entries = globby.sync(['**/*', '!a/**', 'a/*.{js,css}'], { cwd: 'test' });

console.dir(entries, { colors: true });

In this case, globby respects the order of patterns and results will be:

[ 'b/b.css', 'b/b.js', 'b/b.txt', 'a/a.css', 'a/a.js' ]

So in this case we need to read the a directory.

But the fast-glob package does not respect the order of patterns. First, all the negative patterns are applied, and only then the positive patterns. Results will be:

[ 'b/b.css', 'b/b.js', 'b/b.txt' ]

In this case, we can safely ignore the reading of directories that end with the globstar. I file PR #49 for this case.

@mrmlnc mrmlnc mentioned this issue Feb 9, 2018
5 tasks
@mrmlnc
Copy link
Owner

mrmlnc commented Feb 12, 2018

Landed by fast-glob@2.0.3.

@mrmlnc mrmlnc closed this as completed Feb 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants