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

Bug: repeated lints in the same process run out of memory #6462

Closed
4 tasks done
sandersn opened this issue Feb 13, 2023 · 1 comment · Fixed by #6476
Closed
4 tasks done

Bug: repeated lints in the same process run out of memory #6462

sandersn opened this issue Feb 13, 2023 · 1 comment · Fixed by #6476
Assignees
Labels
accepting prs Go ahead, send a pull request that resolves this issue bug Something isn't working package: typescript-estree Issues related to @typescript-eslint/typescript-estree

Comments

@sandersn
Copy link
Contributor

sandersn commented Feb 13, 2023

Before You File a Bug Report Please Confirm You Have Done The Following...

  • I have tried restarting my IDE and the issue persists.
  • I have updated to the latest version of the packages.
  • I have searched for related issues and found none that matched my issue.
  • I have read the FAQ and my problem is not listed.

Issue Description

Expected: Repro should finish, printing memory usage that stays roughly about the same.
Actual: Memory usage grows throughout the run until the VM runs out of memory.

This happens even with all .eslintrc.json rules turned off, as long as typescript-eslint is specified as eslint's parser.

Probably related to #1192

Reproduction Repository Link

https://github.com/sandersn/ts-eslint-oom

Repro Steps

  1. clone the repo inside a folder like repros:
$ cd repros
$ git clone git@github.com:sandersn/ts-eslint-oom.git
  1. clone the Definitely Typed repo two levels up from ts-eslint-oom:
$ cd ..
$ git clone git@github.com:DefinitelyTyped/DefinitelyTyped.git --depth 1
$ cd DefinitelyTyped
$ npm i --no-package-lock

At least, I'm pretty sure a shallow clone will work here.
Yes, there is no package-lock.

  1. Back in ts-eslint-oom, run npm ci
  2. Then node index.js

2a. Optionally, delete all the rules in .eslintrc.json, since that shows that the problem is from typescript-eslint and not the rules. You may also need to delete all the override .eslintrc.jsons in subfolders.

ts-eslint-oom loops over every folder in DefinitelyTyped/types and calls eslint on it. The config at DT root specifies typescript-eslint as the parser.

Versions

package version
@typescript-eslint/eslint-plugin 5.52.0
@typescript-eslint/parser 5.52.0
@typescript-eslint/utils 5.52.0
ESLint 8.34.0
node 18 or 16
@bradzacher
Copy link
Member

bradzacher commented Feb 16, 2023

Okay - so I think I understand the problems.

First - the clearProgramCache function you're using is pretty useless. #6476 fixes that. You can work around this by also calling clearWatchCaches until the PR lands.

Second - you're using import { clearProgramCache } from '@typescript-eslint/typescript-estree', however the path you actually want is ../../DefinitelyTyped/node_modules/@typescript-eslint/typescript-estree/index.js.
ESLint resolves things relative to the file and config, not relative to the execution point. Your script was clearing the cache of the local module which was always empty anyways!


Fun little extra thing - you can add process.env.TSESTREE_SINGLE_RUN = 'true' to improve the performance of your script. This will put our parser into "single run" mode - which is what we'll (soon) be using as the default for ESLint CLI runs (but not for custom scripts!)

In "normal" mode we'll create a watch compiler host and builder program, which is super slow for various reasons. We do this so we can efficiently handle the IDE usecase, but it sucks for the CLI / "one and done" usecase.

In "single run" mode we'll directly create a ts.Program - circumventing the watch/builder overhead. Obv this wouldn't work in the IDE, but it works perfectly in the CLI usecase.
But sadly we can't detect custom scripts - only the ESLint binary - hence you can force it using the environment variable

@bradzacher bradzacher added package: typescript-estree Issues related to @typescript-eslint/typescript-estree accepting prs Go ahead, send a pull request that resolves this issue and removed triage Waiting for maintainers to take a look labels Feb 16, 2023
sandersn added a commit to microsoft/DefinitelyTyped-tools that referenced this issue Feb 16, 2023
1. typescript-eslint memory usage workaround provided by @bradzacher in
typescript-eslint/typescript-eslint#6462.
Clear caches after each run so that ts.Program objects aren't retained
forever.
2. typescript-eslint speedup provided the same way. Avoid constructing
some objects that are only needed for editors.
3. Add yet another jsdoc tag to no-redundant-jsdoc2
4. Correct and reduce amount of perf-related printing at end of
ExpectRule. The correction is needed now that ExpectRule saves the
equivalent of --extendedDiagnostics after running; I changed the name of
the properties to match.
sandersn added a commit to microsoft/DefinitelyTyped-tools that referenced this issue Feb 16, 2023
* typescript-eslint workaround + misc fixes

1. typescript-eslint memory usage workaround provided by @bradzacher in
typescript-eslint/typescript-eslint#6462.
Clear caches after each run so that ts.Program objects aren't retained
forever.
2. typescript-eslint speedup provided the same way. Avoid constructing
some objects that are only needed for editors.
3. Add yet another jsdoc tag to no-redundant-jsdoc2
4. Correct and reduce amount of perf-related printing at end of
ExpectRule. The correction is needed now that ExpectRule saves the
equivalent of --extendedDiagnostics after running; I changed the name of
the properties to match.

* fix semicolon lint
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 28, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accepting prs Go ahead, send a pull request that resolves this issue bug Something isn't working package: typescript-estree Issues related to @typescript-eslint/typescript-estree
Projects
None yet
2 participants