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

TSServer Reference requests taking a long time #17385

Closed
mjbvz opened this issue Jul 25, 2017 · 13 comments
Closed

TSServer Reference requests taking a long time #17385

mjbvz opened this issue Jul 25, 2017 · 13 comments
Assignees
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue VS Code Tracked There is a VS Code equivalent to this issue

Comments

@mjbvz
Copy link
Contributor

mjbvz commented Jul 25, 2017

From microsoft/vscode#31344

TypeScript Version: 2.4.2

While working in the VSCode codebase, @kieferrm noticed some major slow downs while working in a TS file. It seem the references requests against TSServer are taking up to a second.

Here are his repo steps:

  1. open a VS Code workspace on master, no open editors
  2. search for from 'electron' in all *.ts files
  3. click on the first search result and wait until TS is properly initialized
  4. click on the second search result and select ipcMain in the editor
    -> caret stops blinking, it takes several seconds until the underlying word is highlighted and occurrences are shown in the scrollbar (only happens after the reference code lenses are shown)

And the TSServer log:

https://github.com/Microsoft/vscode/files/1171339/tsserver-log.txt

@mjbvz mjbvz added the VS Code Tracked There is a VS Code equivalent to this issue label Jul 25, 2017
@mjbvz
Copy link
Contributor Author

mjbvz commented Jul 25, 2017

Nothing stood out in the logs besides the long references requests. I believe the root case may be that we are closing and then reopening the vscode project when switching between the search results

@DanielRosenwasser DanielRosenwasser added Bug A bug in TypeScript Needs More Info The issue still hasn't been fully clarified labels Aug 1, 2017
@rbuckton rbuckton assigned ghost Aug 2, 2017
@ghost
Copy link

ghost commented Aug 3, 2017

Looking at the logs, it doesn't look like this has to do-with find-all-references. I see 11::references: elapsed time (in milliseconds) 99.1908 but 2::open: async elapsed time (in milliseconds) 7332.0351.

I can reproduce this by:

  • Enable "workbench.editor.enablePreview": true, in my settings
  • Search for any text
  • Click on first result, hover over something. Takes a while to get info.
  • Click on next result -- this closes the file of the first result and opens a new one.
  • Hover again -- takes a while again.

So it looks like it is because we unload the project when the file closes.
I can also reproduce the same behavior using vscode's built-in file explorer; when I click on a new file, the old file closes, closing the project, then it has to be loaded all over again.
There is a PR at #17269 for improving our watch implementation; perhaps we could consider keeping projects open for 1 second after the last time they were closed.

@kieferrm
Copy link
Member

kieferrm commented Aug 8, 2017

@andy-ms this would be comparable to what we do in VS Code when the last TS file closes. We keep the TS language server around for while to make sure that when a TS file is open briefly afterwards we don't have to pay the startup costs again.

@jrieken
Copy link
Member

jrieken commented Sep 5, 2017

This is a real show stopper and makes TypeScript very hard to deal with. I can reproduce this easily by just opening files in our project. Open vscode.d.ts, open shell.ts, open workbench.ts etc. with each file the project is discarded and then the same project is loaded again

30017133-3896de0c-9158-11e7-91ea-c159d0c087bc

@jrieken
Copy link
Member

jrieken commented Sep 5, 2017

@andy-ms Just to clarify how this affects the users of VS Code: When you don't use tabs and navigate around we dispose/close documents more aggressive and you are very likely to run into this on every editor input change. So, the suggestion from @kieferrm to wait a little before throwing everything away would make a lot of sense.

@mhegazy mhegazy assigned sheetalkamat and unassigned ghost Sep 5, 2017
@mhegazy mhegazy added this to the TypeScript 2.6 milestone Sep 5, 2017
@mhegazy mhegazy removed the Needs More Info The issue still hasn't been fully clarified label Sep 5, 2017
@mhegazy
Copy link
Contributor

mhegazy commented Sep 5, 2017

@sheetalkamat was looking at something similar last week. one option is to keep the project information in memory after the file is closed, and then destroy it on the next file open request. this allows us to reuse it if a file from the same project was opened.

@sheetalkamat
Copy link
Member

#17269 takes care of this by keeping the configured project alive till after next file open request and I am able to navigate/get completions correctly with the changes from that PR

@mjbvz
Copy link
Contributor Author

mjbvz commented Sep 13, 2017

Thank you for taking a look @sheetalkamat!

Is there a targeted fix that could be backported from that PR to TS 2.5.3?

@mhegazy
Copy link
Contributor

mhegazy commented Sep 14, 2017

Is there a targeted fix that could be backported from that PR to TS 2.5.3?

the change is rather big, and complicated. we will not be able to port it to 2.5 at the time being.

@mjbvz
Copy link
Contributor Author

mjbvz commented Oct 3, 2017

@sheetalkamat Do you expect the fix for this to be in by the first 2.6 insiders? Please let us know as soon you there is a build or branch that we can help test

@mhegazy
Copy link
Contributor

mhegazy commented Oct 3, 2017

it should be in typescript@next today, it should be in @2.6.0 and the first @2.6.1-insiders build

@mhegazy
Copy link
Contributor

mhegazy commented Oct 3, 2017

It will be helpful if you can give typescript@next a try.

@mhegazy mhegazy added the Fixed A PR has been merged for this issue label Oct 4, 2017
@mhegazy mhegazy closed this as completed Oct 4, 2017
@mjbvz
Copy link
Contributor Author

mjbvz commented Oct 12, 2017

@mhegazy Yes this seems fixed to me when using typescript@next. Will get some other vscode team members to test this to confirm

@microsoft microsoft locked and limited conversation to collaborators Jun 14, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue VS Code Tracked There is a VS Code equivalent to this issue
Projects
None yet
Development

No branches or pull requests

6 participants