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

perf: cancel diagnostics #2216

Merged
merged 3 commits into from
Nov 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 46 additions & 6 deletions packages/language-server/src/lib/DiagnosticsManager.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,18 @@
import { _Connection, TextDocumentIdentifier, Diagnostic } from 'vscode-languageserver';
import {
Connection,
TextDocumentIdentifier,
Diagnostic,
CancellationTokenSource,
CancellationToken
} from 'vscode-languageserver';
import { DocumentManager, Document } from './documents';
import { debounceThrottle } from '../utils';

export type SendDiagnostics = _Connection['sendDiagnostics'];
export type GetDiagnostics = (doc: TextDocumentIdentifier) => Thenable<Diagnostic[]>;
export type SendDiagnostics = Connection['sendDiagnostics'];
export type GetDiagnostics = (
doc: TextDocumentIdentifier,
cancellationToken?: CancellationToken
) => Thenable<Diagnostic[]>;

export class DiagnosticsManager {
constructor(
Expand All @@ -13,6 +22,7 @@ export class DiagnosticsManager {
) {}

private pendingUpdates = new Set<Document>();
private cancellationTokens = new Map<string, { cancel: () => void }>();

private updateAll() {
this.docManager.getAllOpenedByClient().forEach((doc) => {
Expand All @@ -21,14 +31,43 @@ export class DiagnosticsManager {
this.pendingUpdates.clear();
}

scheduleUpdateAll = debounceThrottle(() => this.updateAll(), 1000);
scheduleUpdateAll() {
this.cancellationTokens.forEach((token) => token.cancel());
this.cancellationTokens.clear();
this.pendingUpdates.clear();
this.debouncedUpdateAll();
}

private debouncedUpdateAll = debounceThrottle(() => this.updateAll(), 1000);

private async update(document: Document) {
const diagnostics = await this.getDiagnostics({ uri: document.getURL() });
const uri = document.getURL();
this.cancelStarted(uri);

const tokenSource = new CancellationTokenSource();
this.cancellationTokens.set(uri, tokenSource);

const diagnostics = await this.getDiagnostics(
{ uri: document.getURL() },
tokenSource.token
);
this.sendDiagnostics({
uri: document.getURL(),
diagnostics
});

tokenSource.dispose();

if (this.cancellationTokens.get(uri) === tokenSource) {
this.cancellationTokens.delete(uri);
}
}

cancelStarted(uri: string) {
const started = this.cancellationTokens.get(uri);
if (started) {
started.cancel();
}
}

removeDiagnostics(document: Document) {
Expand All @@ -44,6 +83,7 @@ export class DiagnosticsManager {
return;
}

this.cancelStarted(document.getURL());
this.pendingUpdates.add(document);
this.scheduleBatchUpdate();
}
Expand All @@ -53,5 +93,5 @@ export class DiagnosticsManager {
this.update(doc);
});
this.pendingUpdates.clear();
}, 750);
}, 700);
}
7 changes: 5 additions & 2 deletions packages/language-server/src/plugins/PluginHost.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,10 @@ export class PluginHost implements LSProvider, OnWatchFileChanges {
this.deferredRequests = {};
}

async getDiagnostics(textDocument: TextDocumentIdentifier): Promise<Diagnostic[]> {
async getDiagnostics(
textDocument: TextDocumentIdentifier,
cancellationToken?: CancellationToken
): Promise<Diagnostic[]> {
const document = this.getDocument(textDocument.uri);

if (
Expand All @@ -96,7 +99,7 @@ export class PluginHost implements LSProvider, OnWatchFileChanges {
return flatten(
await this.execute<Diagnostic[]>(
'getDiagnostics',
[document],
[document, cancellationToken],
ExecuteMode.Collect,
'high'
)
Expand Down
8 changes: 6 additions & 2 deletions packages/language-server/src/plugins/svelte/SveltePlugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,15 +49,19 @@ export class SveltePlugin

constructor(private configManager: LSConfigManager) {}

async getDiagnostics(document: Document): Promise<Diagnostic[]> {
async getDiagnostics(
document: Document,
cancellationToken?: CancellationToken
): Promise<Diagnostic[]> {
if (!this.featureEnabled('diagnostics') || !this.configManager.getIsTrusted()) {
return [];
}

return getDiagnostics(
document,
await this.getSvelteDoc(document),
this.configManager.getConfig().svelte.compilerWarnings
this.configManager.getConfig().svelte.compilerWarnings,
cancellationToken
);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
// @ts-ignore
import { Warning } from 'svelte/types/compiler/interfaces';
import { Diagnostic, DiagnosticSeverity, Position, Range } from 'vscode-languageserver';
import {
CancellationToken,
Diagnostic,
DiagnosticSeverity,
Position,
Range
} from 'vscode-languageserver';
import {
Document,
isInTag,
Expand All @@ -19,15 +25,20 @@ import { SvelteDocument, TranspileErrorSource } from '../SvelteDocument';
export async function getDiagnostics(
document: Document,
svelteDoc: SvelteDocument,
settings: CompilerWarningsSettings
settings: CompilerWarningsSettings,
cancellationToken?: CancellationToken
): Promise<Diagnostic[]> {
const config = await svelteDoc.config;
if (config?.loadConfigError) {
return getConfigLoadErrorDiagnostics(config.loadConfigError);
}

if (cancellationToken?.isCancellationRequested) {
return [];
}

try {
return await tryGetDiagnostics(document, svelteDoc, settings);
return await tryGetDiagnostics(document, svelteDoc, settings, cancellationToken);
} catch (error) {
return getPreprocessErrorDiagnostics(document, error);
}
Expand All @@ -39,12 +50,19 @@ export async function getDiagnostics(
async function tryGetDiagnostics(
document: Document,
svelteDoc: SvelteDocument,
settings: CompilerWarningsSettings
settings: CompilerWarningsSettings,
cancellationToken: CancellationToken | undefined
): Promise<Diagnostic[]> {
const transpiled = await svelteDoc.getTranspiled();
if (cancellationToken?.isCancellationRequested) {
return [];
}

try {
const res = await svelteDoc.getCompiled();
if (cancellationToken?.isCancellationRequested) {
return [];
}
return (((res.stats as any)?.warnings || res.warnings || []) as Warning[])
.filter((warning) => settings[warning.code] !== 'ignore')
.map((warning) => {
Expand All @@ -65,7 +83,7 @@ async function tryGetDiagnostics(
.map((diag) => adjustMappings(diag, document))
.filter((diag) => isNoFalsePositive(diag, document));
} catch (err) {
return (await createParserErrorDiagnostic(err, document))
return createParserErrorDiagnostic(err, document)
.map((diag) => mapObjWithRangeToOriginal(transpiled, diag))
.map((diag) => adjustMappings(diag, document));
}
Expand All @@ -74,7 +92,7 @@ async function tryGetDiagnostics(
/**
* Try to infer a nice diagnostic error message from the compilation error.
*/
async function createParserErrorDiagnostic(error: any, document: Document) {
function createParserErrorDiagnostic(error: any, document: Document) {
const start = error.start || { line: 1, column: 0 };
const end = error.end || start;
const diagnostic: Diagnostic = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,11 +80,20 @@ export class DiagnosticsProviderImpl implements DiagnosticsProvider {
];
}

let diagnostics: ts.Diagnostic[] = [
...lang.getSyntacticDiagnostics(tsDoc.filePath),
...lang.getSuggestionDiagnostics(tsDoc.filePath),
...lang.getSemanticDiagnostics(tsDoc.filePath)
];
let diagnostics: ts.Diagnostic[] = lang.getSyntacticDiagnostics(tsDoc.filePath);
const checkers = [lang.getSuggestionDiagnostics, lang.getSemanticDiagnostics];

for (const checker of checkers) {
if (cancellationToken) {
// wait a bit so the event loop can check for cancellation
// or let completion go first
await new Promise((resolve) => setTimeout(resolve, 10));
if (cancellationToken.isCancellationRequested) {
return [];
}
}
diagnostics.push(...checker.call(lang, tsDoc.filePath));
}

const additionalStoreDiagnostics: ts.Diagnostic[] = [];
const notGenerated = isNotGenerated(tsDoc.getFullText());
Expand Down
3 changes: 2 additions & 1 deletion packages/language-server/src/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -353,6 +353,7 @@ export function startServer(options?: LSOptions) {

connection.onDidCloseTextDocument((evt) => docManager.closeDocument(evt.textDocument.uri));
connection.onDidChangeTextDocument((evt) => {
diagnosticsManager.cancelStarted(evt.textDocument.uri);
docManager.updateDocument(evt.textDocument, evt.contentChanges);
pluginHost.didUpdateDocument();
});
Expand Down Expand Up @@ -468,7 +469,7 @@ export function startServer(options?: LSOptions) {
refreshCrossFilesSemanticFeatures();
}

connection.onDidSaveTextDocument(diagnosticsManager.scheduleUpdateAll);
connection.onDidSaveTextDocument(diagnosticsManager.scheduleUpdateAll.bind(diagnosticsManager));
connection.onNotification('$/onDidChangeTsOrJsFile', async (e: any) => {
const path = urlToPath(e.uri);
if (path) {
Expand Down
2 changes: 1 addition & 1 deletion packages/language-server/test/plugins/PluginHost.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ describe('PluginHost', () => {
await pluginHost.getDiagnostics(textDocument);

sinon.assert.calledOnce(plugin.getDiagnostics);
sinon.assert.calledWithExactly(plugin.getDiagnostics, document);
sinon.assert.calledWithExactly(plugin.getDiagnostics, document, undefined);
});

it('executes doHover on plugins', async () => {
Expand Down