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

Pass max_old_space_size to TSServer #33524

Closed
danielscottjames opened this issue Aug 30, 2017 · 7 comments · Fixed by #82630
Closed

Pass max_old_space_size to TSServer #33524

danielscottjames opened this issue Aug 30, 2017 · 7 comments · Fixed by #82630
Assignees
Labels
electron Issues and items related to Electron feature-request Request for new features or functionality typescript Typescript support issues upstream Issue identified as 'upstream' component related (exists outside of VS Code) verification-needed Verification of issue is requested verified Verification succeeded
Milestone

Comments

@danielscottjames
Copy link

danielscottjames commented Aug 30, 2017

Per discussion here: microsoft/TypeScript#18055

Some things I noticed while looking into this:

  • I believe either --max_old_space_size or --max-old-space-size works, v8 args can be dashes or underscores. Underscores work for sure.
  • TSServer appears to have special logic if global.gc is available, so maybe consider adding --expose-gc as well.
  • I believe the maximum settings for 64-bit builds of Node is 4096
  • There's this: max_old_space_size doesn't work when running electron as node electron/electron#9693. Essentially when running VS Code with the ./script/code.sh script, setting these flags in execArgv options in typescriptserviceclient.ts had no effect. I think it'll work if Electron is packaged and ran normally, but I'm not positive.
@vscodebot vscodebot bot added the typescript Typescript support issues label Aug 30, 2017
@mjbvz
Copy link
Contributor

mjbvz commented Sep 5, 2017

I gave this a shot but was unable to get it working even when using a production build of VS Code. Looks like this is blocked by: electron/electron#10413 electron/electron#9693

@mjbvz mjbvz added feature-request Request for new features or functionality upstream Issue identified as 'upstream' component related (exists outside of VS Code) labels Nov 14, 2017
@mjbvz mjbvz added the electron Issues and items related to Electron label Jul 19, 2018
@MLoughry
Copy link
Contributor

@mjbvz I'm running into the same issue. Our project has to tsc with 4GB of memory, and investigations with @amcasey seem to indicate the issue is a lack of memory for tsserver.

It looks like both Electron issues are resolved, so is this issue now unblocked? It would be great if tsserver memory were configurable in workspace settings.

@mjbvz mjbvz added the help wanted Issues identified as good community contribution opportunities label Oct 14, 2019
@mjbvz
Copy link
Contributor

mjbvz commented Oct 14, 2019

@MLoughry Can you try making the change and submit a PR if it works? Here's the code where we spawn the tsserver process:

private getForkOptions(kind: ServerKind) {

@MLoughry
Copy link
Contributor

I'll try to set up an enlistment today/tomorrow and try adding a hard-coded value to see if it works. If it does, do you have a quick pointer on where the guide/code to add a new setting would be?

@amcasey
Copy link
Member

amcasey commented Oct 14, 2019

@mjbvz We did an A/B test when VS made this change and OOMs did go down (as you might hope). When I debugged locally with @MLoughry, we also tried with VS (which passes max_old_space_size) and I don't believe we saw a server crash (though I didn't specifically look for one).

@mjbvz mjbvz added this to the October 2019 milestone Oct 15, 2019
@mjbvz mjbvz added verification-needed Verification of issue is requested and removed help wanted Issues identified as good community contribution opportunities labels Oct 15, 2019
@alexr00 alexr00 added the verification-steps-needed Steps to verify are needed for verification label Oct 29, 2019
@mjbvz
Copy link
Contributor

mjbvz commented Oct 31, 2019

To verify:

  1. Set "typescript.tsserver.maxTsServerMemory": 128
  2. Now open VS Code
  3. This should cause the TSServer to get into a bad state because we cannot load the VS Code project in that amount of memory
  4. Now set it to "typescript.tsserver.maxTsServerMemory": 4000
  5. The project should now load (you may need to restart VS Code)

@mjbvz mjbvz removed the verification-steps-needed Steps to verify are needed for verification label Oct 31, 2019
@connor4312 connor4312 added the verified Verification succeeded label Oct 31, 2019
@connor4312
Copy link
Member

Works as specified and I can see that tsserver is started with --max-old-space-size=4000/128 through htop

@vscodebot vscodebot bot locked and limited conversation to collaborators Nov 29, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
electron Issues and items related to Electron feature-request Request for new features or functionality typescript Typescript support issues upstream Issue identified as 'upstream' component related (exists outside of VS Code) verification-needed Verification of issue is requested verified Verification succeeded
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants