(perf) improvements around diagnostics and completions (#1022)

- Better debounce of diagnostics to not call them more often than needed
- Try to reuse old completions and handle the special case of import completions

#1017

Author: dummdidumm

packages/language-server/src/plugins/typescript/LSAndTSDocResolver.ts

@@ -28,7 +28,7 @@ export class LSAndTSDocResolver {
     ) {
         const handleDocumentChange = (document: Document) => {
             // This refreshes the document in the ts language service
-            this.getLSAndTSDoc(document);
+            this.getSnapshot(document);
         };
         docManager.on(
             'documentChange',

packages/language-server/src/plugins/typescript/features/CompletionProvider.ts

@@ -31,14 +31,20 @@ import {
     scriptElementKindToCompletionItemKind
 } from '../utils';
 import { getJsDocTemplateCompletion } from './getJsDocTemplateCompletion';
-import { getComponentAtPosition } from './utils';
+import { getComponentAtPosition, isPartOfImportStatement } from './utils';
 
 export interface CompletionEntryWithIdentifer extends ts.CompletionEntry, TextDocumentIdentifier {
     position: Position;
 }
 
 type validTriggerCharacter = '.' | '"' | "'" | '`' | '/' | '@' | '<' | '#';
 
+type LastCompletion = {
+    key: string;
+    position: Position;
+    completionList: AppCompletionList<CompletionEntryWithIdentifer> | null;
+};
+
 export class CompletionsProviderImpl implements CompletionsProvider<CompletionEntryWithIdentifer> {
     constructor(private readonly lsAndTsDocResolver: LSAndTSDocResolver) {}
 
@@ -48,6 +54,10 @@ export class CompletionsProviderImpl implements CompletionsProvider<CompletionEn
      * Therefore, only use the characters the typescript compiler treats as valid.
      */
     private readonly validTriggerCharacters = ['.', '"', "'", '`', '/', '@', '<', '#'] as const;
+    /**
+     * For performance reasons, try to reuse the last completion if possible.
+     */
+    private lastCompletion?: LastCompletion;
 
     private isValidTriggerCharacter(
         character: string | undefined
@@ -94,6 +104,21 @@ export class CompletionsProviderImpl implements CompletionsProvider<CompletionEn
             return null;
         }
 
+        if (
+            this.canReuseLastCompletion(
+                this.lastCompletion,
+                triggerKind,
+                triggerCharacter,
+                document,
+                position
+            )
+        ) {
+            this.lastCompletion.position = position;
+            return this.lastCompletion.completionList;
+        } else {
+            this.lastCompletion = undefined;
+        }
+
         const fragment = await tsDoc.getFragment();
         if (!fragment.isInGenerated(position)) {
             return null;
@@ -143,7 +168,28 @@ export class CompletionsProviderImpl implements CompletionsProvider<CompletionEn
             .map((comp) => mapCompletionItemToOriginal(fragment, comp))
             .concat(eventCompletions);
 
-        return CompletionList.create(completionItems, !!tsDoc.parserError);
+        const completionList = CompletionList.create(completionItems, !!tsDoc.parserError);
+        this.lastCompletion = { key: document.getFilePath() || '', position, completionList };
+        return completionList;
+    }
+
+    private canReuseLastCompletion(
+        lastCompletion: LastCompletion | undefined,
+        triggerKind: number | undefined,
+        triggerCharacter: string | undefined,
+        document: Document,
+        position: Position
+    ): lastCompletion is LastCompletion {
+        return (
+            !!lastCompletion &&
+            lastCompletion.key === document.getFilePath() &&
+            lastCompletion.position.line === position.line &&
+            Math.abs(lastCompletion.position.character - position.character) < 2 &&
+            (triggerKind === CompletionTriggerKind.TriggerForIncompleteCompletions ||
+                // Special case: `.` is a trigger character, but inside import path completions
+                // it shouldn't trigger another completion because we can reuse the old one
+                (triggerCharacter === '.' && isPartOfImportStatement(document.getText(), position)))
+        );
     }
 
     private getExistingImports(document: Document) {

packages/language-server/src/plugins/typescript/features/utils.ts

@@ -1,6 +1,11 @@
 import ts from 'typescript';
 import { Position } from 'vscode-languageserver';
-import { Document, getNodeIfIsInComponentStartTag, isInTag } from '../../../lib/documents';
+import {
+    Document,
+    getLineAtPosition,
+    getNodeIfIsInComponentStartTag,
+    isInTag
+} from '../../../lib/documents';
 import {
     DocumentSnapshot,
     SnapshotFragment,
@@ -74,6 +79,11 @@ export function isNoTextSpanInGeneratedCode(text: string, span: ts.TextSpan) {
     return !isInGeneratedCode(text, span.start, span.start + span.length);
 }
 
+export function isPartOfImportStatement(text: string, position: Position): boolean {
+    const line = getLineAtPosition(position, text);
+    return /\s*from\s+["'][^"']*/.test(line.substr(0, position.character));
+}
+
 export class SnapshotFragmentMap {
     private map = new Map<string, { fragment: SnapshotFragment; snapshot: DocumentSnapshot }>();
     constructor(private resolver: LSAndTSDocResolver) {}

packages/language-server/src/server.ts

@@ -1,4 +1,3 @@
-import _ from 'lodash';
 import {
     ApplyWorkspaceEditParams,
     ApplyWorkspaceEditRequest,
@@ -34,7 +33,7 @@ import {
     OnWatchFileChangesPara,
     LSAndTSDocResolver
 } from './plugins';
-import { isNotNullOrUndefined, urlToPath } from './utils';
+import { debounceThrottle, isNotNullOrUndefined, urlToPath } from './utils';
 import { FallbackWatcher } from './lib/FallbackWatcher';
 
 namespace TagCloseRequest {
@@ -319,7 +318,7 @@ export function startServer(options?: LSOptions) {
         pluginHost.getDiagnostics.bind(pluginHost)
     );
 
-    const updateAllDiagnostics = _.debounce(() => diagnosticsManager.updateAll(), 1000);
+    const updateAllDiagnostics = debounceThrottle(() => diagnosticsManager.updateAll(), 1000);
 
     connection.onDidChangeWatchedFiles(onDidChangeWatchedFiles);
     function onDidChangeWatchedFiles(para: DidChangeWatchedFilesParams) {
@@ -358,7 +357,7 @@ export function startServer(options?: LSOptions) {
 
     docManager.on(
         'documentChange',
-        _.debounce(async (document: Document) => diagnosticsManager.update(document), 500)
+        debounceThrottle(async (document: Document) => diagnosticsManager.update(document), 1000)
     );
     docManager.on('documentClose', (document: Document) =>
         diagnosticsManager.removeDiagnostics(document)

packages/language-server/src/utils.ts

@@ -85,6 +85,35 @@ export function debounceSameArg<T>(
     };
 }
 
+/**
+ * Debounces a function but also waits at minimum the specified number of miliseconds until
+ * the next invocation. This avoids needless calls when a synchronous call (like diagnostics)
+ * took too long and the whole timeout of the next call was eaten up already.
+ *
+ * @param fn The function with it's argument
+ * @param miliseconds Number of miliseconds to debounce/throttle
+ */
+export function debounceThrottle<T extends (...args: any) => void>(fn: T, miliseconds: number): T {
+    let timeout: any;
+    let lastInvocation = Date.now() - miliseconds;
+
+    function maybeCall(...args: any) {
+        clearTimeout(timeout);
+
+        timeout = setTimeout(() => {
+            if (Date.now() - lastInvocation < miliseconds) {
+                maybeCall(...args);
+                return;
+            }
+
+            fn(...args);
+            lastInvocation = Date.now();
+        }, miliseconds);
+    }
+
+    return maybeCall as any;
+}
+
 /**
  * Like str.lastIndexOf, but for regular expressions. Note that you need to provide the g-flag to your RegExp!
  */