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

[2.5.0] Lint runs are ~30% slower #1132

Closed
bradzacher opened this issue Oct 23, 2019 · 13 comments · Fixed by #1179
Closed

[2.5.0] Lint runs are ~30% slower #1132

bradzacher opened this issue Oct 23, 2019 · 13 comments · Fixed by #1179

Comments

@bradzacher
Copy link
Member

@bradzacher bradzacher commented Oct 23, 2019

Spinning off of discussion in #1126.
Reported by: @doberkofler

I think that this is because of the change in #1083

There are use cases like Vue, or Markdown, wherein we get provided N distinct content strings for the same filename, where N is the number of <script> tags in the vue file, or the number of code blocks in a markdown file.

We used to not trigger a program sync if we hadn't been asked to parse the file yet, but for these use cases, the file contents TS reads are always different to the file contents eslint gives us, so we have to trigger a file sync on the first parse for a file.

I'll need to look at a solution for this, as invalidating the program for every single file in a CLI run for pure JS/TS codebases (when the file contents from eslint === the file contents in ts) is pretty silly.

@merlinnot

This comment has been minimized.

Copy link

@merlinnot merlinnot commented Oct 23, 2019

I can confirm this issue. For my codebase (Node.js backend app) the linting time jumped in the CI (the exact same VM sizes) from ~1:45 to 2:25 after updating from v2.4.0 to v2.5.0.

@Toxicable

This comment was marked as off-topic.

Copy link

@Toxicable Toxicable commented Oct 23, 2019

Previously it took us about 1:00 to run a lint but after upgrading it now gets to about 2:00-3:00 then runs out of memory.
The files being linted are Angular and Nodejs all TypeScript

@bradzacher

This comment was marked as off-topic.

Copy link
Member Author

@bradzacher bradzacher commented Oct 23, 2019

@Toxicable - there should not have been any discernible changes to the memory usage with the new code, as the new code path should only be used during a long running lint session (i.e. IDE).

Please raise a new issue for out of memory error, and provide as much information as possible, including the output of a DEBUG=typescript-eslint:* yarn lint run.

@Toxicable

This comment has been minimized.

Copy link

@Toxicable Toxicable commented Oct 24, 2019

I havn't been able to reproduce the OOM errors but did get some better timing data in #1134
To summarise we've seen our eslint times go from 56s in 2.3.3 to 4.14m in 2.5.0

@bradzacher bradzacher pinned this issue Oct 24, 2019
@doberkofler

This comment has been minimized.

Copy link
Contributor

@doberkofler doberkofler commented Oct 25, 2019

I just checked out your 2.5.1-alpha.3 and from a performance perspective there does not seem to be any change. Compared to 2.3.3 it is still about 30% slower.

@bradzacher

This comment has been minimized.

Copy link
Member Author

@bradzacher bradzacher commented Oct 25, 2019

the alpha.3 commit was adding support for declare class properties to unblock the prettier release.

I have not looked further into this yet. I'll be sure to explicitly comment back here with updates to save you having to track and test each releases.

@luixo

This comment has been minimized.

Copy link

@luixo luixo commented Oct 25, 2019

Just in case you need more cases, I made measurements on different @typescript-eslint/eslint-plugin versions (also changed @typescript-eslint/eslint-plugin-tslint and @typescript-eslint/parser versions)
On our big (unfortunately, proprietary) codebase:

On 2.1.0: 1m40.139s
On 2.3.0: 1m48.037s
On 2.5.0: 5m3.696s
On 2.5.1-alpha.3: 5m3.067s

If you need more info on our cases - please let me know.

@mpgon

This comment has been minimized.

Copy link

@mpgon mpgon commented Oct 25, 2019

Same in our big (also proprietary) codebase:

2.3.0: 52.68s
2.5.0: 3m30.83s

edit: turns out I had caching enabled for v2.3 and actually it takes the same slow time. I think my problem is unrelated, opened another issue #1140

@Hotell

This comment has been minimized.

Copy link

@Hotell Hotell commented Oct 26, 2019

in our case it is 87% slower than with 2.3...

DATA

  • files count: ts/js/jsx files count : 2760
  • command run: yarn eslint --ext .js,.ts,.tsx --format codeframe our/company/codebase

Before:

  • used typescript-eslint 2.3
  • base tsconfig.json which didn't have all files within includes
  • we used eslint hack createDefaultProgram: true
  • lint time: 169.72s

After:

  • used typescript-eslint 2.5
  • created eslint.tsconfig.json which has all files within includes
  • lint time: 1483.11s

If you need more data please ping me. Thx!

@kelly-tock

This comment has been minimized.

Copy link

@kelly-tock kelly-tock commented Nov 6, 2019

$ npx eslint --cache --cache-location ./.eslint-cache -c .eslintrc.js --no-eslintrc --max-warnings 0 'src/js/**/*.{ts,tsx,js,jsx}' '../shared/src/js/**/*.{ts,tsx,js,jsx}'

2.6.1: 374.75s
2.3.3: 45.13s

similar results. i'm on a very large code base.

@ArkadyDR

This comment has been minimized.

Copy link

@ArkadyDR ArkadyDR commented Nov 7, 2019

Our time has jumped from 50s on version 2.4.0 to 350s on 2.6.1. (with @typescript-eslint/recommended-requiring-type-checking, it's ~3 minutes less without). We have a very large code base.

Edit: With 2.7.0 this is back down to 63s. Thanks!

@yoyo930021

This comment has been minimized.

Copy link
Contributor

@yoyo930021 yoyo930021 commented Nov 7, 2019

If any one can verify #1179 ?

Just do it according to this comment.
#883 (comment)

@doberkofler

This comment has been minimized.

Copy link
Contributor

@doberkofler doberkofler commented Nov 7, 2019

Excellent @yoyo930021 !

I originally reported this SR and have just been testing #1179 with my production repository.

It looks very good and it seems as we are back to the original performance in 2.3.3. I linted my 712 files 3 times with the following averages:

2.3.3: 118 secs
2.4.0: hung
2.5.0: 3016 secs
2.6.1: 161 secs
#1179: 112 secs

@bradzacher bradzacher unpinned this issue Nov 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

10 participants
You can’t perform that action at this time.