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

Too slow on large projects #234

Closed
pixelass opened this issue Jun 22, 2017 · 28 comments
Closed

Too slow on large projects #234

pixelass opened this issue Jun 22, 2017 · 28 comments

Comments

@pixelass
Copy link

pixelass commented Jun 22, 2017

Description

I am using XO and Ava in a large project. Both packages are too slow to be used. I never actually tried to let them finish. I usually give up after 10+ minutes.
I first thought this might be related to globby: sindresorhus/globby#43

As mentioned in that issue I created a test source that reproduces this issue.
The glob targets a folder that exists only once.

I opened a separate issue in Ava. avajs/ava#1418

Test Source

Error Message & Stack Trace

nothing happens

Config

https://github.com/pixelass/ava-xo-test/blob/3f9f228f279804d8634a367f5458f69ffbc7d253/package.json#L35-L41

{
  "xo": {
    "ignores": [
      "node_modules/*"
    ],
    "rules": {
      "import/no-unassigned-import": 0
    }
  }
}

Command-Line Arguments

https://github.com/pixelass/ava-xo-test/blob/3f9f228f279804d8634a367f5458f69ffbc7d253/package.json#L10

{
"scripts": {
    "xo": "xo --quiet './**/*.js'"
  },
}
xo --quiet './**/*.js

Relevant Links

Minimal reproduction:
https://github.com/pixelass/ava-xo-test/

Environment

OS X 10.12.5 (16F73)  
Node 6.11.0  

Requested log:

Platform: darwin 16.6.0
xo: 0.18.2
node: 6.11.0
npm: 3.10.10
@Qix-
Copy link

Qix- commented Jun 22, 2017

Just chiming in - XO already ignores node_modules/.

@pixelass
Copy link
Author

pixelass commented Jun 22, 2017

@Qix- thx. I'm not sure why we have it listed. Probably c&p from other plugins. I was aware that it is ignored though.

I created the test-repo similar to our setup.

@pixelass
Copy link
Author

I switched to eslint using the XO shared config. It takes about 7 seconds to lint all files and the config can be read by the variety of editors our developers use.

So the issue is not related to eslint itself. Hope this helps.

@SamVerschueren
Copy link
Contributor

Is it also slow when running it through the command line? There's an issue with the XO linter in VS Code on large projects. So my question, is this really a XO issue or rather a plugin issue?

@pixelass
Copy link
Author

I first experienced this issue from the command line.
I myself don't use linters in my editor but rather use npm scripts. (See original issue description)

All issues around editors came from developers in my team. I'm just making sure everybody is happy.

sindresorhus added a commit that referenced this issue Jun 22, 2017
@sindresorhus
Copy link
Member

I discovered the main culprit. We didn't define quiet as a boolean, which made the glob pattern an argument to --quiet and not an actuall pattern, which resulted in XO using its default pattern, which is **/*.

Try changing it to:

- "xo": "xo --quiet './**/*.js'",
+ "xo": "xo './**/*.js' --quiet",

@sindresorhus
Copy link
Member

sindresorhus commented Jun 22, 2017

It still takes 18 seconds just to glob the 25891 .js files out of 116329 files on my machine. Although totally unrealistic example, I think it should be faster.

The gitignore logic for example is inefficient.

@pixelass
Copy link
Author

I just added eslint with the xo shared config as a comparison.
I removed the quiet flag and the config of xo.

For me xo is still very slow in that test project (>40 sec) while eslint works as expected (~12 sec) which isn't fast either but acceptable.

The changes can be viewed on the master branch

@pixelass
Copy link
Author

This example is taken from a java/hybris project with multiple brands. The frontend is not separated from the backend. I tried to make it as similar as possible. Only a few files should be globbed now since I forgot the folder name this-is-it. So the glob might have been unrealistic but the the example is not ;)

@sindresorhus
Copy link
Member

sindresorhus commented Jun 23, 2017

For me xo is still very slow in that test project (>40 sec) while eslint works as expected (~12 sec) which isn't fast either but acceptable.

Try deleting ~/.xo-cache and run again. That might improve it a little bit. Since the cache was filled up with all the files, not just .js files.

@sindresorhus
Copy link
Member

As I thought, inefficient gitignore handling. This line takes more than 6 seconds.

@sindresorhus
Copy link
Member

XO will always be slower than ESLint + eslint-config-xo though, as we include many more external plugins here that are not included in eslint-config-xo, but we will try to fix the biggest performance bottlenecks.

sindresorhus added a commit that referenced this issue Jun 23, 2017
@pixelass
Copy link
Author

After deleting the cache:

Eslint: 14s
XO: 59s

@marionebl
Copy link
Contributor

To sum this up there is a bundle of performance related issues we could tackle to improve user experience:

  • Reading gitignores is synchronous

  • Wasted effort on reading irrelevant gitignores

  • Wasted effort on globbing gitignored directory trees

I do not have a coherent idea on how to tackle points 2 and 3 without reimplementing the gitignore logic of ignore in globby. Thoughts @schnittstabil @sindresorhus?

@sindresorhus
Copy link
Member

I do not have a coherent idea on how to tackle points 2 and 3 without reimplementing the gitignore logic of ignore in globby.

Why would we have to reimplement the gitignore logic?

@marionebl
Copy link
Contributor

marionebl commented Jun 26, 2017

Why would we have to reimplement the gitignore logic?

Taking @schnittstabil 's comment about glob into account there are two ways to approach this:

  1. Land a PR on glob to support Function, on .ignore, use that with ignore in globby. No reimplementation necessary here.

  2. Make the translation from.gitignore to negative glob patterns and/or .ignore patterns work correctly. That boils down to reimplementation of ignore to some extent.

I only thought about scenario 2 in my previous comment.

@niftylettuce
Copy link

Any update here? I still find xo to be incredibly slow while linting 1000-2000 LOC and eslint is fast.

@niftylettuce
Copy link

xo albeit nice has greatly hindered my development speed, I'd gladly pay a bug bounty 🐛 💰

@OmgImAlexis
Copy link
Contributor

@niftylettuce looks like sindresorhus/globby#68 has fixed or at least helped with this.

@pvdlg
Copy link
Contributor

pvdlg commented Mar 8, 2018

@pixelass @SamVerschueren @OmgImAlexis @niftylettuce we released a new version of XO with improved performances regarding globbing. That would be great to have your feedback, as you experienced performance issues on your projects before.
Is the new version improve the situation? Do you still experience unreasonable slowness?

@niftylettuce
Copy link

Still incredibly poor performance, even on a file only 600 LOC

@pvdlg
Copy link
Contributor

pvdlg commented Sep 23, 2018

What “incredibly poor performance” means? Please provide numbers.
Can you also provide the file in question and your XO configuration? Otherwise there is absolutely nothing that we can do.

@Qix-
Copy link

Qix- commented Sep 23, 2018

Also, keep in mind Node has a bootup time of about 600ms at the least, which is astronomically high especially if you're running it over and over in a loop.

There's nothing the XO team can do about that either.

@OmgImAlexis
Copy link
Contributor

@Qix- eslint with the same config runs in a LOT less time. It's not node that's the issue. It's very likely still something like globby as it the slow down seem to point to before. Let's hope sindresorhus/globby#68 helps with this.

@pvdlg have a look at the other issues @niftylettuce and I have commented on regarding speed issues with xo; We've both had issues on multiple different projects in multiple configs. If we could reduce it to a single issue we'd all be happy. 😞

Ref: #212

@pvdlg
Copy link
Contributor

pvdlg commented Sep 24, 2018

@OmgImAlexis a lot has been done to improve performances since you opened #212, including changes in globby.

@niftylettuce, @OmgImAlexis any comments need to include:

  • Actual time measured to lint (as in an actual number)
  • The files linted, or a link or to it, or even better a link to the repo (as in "the actual file so I can reproduce", not "600 LOC" or some vague info)
  • If possible, a comparison with ESlint with the same rules

Every comments with vague things like "incredibly poor performance” or "multiple different projects in multiple configs" or references to 2 years old things are not helpful and doesn't allow us to analyze anything.

@niftylettuce
Copy link

I have provided a reproducible test case #212 (comment). The issue is when you have a lot of conditionals, the process for xo will never end as a result.

@Qix-
Copy link

Qix- commented Sep 28, 2018

  1 warning
  295 errors
node_modules/.bin/xo  1.03s user 0.14s system 107% cpu 1.091 total

1 second. Make sure your lockfiles are blown away - they cause lots of issues such as outdated modules.

EDIT Just saw #212 (comment) - glad that was the case :)

@niftylettuce
Copy link

Please close issue

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

No branches or pull requests

8 participants