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.

Sign up for GitHub

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

Jump to bottom

no-multiple-empty-lines performance issue (hang?) #7893

Closed
undeadcat opened this issue Jan 10, 2017 · 3 comments · Fixed by singapore/lint-condo#218 or renovatebot/renovate#63 · May be fixed by iamhunter/teammates#4
Closed

no-multiple-empty-lines performance issue (hang?) #7893

undeadcat opened this issue Jan 10, 2017 · 3 comments · Fixed by singapore/lint-condo#218 or renovatebot/renovate#63 · May be fixed by iamhunter/teammates#4
Assignees
@not-an-aardvark
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion bug ESLint is working incorrectly rule Relates to ESLint's core rules

Comments

@undeadcat
Copy link

Tell us about your environment

  • ESLint Version:
    3.13.1
  • Node Version:
    6.9.2
  • npm Version:
    3.10.9

What parser (default, Babel-ESLint, etc.) are you using?
default
Please show your full configuration:

{
  "rules": {
    "no-multiple-empty-lines": [2, { "max": 1 }]  
  }
}

What did you do
Linted a large file: app.txt
(I have changed the extension to .txt in order to attach it to this issue. It is obviously a js file and you should change the extension to the original .js).

What did you expect to happen?
Expected ESLint to produce output in a reasonable amount of time.

What actually happened? Please include the actual, raw output from ESLint.
ESLint did not produce any output after 10 minutes while consuming 100% of 1 CPU core.


The file is a file generated by WebPack. I am aware that linting generated files makes no sense and have added the generated files dir to .eslintignore so that performance on large files per se is not a problem for me.

The reason I would like to see some fix (a timeout? if improving the performance is not possible?) is that I was using ESLint from inside an IDE that became unresponsive while waiting for linting to complete. It took me a while to figure out what file and combination of rules was causing the behavior and it would be great if others would not have to spend more time only to figure out that they forgot to exclude their build output folder.
@eslintbot eslintbot added the triage An ESLint team member will look at this issue soon label Jan 10, 2017
@not-an-aardvark
Copy link
Member

Thanks for the report. This is a bit confusing, because we just fixed an issue in 3.13.0 (#7803) where no-multiple-empty-lines would hang on large files. However, it seems like you're using 3.13.1, which would already have gotten that fix.

Just to make sure, could you run eslint -v to double-check that you have 3.13.1 installed? If you have 3.13.1 in your package.json file but haven't run npm install recently, it's possible that the installed version of ESLint doesn't match the version in your package.json file.

@not-an-aardvark not-an-aardvark added bug ESLint is working incorrectly evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion rule Relates to ESLint's core rules and removed triage An ESLint team member will look at this issue soon labels Jan 10, 2017
@undeadcat
Copy link
Author

I've looked at the issue before reporting and specifically checked, and confirmed now that eslint-v returns v3.13.1. There is no eslint installed globally or anywhere else, so I can't think of other things that could be wrong with my install.

Have you tried reproducing it?

@not-an-aardvark
Copy link
Member

Not yet, I'm planning to look at it a bit later today. Thanks for confirming the ESLint version.

@not-an-aardvark not-an-aardvark added accepted There is consensus among the team that this change meets the criteria for inclusion and removed evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion labels Jan 11, 2017
@not-an-aardvark not-an-aardvark self-assigned this Jan 11, 2017
not-an-aardvark added a commit that referenced this issue Jan 11, 2017
@not-an-aardvark
Fix: catastrophic backtracking in astUtils linebreak regex (fixes #7893)
90f0651 
This fixes an issue where `astUtils.getLocationFromRangeIndex` and `astUtils.getRangeIndexFromLocation` were using a regular expression susceptible to catastrophic backtracking. The match would take quadratic time in the length of the last line of the file. Since the file in #7893 contains a 1.5 million character source map URL on the last line, rules like `no-multiple-empty-lines` would hang when using ast-utils to split the file into lines.

This issue only applies to files without trailing newlines, and is only noticable when the last line of the file contains more than 30000 characters or so. Since only a few rules use these `astUtils` functions, this would only appear when either `no-useless-escape` or `no-multiple-empty-lines` reports an error for the file.

Simplified example: Node 7.4.0 hangs when evaluating this expression.

```js
/[^\n]*\n/.test('A'.repeat(1000000))
```
not-an-aardvark added a commit that referenced this issue Jan 11, 2017
@not-an-aardvark
Fix: catastrophic backtracking in astUtils linebreak regex (fixes #7893)
2614433 
This fixes an issue where `astUtils.getLocationFromRangeIndex` and `astUtils.getRangeIndexFromLocation` were using a regular expression susceptible to catastrophic backtracking. The match would take quadratic time in the length of the last line of the file. Since the file in #7893 contains a 1.5 million character source map URL on the last line, rules like `no-multiple-empty-lines` would hang when using ast-utils to split the file into lines.

This issue only applies to files without trailing newlines, and is only noticable when the last line of the file contains more than 30000 characters or so. Since only a few rules use these `astUtils` functions, this would only appear when either `no-useless-escape` or `no-multiple-empty-lines` reports an error for the file.

Simplified example: Node 7.4.0 hangs when evaluating this expression.

```js
/[^\n]*\n/.test('A'.repeat(1000000))
```
@not-an-aardvark not-an-aardvark mentioned this issue Jan 11, 2017
Merged
@nzakas nzakas closed this as completed in 427543a Jan 12, 2017
This was referenced Jan 20, 2017
Merged
Merged
@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Feb 6, 2018
@eslint-deprecated eslint-deprecated bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Feb 6, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@not-an-aardvark not-an-aardvark

Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion bug ESLint is working incorrectly rule Relates to ESLint's core rules
Projects
None yet
Milestone
No milestone
3 participants
@undeadcat @not-an-aardvark @eslintbot