Skip to content

⚡️ Performance: Don't open client files unnecessarily for project service #8427

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

Closed
4 tasks done
JoshuaKGoldberg opened this issue Feb 10, 2024 · 2 comments · Fixed by #8322
Closed
4 tasks done
Labels
accepting prs Go ahead, send a pull request that resolves this issue enhancement New feature or request locked due to age Please open a new issue if you'd like to say more. See https://typescript-eslint.io/contributing. performance Issues regarding performance
Milestone

Comments

@JoshuaKGoldberg
Copy link
Member

JoshuaKGoldberg commented Feb 10, 2024

Before You File a Proposal Please Confirm You Have Done The Following...

Relevant Package

typescript-estree

My proposal is suitable for this project

  • I believe my proposal would be useful to the broader TypeScript community (meaning it is not a niche proposal).

Description

As reported by @yepitschunked in #8322 (thanks! ❤️‍🔥):

The current implementation opens all files using the project service, even if the file is not opted into type-aware linting (with the project or programs options).
This makes linting slower than necessary, since project for every file will be loaded.

I like the idea of not calling service.openClientFile unnecessarily. useProgramFromProjectService is indeed called always if parseSettings.EXPERIMENTAL_useProjectService is true:

function getProgramAndAST(
parseSettings: ParseSettings,
hasFullTypeInformation: boolean,
): ASTAndProgram {
if (parseSettings.EXPERIMENTAL_projectService) {
const fromProjectService = useProgramFromProjectService(

Logs in #8424 show that opening files with a warmed-up project service to take a few ms per file. That can really add up in projects with many files.

#8322 (comment) shows that avoiding calling service.openClientFile on files that don't ask for type checking can speed up service-type-checked linting to be on par with traditional type-checked linting. ⚡

Additional Info

Note that I'm not yet convinced that #8322's starting approach is the correct one. That approach only calls openClientFile on files with type-checked linting enabled. But some users enable type information on all files despite only using it in some. And, heck, type-checked rules only use type checking conditionally - and try to avoid calling it if they can bail out early. So gating openClientFile on whether a file wants type information can still allow files through the gate unnecessarily.

An alternative possibility might be to avoid opening the client file altogether until getParserServices is called within a lint rule. I think that's the best possible strategy for avoiding unnecessary openClientFile calls. But that might require moving all the type generation logic to getParserServices... which might be good in general? Investigation required.

@JoshuaKGoldberg JoshuaKGoldberg added enhancement New feature or request triage Waiting for team members to take a look performance Issues regarding performance labels Feb 10, 2024
@JoshuaKGoldberg JoshuaKGoldberg added this to the 8.0.0 milestone Feb 10, 2024
@bradzacher bradzacher added accepting prs Go ahead, send a pull request that resolves this issue and removed triage Waiting for team members to take a look labels Feb 20, 2024
@JoshuaKGoldberg
Copy link
Member Author

moving all the type generation logic to getParserServices... which might be good in general? Investigation required.

I spelunked back through the code and re-learned why this not easily doable: we need to use the same ts.SourceFiles (ASTs) from the program.

So, yes, +1 to this just being accepting PRs. As a good-enough-for-now enhancement it makes sense to avoid opening client files if we know they definitely won't need it.

@JoshuaKGoldberg JoshuaKGoldberg changed the title Enhancement: Don't open client files unnecessarily for project service ⚡️ Performance: Don't open client files unnecessarily for project service Jul 19, 2024
Copy link

Closed by #8322.

@github-actions github-actions bot added the locked due to age Please open a new issue if you'd like to say more. See https://typescript-eslint.io/contributing. label Jul 29, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 29, 2024
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 enhancement New feature or request locked due to age Please open a new issue if you'd like to say more. See https://typescript-eslint.io/contributing. performance Issues regarding performance
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants