tsserver crashes with large projects

[jrieken]

I am unsure if this is caused by TS eagerly loading all syntax trees or by the tsserver eagerly loading all symbols (which btw seems crazy to me) but I think the tsserver should check how large a project is (or how much memory is already used) and fail gracefully.

I have a JavaScript project with little code but a lots of dependencies like node_modules and bower_components as well as files that get generated into a temp folder (on server start). I am aware of the exclude-property and the default excludes, but the challenge is to provide guidance to the user to use them. I have no chance to ask tsserver how many files are included by a project because it crashes already with the first open-file request.

A diagnostics message ala too many files (foo/jsconfig.json, files: [...]), would be much appreciated because otherwise we are going to duplicate a lot of logic on our (VS Code) end for such checks (and the server will still crashes).

[mhegazy]

//CC: @billti and @zhengbli

[zhengbli]

A quick solution I can think of:

1. For projects with a jsconfig.json or tsconfig.json file, the tsserver provides an optional parameter called maxFileNumber. Once the parameter is given, the server would not try to open the project if the number of files exceeds that value, and a corresponding error message as you proposed will be returned as well. If the parameter is not given, the server would always try to open the project (even if it may cause crashed)
2. With this change, the editor could pass a default maxFileNumber (maybe 1000) first. If the server returns an error message, then ask the user to consider changing the default excludes, also give the user the option to proceed regardless. If the user chooses to do so, send another command to the server without the parameter to force open the project.

Does that sound reasonable?

[mhegazy]

the file number is not really the correct metric. maybe total file size, as there is a direct correlation between text size and AST size.

[billti]

Agreed. File count doesn't help much if you're loading a 10MB bundled/minified file. Let's chat in the room and see what we can come up with and report back...

[billti]

We had a chat, and he's what we're thinking...

We'll accumulate the size of the source files as we load them, and if this exceeds some TBD limit (e.g. 20MB?), we'll add an error and stop loading files. There will be a compiler options disableSizeLimit to suppress this check in case of false negatives.

Some pertinent details:

• We're not confident we can catch the crash when hit and give a useful error message, so we'd rather set a limit with some headroom that we can detect and operate within.
• We haven't seen users hit this due to loading too many TypeScript files, only including too many JavaScript files (usually build artifacts), so initially we'll only count the size of JavaScript files.
• This will be a "global" error, but we'll associate it with the span of the first line of the file open in the editor to make it visible. (Longer term we should have some type of event for global errors - such as misconfigured tsconfig.json - that the editor always surfaces regardless of the open file).
• We could potentially sum the file size by top level folder and reported the largest in the error message to make it more helpful, e.g. "Total file size limit exceeded. (Folder ./temp contains 19MB). Set the disableSizeLimit option to suppress this check".

Thoughts?

[jrieken]

To reproduce this I have taken derived steps from microsoft/vscode#3170 which is on OS X this:

mkdir test
npm install ember-cli
npm install bower
./node_modules/.bin/ember init
./node_modules/.bin/ember serve

Note that ember serve runs a server and while that is the temp folder is populated with lots of files.

Last create a jsconfig.json file and open VS Code on the folder.

[jrieken]

@billti I like your suggestions.

• With the web worker based solution we had a similar approach of limiting the total size of all projects to ~35MB and giving a warning to the user. It makes sense to check only for JS files as such projects usually bring in many files (temp, modules, et al)
• Wrt to the "global" error I would suggest to set the location to the jsconfig.json file in question. Usually this behaviour only shows when having a project file and a missing/flawed exclude configuration.

Most important for VS Code is that the server does not crash and that the projectInfo route always returns. With that we can already provide good guidance to the user (like 'yeah, configure jsconfig for more fun', 'yeah, exclude tmp, node_modules etc to reduce the amount of files'). To summarise

1. don't crash and have 'projectInfo' always working
2. emit error event in case of too much data/near death
3. make error event rich and suggest excludes based on files

To survive we need #1 and having #2 will make the experience even better. It is OK for us if #3 happens post 1.8.x

[billti]

If the error location is on the jsconfig.json, is the user going to be surfaced this error even if they don't have that file (or the error list) open? Our concern was that they may miss the error (as you'll likely have a ton of other errors if we just stop including files half way through loading).

[zhengbli]

This may not be only memory issue. I've tried the repro @jrieken provides, with the ember server running, the TS language service would crash as mentioned; however after I shutdown the ember server, even with the same flies, the TS language service works normally. @jrieken can you give it a try as well?

[jrieken]

@zhengbli check out the /tmp folder. only when the server runs a large amount of files sit in there. On server-shutdown they are deleted

[zhengbli]

I realized why I wasn't seeing file size changes between when server is running and when the server is not. There are many symlink files generated in the tmp folder, therefore when I directly select the folder and check the total size, I was getting the size of the symlink files, not the targeted files they link to.

That also partially contributes to the crash, because when sys.readDirectory is using fs.statSync to get the file information; while for symlink files without a valid target file (not exist), fs.statSync would throw an exception.

[zhengbli]

I submitted a fix at #7486. Tested using the project mentioned above, and I do already see the "configure excludes" tips using the latest VSCode:

Great job @jrieken ! Though I am curious what matrices are you guys using to determine if a project is too large?

[jrieken]

Thanks. We only use the number of files and show the warning the project has more than 500 files. We know it's not precise but it can be easily swapped with the trigger/diag event from this issue

[zhengbli]

Status update:
With #7513, now both the compiler and the server have a upper limit of 20M for the total size of non-TS files in the program; when the limit was reached, an error like the following one will be generated:

Too many JavaScript files in the project. Consider specifying the 'exclude' setting in project configuration to limit included source folders. The likely folder to exclude is 'c:/users/lizhe/documents/playground/testlargeproject/tmp/'. To disable the project size limit, set the 'disableSizeLimit' compiler option to 'true'.

We also added a compiler option called disableSizeLimit to disable such check.

However, as we still process up to 20M of files, it could take a long time before the error was generated. Currently with the repro sample you gave, it took around 40 seconds to get intelligent pop-up (when the serve is running) on my i-5 machine (around 18 seconds to parse jsconfig.json and obtain the file list for the project). I'm looking into how this time could be shortened.

To summarize:
What have been done:

• Prevent the server / compiler from crashing on large project with numerous non-TS files

What's next:

• Reduce the time before the "Too many JavaScript" file was generated for tsserver
• Surface the diagnostics to VSCode via a new event

I'll open a new issue for tracking this task items.

[jrieken]

Thanks. I can verify and that server doesn't crash anymore tho it is very slow... For now we will stay with our >500 files condition but are eagerly awaiting the new event.

[mhegazy]

Moving it to 2.0 to see if we can do better on the recovery time.