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

(perf) cache fileExists resolutions #1325

Merged
merged 3 commits into from
Jan 9, 2022

Conversation

dummdidumm
Copy link
Member

@dummdidumm dummdidumm commented Jan 7, 2022

This makes getCompletions perform many times faster, the bigger the project the more it benefits from this cache.

I noticed that there's a lot of time spent in fileExists during analyzing whether or not #1192 does make any difference. I then also found that TypeScript does something similar for its watcher at https://github.com/microsoft/TypeScript/blob/1298f498f4cde7fa53cbf050ebfeefdf85c848ec/src/compiler/watchUtilities.ts#L42 , and that ts.sys.fileExists does not cache anything, so we need to do it ourselves.

Should help #1130 and #1139

I also found out during that that we are likely not properly removing Svelte snapshots when its file is deleted. I'll try this out later on.

Another possible speed-up I found was this method in completions, according to the perf analysis some time is spent in that area in total, probably due to source mapping. We could speed that up in this case because the textEdit ranges are probably all the same, so we can create a cache there. Update: I was wrong, textEdit is rarely filled especially when it's about global completions. But I found that we can compute path once only because pathToUrl is not that fast either.

@jasonlyu123 thoughts?

Simon Holthausen and others added 3 commits January 7, 2022 14:47
This make getCompletions perform many times faster, the bigger the project the more it benefits from this cache
@jasonlyu123
Copy link
Member

Looks good. The only downside I could think of it's where files from outside of the workspace or inside node_module got created or deleted. But it has also been cached in module resolution cache anyway.

@dummdidumm dummdidumm merged commit c951ccd into sveltejs:master Jan 9, 2022
@dummdidumm dummdidumm deleted the file-exists-cache branch January 9, 2022 08:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants