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][Windows] 100% cache miss rate for CLI runs #1126

Closed
bradzacher opened this issue Oct 22, 2019 · 2 comments · Fixed by #1128
Closed

[2.5.0][Windows] 100% cache miss rate for CLI runs #1126

bradzacher opened this issue Oct 22, 2019 · 2 comments · Fixed by #1128
Assignees
Labels
bug Something isn't working has pr there is a PR raised to close this package: typescript-estree Issues related to @typescript-eslint/typescript-estree

Comments

@bradzacher
Copy link
Member

Spinning off of #1079

For some reason every file is missing the caches on windows, and never getting found.
I cannot reproduce this on macos, so it makes me think that TS does something different between the two OS's.

Reported by: @doberkofler

@bradzacher bradzacher added bug Something isn't working package: typescript-estree Issues related to @typescript-eslint/typescript-estree labels Oct 22, 2019
@bradzacher bradzacher self-assigned this Oct 22, 2019
@bradzacher bradzacher pinned this issue Oct 22, 2019
@bradzacher bradzacher added the has pr there is a PR raised to close this label Oct 23, 2019
@doberkofler
Copy link
Contributor

Thank you for all your work!

I've just tested the 2.5.1-alpha.2 tag and there is a major improvement in speed.
Unfortunately 2.5.1-alpha.2 still seems to be perform about 30% slower then 2.3.3.

I upload:

  • 2.5.1-alpha.2.txt -> the output of 2.5.1-alpha.2 with the DEBUG=typescript-eslint:* setting
  • perf_2.5.1-alpha.2.txt -> the output of 2.5.1-alpha.2 with the DEBUG=eslint:cli-engine setting
  • perf_2.3.3.txt -> the output of 2.3.3 with the DEBUG=eslint:cli-engine setting

2.5.1-alpha.2.txt
perf_2.3.3.txt
perf_2.5.1-alpha.2.txt

@bradzacher bradzacher unpinned this issue Oct 23, 2019
@bradzacher
Copy link
Member Author

We're making progress.

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 is pretty silly.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working has pr there is a PR raised to close this package: typescript-estree Issues related to @typescript-eslint/typescript-estree
Projects
None yet
2 participants