-
Notifications
You must be signed in to change notification settings - Fork 12.8k
require('sass.js') slows down TS Server #19458
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
Comments
Here's a tsserver log of this:
The bug seems to be triggered even if I create a
|
The problem does appear to be |
I am not seeing this locally.. i see a slight delay on the first completion, which is something i expect since the file is 3 MBs and we need to parse it. but after that completions almost instantaneously.. am i missing something in my setup? from the log above, i see the time between open command and getting a completion command response is less than 1 sec. |
Hmm, not sure. @octref and I have been able to reproduce it pretty reliably on mac using TS 2.6.1-20171019-insiders. I was using npm 5 and he was on yarn but I'm not sure if that makes any difference I'm also not sure the full delay shows up in the tsserver log. Here's what I see for VS Code's communication with the TSServer for example:
The tsserver log for this same event:
|
I see a 2 sec delay on my machine on the first completion, but later ones seem to be fine:
do you see the same? |
I sometimes see a much larger delay whenever triggering completions:
That was in a new project with only |
The first time the delay is in parsing/binding the file. locally it takes somewhere between 2-4 seconds. once that has happened things should be fast. that is not different from a large project, where building the project takes a few seconds, then things should all be in memory. do you see delays in later completions requests after the first one? |
I still see a significant delay for each subsequent completions request after I edit the file. Looking at the logs again, this delay seems to come when we invoke
The delay seems to happen between |
tested this side by side with @mjbvz and there is definitely something going on mac's that is not happening on windows boxes. need to investigate further on a mac. |
Investigating this shows the issue with the way events are received and processed:
//@ts-check
// key to repro is having ts-check so diagnostics are calculated
const compile = require('sass.js/dist/sass.node');
compile('test.scss', { style: compile.Sass.style.compressed}, res => {
res.
})
Here is whats happening at vscode side and tsserver side according to chronological order:
Resulting in delay in completion because tsserver cannot receive message till its too late. The issue here are multiple:
|
@mjbvz is it possible on vscode side to add cancellation on |
Messaged @mjbvz offline about the microsoft/vscode#49251 not fixing the issue and detailed logs. |
Investigating this with latest code-insiders still repros the issue but the issue now lies with us since we spend time in parsing the sass.async file
This is what I did: After this I completely deleted the line and typed it again followed by new line and completion list invoke. At this point I see long wait for the completion just like you will see right after opening index.js(and not waiting for initial refresh) The cause of this is that, even though the script info for sass.async.js is in memory, its source file that's registered in the documentRegistery is released as soon as it gets removed from all projects, resulting in not being able to reuse the source file. If we can keep source file alive, that will help with the faster program creation. But interesting to note is that sourceFiles document registry is dependent on the compiler options so its not as simple and we need to handle cases when document is part of multiple projects etc well. |
Used branch programInServerPerfLogging to investigate this |
…s check for open files This will ensure that the getErr will be queued in by host and hence would make sure that it is cancellable. Handles one of the scenario delaying completions in #19458
…s check for open files This will ensure that the getErr will be queued in by host and hence would make sure that it is cancellable. Handles one of the scenario delaying completions in #19458
This especially needed if its a js file without the ts-check, the file wont be typechecked in getSemanticDiagnostics Fixes part of #19458
Uh oh!
There was an error while loading. Please reload this page.
TypeScript Version: 2.6.1-insiders
Code
In a new project
With an
index.js
file:Trigger intellisense in the file
Expected behavior:
Should return suggestions pretty quickly.
Actual behavior:
Delay of a few second before suggestions are returned
// @octref
The text was updated successfully, but these errors were encountered: