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

fix potential performance issue caused by language server settings being re-loaded on every keystroke when doing inlay hints #922

Merged
merged 1 commit into from
Dec 3, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ import { TextRange } from '../common/textRange';
import { convertRangeToTextRange } from '../common/positionUtils';
import { Uri } from '../common/uri/uri';
import { ParseFileResults } from '../parser/parser';
import { InlayHintSettings } from '../common/languageServerInterface';
import { transformTypeForEnumMember } from './enums';
import { InlayHintSettings } from '../workspaceFactory';

export type TypeInlayHintsItemType = {
inlayHintType: 'variable' | 'functionReturn' | 'parameter' | 'generic';
Expand Down
13 changes: 1 addition & 12 deletions packages/pyright-internal/src/common/languageServerInterface.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import { MarkupKind } from 'vscode-languageserver';
import { MaxAnalysisTime } from '../analyzer/program';
import { BackgroundAnalysisBase } from '../backgroundAnalysisBase';
import { Workspace } from '../workspaceFactory';
import { InlayHintSettings, Workspace } from '../workspaceFactory';
import { CancellationProvider } from './cancellationUtils';
import { DiagnosticBooleanOverridesMap, DiagnosticSeverityOverridesMap } from './commandLineOptions';
import { SignatureDisplayType } from './configOptions';
Expand All @@ -21,17 +21,6 @@ import { ServiceProvider } from './serviceProvider';
import { Uri } from './uri/uri';
import { FileDiagnostics } from './diagnosticSink';

export interface InlayHintSettings {
/**
* pylance's version of this option supports 3 settings: `"all" | "partial" | "off"`. `"all"` shows inlay hints
* for positional only arguments which i think is dumb so we don't support it
*/
callArgumentNames: boolean;
functionReturnTypes: boolean;
variableTypes: boolean;
genericTypes: boolean;
}

export interface ServerSettings {
venvPath?: Uri | undefined;
pythonPath?: Uri | undefined;
Expand Down
13 changes: 6 additions & 7 deletions packages/pyright-internal/src/languageServerBase.ts
Original file line number Diff line number Diff line change
Expand Up @@ -363,6 +363,7 @@ export abstract class LanguageServerBase implements LanguageServerInterface, Dis
workspace.disableLanguageServices = !!serverSettings.disableLanguageServices;
workspace.disableTaggedHints = !!serverSettings.disableTaggedHints;
workspace.disableOrganizeImports = !!serverSettings.disableOrganizeImports;
workspace.inlayHints = serverSettings.inlayHints;
} finally {
// Don't use workspace.isInitialized directly since it might have been
// reset due to pending config change event.
Expand Down Expand Up @@ -1005,21 +1006,19 @@ export abstract class LanguageServerBase implements LanguageServerInterface, Dis
protected async onInlayHints(params: InlayHintParams, token: CancellationToken): Promise<InlayHint[] | null> {
const uri = Uri.parse(params.textDocument.uri, this.serviceProvider);
const workspace = await this.getWorkspaceForFile(uri);

const inlayHintSettings = (await this.getSettings(workspace)).inlayHints;
if (
workspace.disableLanguageServices ||
// don't bother creating the inlay hint provider if all the inlay hint settings are off
(inlayHintSettings && !Object.values(inlayHintSettings).some((value) => value))
(workspace.inlayHints && !Object.values(workspace.inlayHints).some((value) => value))
) {
return null;
}
return workspace.service.run((program) => {
return new InlayHintsProvider(program, uri, params.range, {
callArgumentNames: inlayHintSettings?.callArgumentNames ?? true,
functionReturnTypes: inlayHintSettings?.functionReturnTypes ?? true,
variableTypes: inlayHintSettings?.variableTypes ?? true,
genericTypes: inlayHintSettings?.genericTypes ?? false,
callArgumentNames: workspace.inlayHints?.callArgumentNames ?? true,
functionReturnTypes: workspace.inlayHints?.functionReturnTypes ?? true,
variableTypes: workspace.inlayHints?.variableTypes ?? true,
genericTypes: workspace.inlayHints?.genericTypes ?? false,
}).onInlayHints();
}, token);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { convertOffsetToPosition } from '../common/positionUtils';
import { TypeInlayHintsWalker } from '../analyzer/typeInlayHintsWalker';
import { Uri } from '../common/uri/uri';
import { Range } from 'vscode-languageserver-types';
import { InlayHintSettings } from '../common/languageServerInterface';
import { InlayHintSettings } from '../workspaceFactory';

export class InlayHintsProvider {
private readonly _walker: TypeInlayHintsWalker;
Expand Down
2 changes: 1 addition & 1 deletion packages/pyright-internal/src/tests/testUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ import { SemanticTokenItem, SemanticTokensWalker } from '../analyzer/semanticTok
import { TypeInlayHintsItemType, TypeInlayHintsWalker } from '../analyzer/typeInlayHintsWalker';
import { Range } from 'vscode-languageserver-types';
import { ServiceProvider } from '../common/serviceProvider';
import { InlayHintSettings } from '../common/languageServerInterface';
import { InlayHintSettings } from '../workspaceFactory';

// This is a bit gross, but it's necessary to allow the fallback typeshed
// directory to be located when running within the jest environment. This
Expand Down
12 changes: 12 additions & 0 deletions packages/pyright-internal/src/workspaceFactory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,17 @@ import { createDeferred } from './common/deferred';
import { ServiceProvider } from './common/serviceProvider';
import { Uri } from './common/uri/uri';

export interface InlayHintSettings {
/**
* pylance's version of this option supports 3 settings: `"all" | "partial" | "off"`. `"all"` shows inlay hints
* for positional only arguments which i think is dumb so we don't support it
*/
callArgumentNames: boolean;
functionReturnTypes: boolean;
variableTypes: boolean;
genericTypes: boolean;
}

let WorkspaceFactoryIdCounter = 0;

export enum WellKnownWorkspaceKinds {
Expand Down Expand Up @@ -87,6 +98,7 @@ export interface Workspace extends WorkspaceFolder {
disableWorkspaceSymbol: boolean;
isInitialized: InitStatus;
searchPathsToWatch: Uri[];
inlayHints?: InlayHintSettings | undefined;
}

export interface NormalWorkspace extends Workspace {
Expand Down
Loading