Skip to content

Commit

Permalink
perf: filter completion list
Browse files Browse the repository at this point in the history
  • Loading branch information
@johnsoncodehk
johnsoncodehk committed Feb 19, 2023
1 parent 574ee22 commit fd0e24c
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 5 deletions.
37 changes: 32 additions & 5 deletions packages/language-server/src/common/features/languageFeatures.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,18 @@ import { AutoInsertRequest, FindFileReferenceRequest, ShowReferencesNotification
import { CancellationTokenHost } from '../cancellationPipe';
import type { Workspaces } from '../workspaces';
import * as shared from '@volar/shared';
import { RuntimeEnvironment } from '../../types';
import { RuntimeEnvironment, LanguageServerInitializationOptions } from '../../types';
import { createDocuments } from '../documents';

export function register(
connection: vscode.Connection,
projects: Workspaces,
initParams: vscode.InitializeParams,
initOptions: LanguageServerInitializationOptions,
cancelHost: CancellationTokenHost,
semanticTokensLegend: vscode.SemanticTokensLegend,
runtime: RuntimeEnvironment,
documents: ReturnType<typeof createDocuments>,
) {

let lastCompleteUri: string;
Expand Down Expand Up @@ -71,15 +74,39 @@ export function register(
return worker(params.textDocument.uri, token, async service => {
lastCompleteUri = params.textDocument.uri;
lastCompleteLs = service;
const document = documents.data.uriGet(params.textDocument.uri)?.getDocument();
const list = await service.doComplete(
params.textDocument.uri,
params.position,
params.context,
);
if (list) {
for (const item of list.items) {
fixTextEdit(item);
}
for (const item of list.items) {
fixTextEdit(item);
}
if (!initOptions.fullCompletionList && document) {

This comment has been minimized.

Sign in to view

Copy link
@rchl

rchl Feb 19, 2023

Contributor

Why is it implemented as an option? Are there caveats to using it? It would be good to explain as I don't know as an user what effect will it have. Will I miss some completions?

Also, is the issue mostly caused by Typescript? If so, shouldn't those be filtered out at the place where those are generated?

This comment has been minimized.

Sign in to view

Copy link
@rchl

rchl Feb 19, 2023

Contributor

Also, this seems to filter out all completions that have no range. Is that on purpose?

This comment has been minimized.

Sign in to view

Copy link
@rchl

rchl Feb 19, 2023

Contributor

Thirdly, how much smaller the resulting payload is with and without this option in the best scenario? And is it because of the part that handles filterText or due to excluding all completions without range?

This comment has been minimized.

Sign in to view

Copy link
@johnsoncodehk

johnsoncodehk Feb 19, 2023 •
edited
Loading

Author Member

Different IDEs may have different triggering timings for auto-complete. For example, if some IDEs do not re-trigger auto-complete when deleting characters, the result will be incomplete, so whether to enable it depends on the behavior of the client. I haven't found any problems with this in vscode for now.

CSS and HTML completions have the same problem, both return ~800 results.

This comment has been minimized.

Sign in to view

Copy link
@rchl

rchl Feb 19, 2023 •
edited
Loading

Contributor

Wouldn't it make sense to disable it by default then and optimize only in editors where the behavior was verified to be safe?

EDIT: By "disable" I meant not limit anything be default.

This comment has been minimized.

Sign in to view

Copy link
@rchl

rchl Feb 19, 2023

Contributor

CSS and HTML completions have the same problem, both return ~800 results.

That doesn't sound like much.

Typescript can return 50MB payload (~71K items) when aws-sdk is installed in the project.

This comment has been minimized.

Sign in to view

Copy link
@johnsoncodehk

johnsoncodehk Feb 19, 2023

Author Member

Also, this seems to filter out all completions that have no range. Is that on purpose?

No. 😅

Thirdly, how much smaller the resulting payload is with and without this option in the best scenario?

It all depends on how many characters have been entered so far, the number is usually reduced to less than 10 when more than 5 characters are entered.

Wouldn't it make sense to disable it by default then and optimize only in editors where the behavior was verified to be safe?

Considering the severity of the hazards to the IDE in extreme cases whether it is enabled or not, I will disable fullCompletionList by default.

In addition, users can find that the completion items are missing, but they may not be able to find that the IDE is often blocked because of big completion list.

This comment has been minimized.

Sign in to view

Copy link
@rchl

rchl Feb 19, 2023 •
edited
Loading

Contributor

No. 😅

Can you then verify whether this logic still makes a significant difference after fixing it?

Considering the severity of the hazards to the IDE in extreme cases whether it is enabled or not, I will disable fullCompletionList by default.

I'm not sure I agree with that...

n addition, users can find that the completion items are missing, but they may not be able to find that the IDE is often blocked because of big completion list.

Different editors have different thresholds where performance gets affected. Some maybe will start showing issues with 5MB payload while other at 30MB. So it's not quite right to assume that "too big" is the same for all.

This comment has been minimized.

Sign in to view

Copy link
@johnsoncodehk

johnsoncodehk Feb 19, 2023

Author Member

Can you then verify whether this logic still makes a significant difference after fixing it?

Actually no difference. The current @volar-plugins/* should all provide edit range.

I will do the worst, and sublimelsp/LSP-volar enabled it by default is totally okay, but maybe I should also ping other related IDE plugin maintainers to note this option.

This comment has been minimized.

Sign in to view

Copy link
@rchl

rchl Feb 19, 2023 •
edited
Loading

Contributor

Actually no difference. The current @volar-plugins/* should all provide edit range.

I don't believe so. I see plenty of completions without range (EDIT: textEdit more specifically) when requesting in a vue file using Volar 1.1.2.

Tried within :style="|" and {{ | }}.

<template>
  <div :style="|" class="x">
    {{ | }}
  </div>
</template>
list.items = list.items.filter(item => {
const range = item.textEdit ? (
vscode.InsertReplaceEdit.is(item.textEdit) ? item.textEdit.replace :
item.textEdit.range
) : list.itemDefaults?.editRange ? (
vscode.Range.is(list.itemDefaults.editRange) ? list.itemDefaults.editRange :
list.itemDefaults.editRange.replace
) : undefined;
if (range) {
const sourceText = document.getText(range).toLowerCase();
if (sourceText.trim()) {
let filterText = (item.filterText ?? item.label).toLowerCase();
for (const char of sourceText) {
const index = filterText.indexOf(char);
if (index === -1) {
return false;
}
filterText = filterText.slice(index + 1);
}
}
return true;
}
});
}
return list;
});
Expand Down
2 changes: 2 additions & 0 deletions packages/language-server/src/common/server.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -146,9 +146,11 @@ export function startCommonLanguageServer(context: ServerContext) {
context.connection,
projects,
initParams,
options,
cancelTokenHost,
getSemanticTokensLegend(),
context.runtimeEnv,
documents,
);

for (const plugin of plugins) {
Expand Down
6 changes: 6 additions & 0 deletions packages/language-server/src/types.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,12 @@ export interface LanguageServerInitializationOptions {
serverMode?: ServerMode;
diagnosticModel?: DiagnosticModel;
textDocumentSync?: vscode.TextDocumentSyncKind | number;
/**
* For better JSON parsing performance language server will filter CompletionList.
*
* Enable this option if you want to get complete CompletionList in language client.
*/
fullCompletionList?: boolean;
// for resolve https://github.com/sublimelsp/LSP-volar/issues/114
ignoreTriggerCharacters?: string[];
/**
Expand Down

0 comments on commit fd0e24c

Please sign in to comment.