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

Using src globs can be really slow with negation #24

Closed
necolas opened this issue Sep 16, 2014 · 12 comments
Closed

Using src globs can be really slow with negation #24

necolas opened this issue Sep 16, 2014 · 12 comments
Milestone

Comments

@necolas
Copy link

necolas commented Sep 16, 2014

Trying to use negation in globs can be extremely slow if there are many files in the negated paths (e.g., node_modules). For example:

[ '**/*.js', '!node_modules/**' ]

We're still in the initial phases of a project and while that pattern works, it was causing a lint task to take more than 4s to finish. Changing the pattern to only use positives (but that means manually adding half a dozen paths and files) brought the task time down to 200ms.

@UltCombo
Copy link
Contributor

When faced with the same issue, I've moved all source files to a src subdirectory (sibling of node_modules), then you can just use a simple src/**/*.js glob.

@phated
Copy link
Member

phated commented Dec 26, 2014

Don't glob things under node_modules. Remove the **

@yocontra
Copy link
Member

Negation is done post-read right now which is kind of crappy. the node-glob module which we use to find results does not accept multiple globs, so there is no way for us to negate prior to that. If we switch to a new glob module or that module adds support for negation it will speed things up a ton. Really, this is probably one of the biggest bottlenecks in gulp.

@UltCombo
Copy link
Contributor

@contra should we still support regex (post-read) filtering after implementing node-glob's ignore option?

@yocontra
Copy link
Member

@UltCombo Yeah I think so

@UltCombo
Copy link
Contributor

Alright then, I'll have a PR in 30~45mins.

@yocontra
Copy link
Member

@UltCombo Any update on this?

@UltCombo
Copy link
Contributor

@contra #40 (comment)

@thirdcreed
Copy link
Contributor

nearly have a working PR for this, there's one test broken:
'should return a file name stream with dotfiles negated'
and it's timing out at 2000ms.

Any thoughts on what that might be, for some reason the stream is not emitting anything.
https://github.com/thirdcreed/glob-stream/blob/master/index.js

@thirdcreed
Copy link
Contributor

Even when I set the timeout for that test to 15 seconds, it doesn't emit. It runs in like <1 sec when using glob.sync. So I know something's up.

node -p 'require("glob").sync("/home/thirdcreed/Projects/glob-stream/test/fixtures/*swag",{ignore:["/home/thirdcreed/Projects/glob-stream/test/fixtures/**"],dot:true,cwd: "/home/thirdcreed/Projects/glob-stream/test",cwdbase:false,nonull:false})'

That returns [ ], like it should. The ignore actually works, whereas random text in the ignore would allow .swag in, the above command does not; those are the exact parameters being sent into glob.Glob();

The ignore code is 8 days old, so it could be a bug in there, I'm starting to dive in, but any kind of expertise here would be much appreciated. Thanks!

@luengnat
Copy link

any update to this issue? I encounter a similar problem without negation though.

@yocontra
Copy link
Member

I think this is the same as another ticket - the ** is being expanded by recursing the fs (this is how globs work). Using !node_modules instead of !node_modules/** should fix it.

phated pushed a commit that referenced this issue Sep 7, 2016
@phated phated modified the milestone: 6.0 Sep 8, 2016
@phated phated closed this as completed in 1e51a69 Sep 14, 2016
phated pushed a commit that referenced this issue Feb 16, 2017
phated pushed a commit that referenced this issue Feb 16, 2017
phated pushed a commit that referenced this issue Feb 16, 2017
phated pushed a commit that referenced this issue Feb 16, 2017
phated pushed a commit that referenced this issue Feb 20, 2017
phated pushed a commit that referenced this issue Feb 21, 2017
phated pushed a commit that referenced this issue Feb 21, 2017
phated pushed a commit that referenced this issue Feb 21, 2017
phated pushed a commit that referenced this issue Feb 21, 2017
phated pushed a commit that referenced this issue Feb 21, 2017
phated pushed a commit that referenced this issue Feb 21, 2017
phated pushed a commit that referenced this issue Feb 21, 2017
phated pushed a commit that referenced this issue Feb 21, 2017
phated pushed a commit that referenced this issue Feb 21, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants