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

Excessive file system IO #1277

Closed
octref opened this issue May 5, 2019 · 20 comments · Fixed by #1283
Closed

Excessive file system IO #1277

octref opened this issue May 5, 2019 · 20 comments · Fixed by #1283
Labels
Milestone

Comments

@octref
Copy link
Member

octref commented May 5, 2019

#784 (comment)

VueServer: 181,000 IO reads, ~10% CPU
TSServer: 1800 reads, ~1% CPU

The root cause seems to be that Vetur loads all js/ts files from workspace, and this will result in TS Server loading all their dependencies:

const files = parsedConfig.fileNames;

@octref octref added the perf label May 5, 2019
@octref octref added this to the April 2019 milestone May 5, 2019
@octref octref mentioned this issue May 5, 2019
9 tasks
@octref
Copy link
Member Author

octref commented May 5, 2019

#1278, VSIX to test: https://github.com/vuejs/vetur/releases/tag/perf

Only one failing test so far, which I believe can be fixed by always including vuex and vue-router's definitions files in the scriptFiles.

@octref
Copy link
Member Author

octref commented May 5, 2019

Hmm, all responses for getScriptSnapshot should be cached as well.

@octref
Copy link
Member Author

octref commented May 5, 2019

Cached all access to node_modules. In macOS Instruments, I see 10x reduced file system access (basically all reads in node_modules are cached).

I updated the pre-release VSIX. @NoahStahl, can you give it a try?

@petternordholm
Copy link

@octref CPU usage is really low with the new version. Using the latest version from scriptFiles branch (latest commit is "Cache file system acccess).

@octref
Copy link
Member Author

octref commented May 6, 2019

Sounds good. Will do a release tomorrow.

@petternordholm
Copy link

In our patched version, we are at the moment using a slightly different approach to minimize IO and speed up completion.

petternordholm@60eef39

The commit above enables us to use a vetur specific tsconfig.json file and in this file, we only include the bare minimum needed for code completion from vue files.

{
  "extends": "./tsconfig",
  "include": [
    "**/*service.ts",
    "**/*utils.ts",
    "**/*helper.ts"
  ]
}

The vetur specific tsconfig enables us to keep existing completion ts files.

All our business logic is accessed via "services" from our vue files. So, the vetur specific configuration file also forces the developer to follow our development guidelines.

@NoahStahl
Copy link

Thanks for the progress @octref. I tried the updated version but it doesn't work at least on Windows, in particular validation is now always skipped. Looks like it is listing files using fsPath, but then using path to compare against. In my environment (Win 10), fsPath and path appear to return different values, so filePaths.includes(filePath) is always false due to slashes. Can it be updated to use normalized paths?

Relevant sections:

export function languageServiceIncludesFile(ls: ts.LanguageService, documentUri: string): boolean {

export function getFileFsPath(documentUri: string): string {

@NoahStahl
Copy link

NoahStahl commented May 7, 2019

After dropping in the slash normalization that typescript uses, perhaps a bit hacky:

605: const filePaths = ls.getProgram()!.getRootFileNames().map(p => p.replace(/\\/g, '/'));

It works. And performance is much improved. Roughly, it looks like about 80% less IO, 40-50% less CPU utilization, and significantly less memory use. Most importantly, error checking and completion is usable again with responses coming back quickly. Awesome!

I still suspect there's more room to optimize, as the usage is still multiples of what .ts files do for same input. But, this change alone will make a huge difference for me. Thanks!

@octref
Copy link
Member Author

octref commented May 7, 2019

That function should have used getFileFsPath:

export function languageServiceIncludesFile(ls: ts.LanguageService, documentUri: string): boolean {
  const filePaths = ls.getProgram()!.getRootFileNames();
  const filePath = getFileFsPath(documentUri);
  return filePaths.includes(filePath);
}

filePaths have the fsPath type of paths, right?

* - Windows
* ```
* > Uri.parse('file:///c%3A/foo/bar.vue').fsPath
* 'c:\\foo\\bar.vue' (\\ escapes to \)
* > Uri.parse('file:///c%3A/foo/bar.vue').path
* '/c:/foo/bar.vue'
* ```

Let me setup #1266 and fix it tomorrow.

@octref
Copy link
Member Author

octref commented May 8, 2019

@NoahStahl

New: (and mostly only fs.stat, not fs.readFile)

newfs

Old:

oldfs

@octref
Copy link
Member Author

octref commented May 8, 2019

And feel free to open new issues if something fails on Windows.

@NoahStahl
Copy link

@octref Yes, 0.20.0 is working well. Thanks again for digging into this!

It still looks like there is quite a bit of CPU and IO used (definitely less than before), but on my high-end desktop machine it cuts through it now without much issue. For people with slower machines they might still notice it, but I'm quite happy with where you've been able to get it with the changes so far.

@petternordholm
Copy link

@octref 0.20 release is performing really well for us, even on our "low-end" laptops. Great work!

@nobelhuang
Copy link

Maybe I should open a new issue, but I feel the discussion here is more relevant. If improper, please let me know.

Thanks for all your tremendous efforts to improve the performance, but by using 0.21.1, I'm still suffering from the slowness of intellisense in my project adopting vue+typescript. I followed the discussion here and tried to do similar profiling to prove my suspect though I am not familiar with profiling. Here is what I found:

  1. Though VLS cached file access, the cache is used within the code flow of synchronizeHostData() via HostCache.createEntry(). While there are still a lot of invocation of synchronizeHostData() which will instantiate a new hostCache every time, there will be a lot of createEntry() calls for each request, and eventually consumes most of cycles.
  2. What makes this even worse is, the cache map localScriptRegionDocuments used for caching vue script portion only contains the portion for the current editing vue file (if I don't switch back and forth among those vue files), it never gets updated to contains other vue files even if they were walked through in getScriptSnapshot(). As a result, the line
    let doc = localScriptRegionDocuments.get(fileName);
    in getScriptKind(), and the line
    const doc = localScriptRegionDocuments.get(fileFsPath);
    in getScriptSnapshot() will not return cached doc object, hence everytime when it comes here, readFile or parseVueScript or refreshAndGet will be called, they are expensive...

In my project, oldProgram has ~1500 source files, while the total cached by serviceHost is ~1000, which means extra ~500 files will be through readFile and parseVueScript, which I don't think it is necessary.

Here is a screenshot of this analysis:
Capture

Profile file:
CPU-20190721T234415.zip

@nobelhuang
Copy link

FYI, just got a chance to make some simple changes based of v0.21.1 as below
nobelhuang@9f38811

Though not sure about if this change has any bad side effects, it does improve the performance significantly in my project. The waiting time for intelliSense popout of any keystroke is reduced from ~10s to ~3s.

@bryding
Copy link

bryding commented Sep 25, 2019

@nobelhuang Thank for this. I am experiencing the exact same performance problems as you in vetur 22. I applied your changes to that script and experienced a large performance boost.

Hopefully we can get a real fix soon for large Vue/typescript projects. The performance basically makes vetur unusable without this hack.

@nobelhuang
Copy link

@bryding Thanks for trying my hack out and proving it is working. You may also want to apply the second change that fixes an issue introduced by the previous hack:
nobelhuang@7676222

Enjoy the performance boost, though now I'm a little bit hesitating to upgrade to any newer versions of Vetur because I have to apply the hack again...

@petternordholm
Copy link

petternordholm commented Sep 26, 2019

@nobelhuang @bryding AFAIK, by only including vue files in VLS, intellisense for "auto imports" will break, except for existing imports in current vue file. The same applies to inclusion of typescript definition files (*.d.ts). For instance errors when accessing this.$router from components in vscode even though the code compile fine.

We use a slightly different approach to minimize the number of files processed by the vls. In our project, we have lots of unit tests referring to vue files. With a vanilla vetur, the vls will parse all test files and, pull in all other vue files resulting in the full project being processed.

Our approach is to add support for vetur specific tsconfig overriding the default config. And in this file, we only include files that should be accessible by our UI components.

petternordholm@7ea5239

Our vetur-tsconfig.json looks like follows:

{
  "extends": "./tsconfig",
  "include": [
    "**/*service.ts",
    "**/*utils.ts",
    "**/*helper.ts",
    "**/*.d.ts"
  ]
}

Our vue files are only allowed to access services, utils, helpers and definition files. This is actually a great improvement since it prevents our developers from pulling in test code in vue files by mistake.

With this fix, code completion is instant (~30ms) even though our project contains ~7000 ts files and 700 vue components.

@octref
Copy link
Member Author

octref commented Sep 27, 2019

I start to think that adding a setting to allow you to pick a different tsconfig makes sense.

One tsconfig is for compilation so it would require all scripts be included, but for VLS it can do without the test scripts.

@nobelhuang Your change is good and I have included it in #1442. It was a mistake of me not to set the local cache after reading from FS. This happens quite often when you open just a Vue file, it loads all dependencies and fallback to FS read without hitting cache.

I'll publish a new version soon so you can try it out.

@nobelhuang
Copy link

@petternordholm maybe I didn't understand you right, or we have different setup about vue+typescript. Without doing anything special, I never see the problem you mentioned about "auto import", I can always auto import symbols from modules that haven't been imported in current vue file. But I do like your idea that by using a different tsconfig to limit the files processed by vetur, it is great for certain scenarios.

@octref thanks for your effort, I would love to try it out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants