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

Memory issues with import/order autofixer in 2.11.0 #1086

Closed
JakubJagoda opened this issue Apr 19, 2018 · 11 comments
Closed

Memory issues with import/order autofixer in 2.11.0 #1086

JakubJagoda opened this issue Apr 19, 2018 · 11 comments

Comments

@JakubJagoda
Copy link

After upgrading to 2.11.0 my linting process started to fail due to running out of memory. The reason seem to be the autofixer for the import/order rule, because either

  • running linter without --fix option or
  • disabling the import/order rules in .eslintrc

makes it working fine again. This is what I see in the console:

<--- Last few GCs --->

[23224:0000012CB4B8B680]   137777 ms: Mark-sweep 1400.1 (1486.3) -> 1400.1 (1486.3) MB, 1995.8 / 0.0 ms
 allocation failure GC in old space requested
[23224:0000012CB4B8B680]   139784 ms: Mark-sweep 1400.1 (1486.3) -> 1400.1 (1455.3) MB, 2006.7 / 0.0 ms
 last resort
[23224:0000012CB4B8B680]   141845 ms: Mark-sweep 1400.1 (1455.3) -> 1400.1 (1455.3) MB, 2059.5 / 0.0 ms
 last resort


<--- JS stacktrace --->

==== JS stack trace =========================================

Security context: 000000DD8DD1BBE9 <JS Object>
    1: walk(aka walk) [(redacted)\node_modules\typescript-eslint-parser\lib
\node-utils.js:~716] [pc=0000014687C787FF](this=000000DD8DD02241 <undefined>,node=00000374E5FA2321 <a To
kenObject with map 0000033396E2CAF9>)
    2: arguments adaptor frame: 3->1
    3: InnerArrayForEach(aka InnerArrayForEach) [native array.js:~776] [pc=000001468819EC38](this=000000
DD8DD022...

FATAL ERROR: CALL_AND_RETRY_LAST Allocation failed - JavaScript heap out of memory

I was asked to create this issue with a link back to the PR that introduced autofixer for this rule, so here it is:
#908

Just in case, here's my config for the import/order rule:

    "import/order": [
      "error",
      {
        "groups": [
          [
            "builtin",
            "external"
          ],
          [
            "internal",
            "index",
            "sibling",
            "parent"
          ]
        ],
        "newlines-between": "ignore"
      }
    ]
@lukeapage
Copy link
Contributor

We used autofix order on a large codebase (1000s of files) where it had a failure on every file. First run through was very slow as you would expect, then it was fine after fixing. No memory issues.

How many errors do you have if not fixing?
Can you narrow it down to one file?
If you increase the memory limit for the node process does it eventually complete?

@tihonove
Copy link
Contributor

I have assumptions what may cause memory overconsumption, but i want to reproduce the problem.

@JakubJagoda in addition, please provide your node.js version.

@JakubJagoda
Copy link
Author

JakubJagoda commented Apr 20, 2018

@lukeapage I have 0 errors, I discovered the problem by accident after adding a new file. I can't really narrow it down to a single file, because it's not a matter of a single file - whenever there's an incorrect import order in any of the files, the problem happens. When I, according to your advice, had increased the memory limit (using package increase-memory-limit) it ran for like 20 minutes (way longer than previously, when the memory error happened), but didn't complete either, this time with a JS error:

Invalid string length
RangeError: Invalid string length
    at attemptFix ((redacted)\node_modules\eslint\lib\util\source-code-fixer.js:104:22)
    at Function.SourceCodeFixer.applyFixes ((redacted)\node_modules\eslint\lib\util\source-code-fixer.js:123:17)
    at Linter.verifyAndFix ((redacted)\node_modules\eslint\lib\linter.js:1141:43)
    at processText ((redacted)\node_modules\eslint\lib\cli-engine.js:180:32)
    at processFile ((redacted)\node_modules\eslint\lib\cli-engine.js:224:18)
    at fileList.map.fileInfo ((redacted)\node_modules\eslint\lib\cli-engine.js:550:20)
    at Array.map (native)
    at CLIEngine.executeOnFiles ((redacted)\node_modules\eslint\lib\cli-engine.js:519:34)
    at Object.execute ((redacted)\node_modules\eslint\lib\cli.js:189:111)
    at Object.<anonymous> ((redacted)\node_modules\eslint\bin\eslint.js:74:28)

Do you also use typescript-eslint-parser in your setup?

@tihonove I am using Node 8.2.1 on x64 Windows. I wish I could give the project that causes the problem, but unfortunately I can't do that due to legal reasons.

@lukeapage
Copy link
Contributor

It seems like it might be some infinite loop to do with typescript-eslint-parser ?

Can you reproduce with a new file with just 2 import statements? If so you could post the contents of that with changed file paths.. if that doesn’t reproduce, what does?

@mikeallisonJS
Copy link

Also running across this. also using typescript-eslint-parser with a large codebase

justinanastos added a commit to justinanastos/eslint-plugin-import-1086 that referenced this issue Jun 16, 2018
@justinanastos
Copy link
Contributor

@lukeapage Here's a sample repo https://github.com/justinanastos/eslint-plugin-import-1086

@justinanastos
Copy link
Contributor

Also, FWIW, we saw this upgrading from v2.7 to v2.12. I haven't tried to figure out where in between there things went awry.

@justinanastos
Copy link
Contributor

Lastly, I've used eslint-plugin-import for years and love it. Thank you to all the contributors!

@justinanastos
Copy link
Contributor

I believe this is tested and resolved by #1137 that was merged 21 days ago. Can the maintainers please cut a release?

@st-sloth
Copy link
Contributor

As of 2.14.0 released on 2018-08-13 this issue seems to be resolved.

@ljharb
Copy link
Member

ljharb commented Dec 23, 2018

@JakubJagoda is your issue resolved? I’ll be happy to reopen this if not.

@ljharb ljharb closed this as completed Dec 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

7 participants