perf: prevent requests from being sent to the server when outside Angular context #1180
Conversation
atscott
force-pushed
the
shortcircuitclientfix
branch
from
e723fa4 to
fea3d90
Compare
atscott
added
extension
kyliau
changed the title
| * Use of this source code is governed by an MIT-style license that can be | ||
| * found in the LICENSE file at https://angular.io/license | ||
| */ | ||
| import * as ts from 'typescript'; |
There was a problem hiding this comment.
Since this requires parsing the whole typescript library, could you please do a quick profiling to see if there's any significant difference in startup time before and after this change?
The startup time can be retrieved by running the command Show running extensions, or if you want in-depth report you can run the command Startup performance
There was a problem hiding this comment.
Yes, the startup time before appears to be 200-300ms and the startup time now looks like 500-600ms. So 2-3x slower.
There was a problem hiding this comment.
I've done some experimentation based on the regex in the syntaxes and it seems like something that would be feasible without introducing a dependency on typescript: https://github.com/atscott/vscode-ng-language-service/blob/c1b86ff50dd9b4e8c9b29c4e71127483ee26b0e6/client/src/embedded_support.ts
There was a problem hiding this comment.
As discussed, we'll proceed with this change, but note the regression in activation time in the change log.
atscott
force-pushed
the
shortcircuitclientfix
branch
from
fea3d90 to
743f109
Compare
| * Use of this source code is governed by an MIT-style license that can be | ||
| * found in the LICENSE file at https://angular.io/license | ||
| */ | ||
| import * as ts from 'typescript'; |
There was a problem hiding this comment.
As discussed, we'll proceed with this change, but note the regression in activation time in the change log.
atscott
force-pushed
the
shortcircuitclientfix
branch
2 times, most recently
from
4966017 to
008fdd4
Compare
…gular context The Angular Language Service extension of vscode has several pieces to it: * The client, which communicates directly with vscode * The server, which handles LSP requests over an lsp connection * @angular/language-service which provides concrete answers to LS queries We added an optimization the @angular/language-service which exits early when a request is outside the Angular context. This prevents unnecessary Angular analysis of a file when we know from the start that there are no Angular-specific results at a location. This commit provides an additional optimization by adding a similar preventing short-circuit from the client side. This prevents requests from even being sent to the server when we know there is no Angular information at a location. This optimization is a necessary addition because the server can be blocked from processing requests if another one is taking a while to respond. We found that requests for diagnostics, which are necessary when opening a new file, block other server requests from being processed. This commit would prevent that block from happening by never making the request to the server in the first place. fixes angular#1176
atscott
force-pushed
the
shortcircuitclientfix
branch
from
008fdd4 to
f7836eb
Compare
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
…gular context
The Angular Language Service extension of vscode has several pieces to
it:
We added an optimization the @angular/language-service which exits early
when a request is outside the Angular context. This prevents unnecessary
Angular analysis of a file when we know from the start that there are no
Angular-specific results at a location.
This commit provides an additional optimization by adding a similar preventing
short-circuit from the client side. This prevents requests from even
being sent to the server when we know there is no Angular information at
a location. This optimization is a necessary addition because the server
can be blocked from processing requests if another one is taking a while
to respond. We found that requests for diagnostics, which are necessary
when opening a new file, block other server requests from being
processed. This commit would prevent that block from happening by never
making the request to the server in the first place.
fixes #1176