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

Makes file matching / expanding much faster (in some situations twice as fast) #1562

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

cspotcode
Copy link

This PR avoids lots of unnecessary or duplicate filesystem operations within grunt's file.expand, file.expandMapping, and task.normalizeMultiTaskFiles. It makes file matching much faster without adding much complexity to grunt.

The first change is to persist glob's internal caches across multiple calls to glob.sync. This is a documented feature of the glob library: https://github.com/isaacs/node-glob#options
(see also: isaacs/node-glob#306)

The second change is to avoid all fs calls for exclusion patterns beginning with a "!". These patterns only exclude filepaths that have already been discovered by an inclusion pattern, so they never need to do any filesystem calls of their own.

I've created a demonstration repository that shows how in certain situations these changes make file matching twice as fast. On my laptop, a Macbook with SSD, these improvements reduce glob matching from 2.2 to 1.1 seconds.
https://github.com/cspotcode/grunt-file-matching-performance

@jsf-clabot
Copy link

jsf-clabot commented Nov 7, 2016

CLA assistant check
All committers have signed the CLA.

@jquerybot
Copy link

Thank you for your pull request. It looks like this may be your first contribution to a jQuery Foundation project, if so we need you to sign our Contributor License Agreement (CLA).

📝 Please visit http://contribute.jquery.org/CLA/ to sign.

After you signed, the PR is checked again automatically after a minute. If there's still an issue, please reply here to let us know.


If you've already signed our CLA, it's possible your git author information doesn't match your CLA signature (both your name and email have to match), for more information, check the status of your CLA check.

…k.normalizeMultiTaskFiles` by caching glob fs results across multiple calls to `glob.sync`
- IO is not necessary for exclusions because they exclude file paths that have already been discovered.  Using `file.match` / `minimatch` is sufficient.
- adds optional `excludeFn` callback to `processPatterns`. If provided, it will be called instead of `fn` for exclusion patterns.
@cspotcode cspotcode force-pushed the feature/file-matching-performance branch from 3c94d8b to 56131e3 Compare November 8, 2016 17:48
@cspotcode
Copy link
Author

Any chance of getting this merged? Is there anything I can clarify or update? Are there any problems that prevent merging? I've signed the CLA; @jquerybot's commit status is incorrect.

Base automatically changed from master to main March 22, 2021 14:45
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.

5 participants