Use minimatch to handle negative globs, fixes #2; Run positive globs in parallel, fixes #3

[UltCombo]

Fixes #2
Fixes #3

Negative globs handling is over 2.5x faster now in regards to my previous benchmark.

[UltCombo]

Here's a diagram of the control flow this PR implements:

@sindresorhus PTAL when you have time.

[UltCombo]

(moved benchmarking to #10)

Benchmarks before this PR:

Benchmarks after this PR:

Looks like the negative glob perf improvement is not visible in this benchmark because all the negated files are in the same directory, which is far less consuming than walking deeply nested directories (as node_modules/** would). Guess I'll need to tweak the benchmark.

[UltCombo]

The benchmarks above were using 100 negated files. It looks like the minimatch approach is only faster when there are > 500 negated files (my initial test before proposing this PR was using 11,000 negated files).

[UltCombo]

Improved tests:

It is odd how the current master's negative globs handling is faster in some cases -- node-glob reads from disk and matches using minimatch, it should always be slower than just using minimatch directly. Perhaps it is because they use the Minimatch class.

[UltCombo]

(Travis data in comment above was wrong, removed it)

And yes, using the Minimatch class is faster, now the perf branch is always faster than master:

You need to merge/rebase the bench branch onto perf to run the bench.

[UltCombo]

@sindresorhus do you want to do some review (code style validation, etc.) or should I just merge?

[UltCombo]

Sorry for the spam of screenshots, the only important one is the latest one, the rest are outdated.

[sindresorhus]

Reviewed. Superb work. And thanks for including the graph, nice touch!

[sindresorhus]

🎆 And happy NYE! 🎆

[sindresorhus]

Wonder if there's more opportunities for perf improvements. Probably not, but the bench makes it a lot easier to explore, thanks for that :)

[sindresorhus]

Weird. I'm seeing different results for globby async (working directory) and globby async (upstream/master) even though both are really master.

             354 op/s » globby async (working directory)
             384 op/s » globby async (upstream/master)
             283 op/s » globby sync (working directory)
             281 op/s » globby sync (upstream/master)

The sync version seems more stable though.

[UltCombo]

@sindresorhus Happy new year to you too. :D

I'm seeing different results

That's odd indeed. Did you update the globby dependency after merging?

rm -rf node_modules && npm i

Ah, and last time I checked, npm had a caching issue -- when pulling from a GitHub repo, it would cache the repository's package.json version as if it was the real version release, then next time npm install pulls from the same GitHub repo, it would see the package.json version is still the same and use the old cached version. I'm not sure if this is fixed, but do a npm cache clean globby just to be sure.

[UltCombo]

@sindresorhus

Wonder if there's more opportunities for perf improvements.

Oh, I just found one more small optimization we can make (see gulpjs/glob-stream#33), but it shouldn't matter much. I'll push it to globby later if it doesn't make the code too ugly.

I believe the next big optimization would be to use node-glob's ignore API, but that is not implemented yet. ;)

As for benchmarking, I'll open an issue to discuss some possible improvements.

[UltCombo]

e402d9f

[sindresorhus]

Did you update the globby dependency after merging?

Yes, ofc. npm caches branches so you kinda have to do it.

I believe the next big optimization would be to use node-glob's ignore API, but that is not implemented yet. ;)

That will help a lot! :)

Seeing how Isaac don't want to support multiple patterns natively it seems that this module will have to stay.

[UltCombo]

Oh I'm also running into the same perf diff issue, looks like the first bench is up to 10% slower for some reason.

If I add another benchmark test before the actual benchmarks then it evens out the difference a bit:

                     negative globs (100 negated paths)
            227 op/s » warm up (ignore)
            240 op/s » globby async (working directory)
            244 op/s » globby async (upstream/master)

[UltCombo]

I fiddled a bit with benchmark.js, and according to it, upstream/master is 10% slower hahah.
This benchmarking stuff is too much headache.

[sindresorhus]

@UltCombo hmm, maybe it has something to do with the filesystem/harddrive cache.

[UltCombo]

That's the most likely yeah, seeing as filesystem access is the biggest bottleneck here.