Extremely slow with antivirus

[dizlexik]

The company I work for requires that I run some horrible (but somehow popular) antivirus software on my PC which makes my eslint . take nearly 30 seconds to run on a very small project.

I did a little investigating and all of that slowness occurs in glob-util.js in the call to glob.sync(pattern, globOptions). If I modify globOptions to include ignore: 'node_modules/**' my linting drops to one second.

It appears that crawling node_modules causes my antivirus to kick in for each file, and I'm assuming I can't be the only one with this problem. In any case, filtering out node_modules before the glob.sync call should certainly speed things up to some degree for everyone.

I understand that there is some complex logic involved with ignoring and unignoring paths so I see why all node_modules paths are included at this point in the code, but is there a chance this could be handled in some other way that would short-circuit the needless crawling? It would be a massive 99% speed increase, at least for me and anyone else in my position.

[eslintbot]

Hi @dizlexik, thanks for the issue. It looks like there's not enough information for us to know how to help you. If you're reporting a bug, please be sure to include:

1. The version of ESLint you are using (run eslint -v)
2. What you did (the source code and ESLint configuration)
3. The actual ESLint output complete with numbers
4. What you expected to happen instead

Requesting a new rule? Please see Proposing a New Rule for instructions.

If it's something else, please just provide as much additional information as possible. Thanks!

[alberto]

node_modules is ignored by default. Please provide the information requested

[dizlexik]

I understand that node_modules is ignored by default, but glob.sync(pattern, globOptions) happens before ignoredPaths rules are applied. My question is whether or not node_modules can be excluded from the glob crawling instead of each file under node_modules being filtered out one-by-one after the fact.

I'm using 2.5.0, but this was also an issue in 2.4.0.

[ilyavolodin]

@alberto node_modules are ignored by default from linting, but I don't think we ignore them by default for globing. That happens before we even get to ignore options. I think this would be a good enhancement to add, but at the same time, we need to make it in such a way that users would be able to un-ignore node_modules, and I don't know if we have enough information to do that at the point of globbing.

[dizlexik]

That's exactly right. Thanks for summing it up clearly!

And yes, the un-ignoring is the real complication here. It seems to me that the information on whether or not node_modules or some file(s) under it are to be un-ignored is present at the point of globbing, but I can imagine it might be quite a complicated bit of potentially error-prone code to jam in there to fix this :/

Hopefully someone can find the time to tackle this because I currently have a build process that runs across a number of sub-modules that each perform linting individually and this would reduce the build time from minutes to seconds. I've patched eslint on our private npm server with this hack for now since we don't do any un-ignoring, but an official fix would be awesome.

[alberto]

Sorry, I didn't read close enough.

[nzakas]

@dizlexik since you've already done a bit of research here, would you be willing to submit a pull request? We are pretty strapped with some high priority issues right now, so it's unlikely that we will find the time to dig into this anytime soon.

[dizlexik]

I'd love to help out, but after reviewing the code in this area I don't see any easy way of determining whether or not any node_modules paths are unignored, so it looks to me like some changes will be required at a lower level.

I see that ingored-files.js utilizes the ignore package, which doesn't appear to expose anything useful regarding this so some manual testing of each ignore pattern may be required to resolve this. Sounds like a mini-project that I don't have the time for right now, I'm sorry :/

[nzakas]

Unfortunately, it's unlikely to get done unless we get a volunteer. I'll flag it as such to see if we get any takers.

[maxtruxa]

From the looks of it this could be a huge performance improvement. Provided nobody else wants to solve it in the near future, I'll take a look at it until the end of the week.

[alberto]

Note .eslintignore uses ignore pattern syntax, while glob uses minimatch.

[holm]

Could node_modules just not always be ignored? It makes a huge difference to the speed of eslint, and I have a hard time seeing the use-case for linting node_modules.

[nzakas]

@holm sorry, I can't quite parse your meaning ("just not always"). We ignore node_modules by default already.

Ultimately, we need some real data here to move forward. By data, I mean we need a benchmark that shows what we're doing now is slower than something we could potentially be doing.

@maxtruxa are you still planning on investigating this?

[alberto]

@nzakas I think he means in glob walking

[holm]

Sorry for the unclear comment. Yes, I mean in the glob walking. In large projects, with many dependencies, node_modules can contain a lot of files. One project I am currently working on has 43,432 files in node_modules.

[alberto]

@holm here is an example of someone needing to lint a folder inside node_modules

[nzakas]

@kaelzhang node-glob does have an ignore option that lets you specify directories it should not traverse. It's buried in the list of options here: https://github.com/isaacs/node-glob#options

[kaelzhang]

@nzakas Yes, I've read the source code of glob and minimatch ( although izs's work is not that readable, honestly, not meant to offence :p )

But options.ignore will be mapped into an array of minimatch instances. I thought it it not that logical to use gitignore patterns as minimatch patterns.

[nzakas]

@kaelzhang you don't have to. My suggestion was only to do that in the couple cases that we know we don't have to search the default directories, so just use "Node_modules" and "bower_components" for options.ignore. There's no need to do anything more complicated.

[ilyavolodin]

@nzakas I'm not sure I agree. We just merged a change that allows users to un-ignore directories ignored by default. If we were to hard-code node_modules and bower_components as ignored by node_glob, that would invalidate previous change.
Unfortunately, I have no good suggestions on how to do this without hard-coding directories.

[platinumazure]

@ilyavolodin One of these modules allows us to work with strings only without going into the filesystem, right? If so, is it possible to run node_modules/, bower_components/, and the user's ignore patterns through that logic to see if anything in node_modules or bower_components needs to be traversed at all? And if so, we can specify skip-traverse for those directories?

If that works, maybe we can find a way to generalize that to directories initially ignored in .eslintignore?

[Zarel]

One first step would be to hard-code all the ignored directories if there are no unignored directories.

[nzakas]

@ilyavolodin please read my previous comment. What I'm suggesting would not interfere with re-including.

[alberto]

I'm working on a simple fix for this just for the default ignores, as mentioned here

[Zarel]

This doesn't solve my own problem, which is that the huge ignored directory is logs/, not node_modules/...

[ilyavolodin]

@Zarel We are aware that the fixed that we just merged doesn't solve all of the problems. It's a temporary solution that should help most of the users. We will be working on the more permanent solution.

[Zarel]

It's just that this issue was closed. Should it be reopened, or is there a different issue that tracks that problem?

[platinumazure]

@Zarel Very sorry for the delayed response. We should do further work under a new issue, perhaps titled something like "Skip glob traversal of ignored directories (besides node_modules and bower_components)". Thanks!

[Zarel]

Sure. Are you planning on opening the issue or should I?

[rocketmonkeys]

@platinumazure @ilyavolodin @Zarel - I've made a follow-up issue to improve on this fix here: #6710 . I also have a very rough PR (that I'm sure isn't good yet) that I can link to on that issue.