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.

Sign up for GitHub

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: auto import cache #2237

Merged
merged 15 commits into from
Jan 11, 2024
Merged

perf: auto import cache #2237

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
6752119
wip auto-import-cache
jasonlyu123 Dec 21, 2023
9fceacb
ts 5.3 compatibility
jasonlyu123 Dec 22, 2023
2857bd5
patching the cache methods ourself for more control
jasonlyu123 Dec 28, 2023
86e2d80
format
jasonlyu123 Dec 28, 2023
b6e5e45
fix package.json import
jasonlyu123 Dec 29, 2023
e5791f1
monitor package.json from module resolution as well, like before
jasonlyu123 Dec 29, 2023
bfa7eb4
let ts cache IncompleteCompletions
jasonlyu123 Dec 29, 2023
ee8e224
most module specifier is resolved in getCompletionsAtPosition now
jasonlyu123 Dec 29, 2023
4539429
another cache
jasonlyu123 Jan 4, 2024
29d34db
format
jasonlyu123 Jan 4, 2024
0329297
cleanup
jasonlyu123 Jan 4, 2024
9244da7
revert incomplete completion cache
jasonlyu123 Jan 11, 2024
527be3e
clarify createLanguageService passed into typescript-auto-import-cache
jasonlyu123 Jan 11, 2024
9b012d3
handle delete file and new export
jasonlyu123 Jan 11, 2024
4620a28
slight optimization. no cache to clear for the first run
jasonlyu123 Jan 11, 2024
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
1 change: 1 addition & 0 deletions packages/language-server/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
"svelte-preprocess": "~5.1.0",
"svelte2tsx": "workspace:~",
"typescript": "^5.3.2",
"typescript-auto-import-cache": "^0.3.2",
"vscode-css-languageservice": "~6.2.10",
"vscode-html-languageservice": "~5.1.1",
"vscode-languageserver": "8.0.2",
Expand Down
1 change: 1 addition & 0 deletions packages/language-server/src/ls-config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -439,6 +439,7 @@ export class LSConfigManager {
includeCompletionsWithObjectLiteralMethodSnippets:
config.suggest?.objectLiteralMethodSnippets?.enabled ?? true,
preferTypeOnlyAutoImports: config.preferences?.preferTypeOnlyAutoImports,
allowIncompleteCompletions: true,

includeInlayEnumMemberValueHints: inlayHints?.enumMemberValues?.enabled,
includeInlayFunctionLikeReturnTypeHints: inlayHints?.functionLikeReturnTypes?.enabled,
Expand Down
130 changes: 112 additions & 18 deletions packages/language-server/src/plugins/typescript/LSAndTSDocResolver.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { dirname } from 'path';
import { dirname, join } from 'path';
import ts from 'typescript';
import { TextDocumentContentChangeEvent } from 'vscode-languageserver';
import { Document, DocumentManager } from '../../lib/documents';
Expand All @@ -19,8 +19,10 @@ import {
LanguageServiceContainer,
LanguageServiceDocumentContext
} from './service';
import { createProjectService } from './serviceCache';
import { GlobalSnapshotsManager, SnapshotManager } from './SnapshotManager';
import { isSubPath } from './utils';
import { FileMap } from '../../lib/documents/fileCollection';

interface LSAndTSDocResolverOptions {
notifyExceedSizeLimit?: () => void;
Expand Down Expand Up @@ -71,6 +73,40 @@ export class LSAndTSDocResolver {
this.getCanonicalFileName = createGetCanonicalFileName(
(options?.tsSystem ?? ts.sys).useCaseSensitiveFileNames
);

this.tsSystem = this.wrapWithPackageJsonMonitoring(this.options?.tsSystem ?? ts.sys);
this.globalSnapshotsManager = new GlobalSnapshotsManager(this.tsSystem);
this.userPreferencesAccessor = { preferences: this.getTsUserPreferences() };
const projectService = createProjectService(this.tsSystem, this.userPreferencesAccessor);

configManager.onChange(() => {
const newPreferences = this.getTsUserPreferences();
const autoImportConfigChanged =
newPreferences.includePackageJsonAutoImports !==
this.userPreferencesAccessor.preferences.includePackageJsonAutoImports;

this.userPreferencesAccessor.preferences = newPreferences;

if (autoImportConfigChanged) {
forAllServices((service) => {
service.onAutoImportProviderSettingsChanged();
});
}
});

this.watchers = new FileMap(this.tsSystem.useCaseSensitiveFileNames);
this.lsDocumentContext = {
ambientTypesSource: this.options?.isSvelteCheck ? 'svelte-check' : 'svelte2tsx',
createDocument: this.createDocument,
transformOnTemplateError: !this.options?.isSvelteCheck,
globalSnapshotsManager: this.globalSnapshotsManager,
notifyExceedSizeLimit: this.options?.notifyExceedSizeLimit,
extendedConfigCache: this.extendedConfigCache,
onProjectReloaded: this.options?.onProjectReloaded,
watchTsConfig: !!this.options?.watch,
tsSystem: this.tsSystem,
projectService: projectService
};
}

/**
Expand All @@ -89,26 +125,15 @@ export class LSAndTSDocResolver {
return document;
};

private globalSnapshotsManager = new GlobalSnapshotsManager(
this.lsDocumentContext.tsSystem,
/* watchPackageJson */ !!this.options?.watch
);
private tsSystem: ts.System;
private globalSnapshotsManager: GlobalSnapshotsManager;
private extendedConfigCache = new Map<string, ts.ExtendedConfigCacheEntry>();
private getCanonicalFileName: GetCanonicalFileName;

private get lsDocumentContext(): LanguageServiceDocumentContext {
return {
ambientTypesSource: this.options?.isSvelteCheck ? 'svelte-check' : 'svelte2tsx',
createDocument: this.createDocument,
transformOnTemplateError: !this.options?.isSvelteCheck,
globalSnapshotsManager: this.globalSnapshotsManager,
notifyExceedSizeLimit: this.options?.notifyExceedSizeLimit,
extendedConfigCache: this.extendedConfigCache,
onProjectReloaded: this.options?.onProjectReloaded,
watchTsConfig: !!this.options?.watch,
tsSystem: this.options?.tsSystem ?? ts.sys
};
}
private userPreferencesAccessor: { preferences: ts.UserPreferences };
private readonly watchers: FileMap<ts.FileWatcher>;

private lsDocumentContext: LanguageServiceDocumentContext;

async getLSForPath(path: string) {
return (await this.getTSService(path)).getService();
Expand Down Expand Up @@ -251,4 +276,73 @@ export class LSAndTSDocResolver {
nearestWorkspaceUri ? urlToPath(nearestWorkspaceUri) : null
);
}

private getTsUserPreferences() {
return this.configManager.getTsUserPreferences('typescript', null);
}

private wrapWithPackageJsonMonitoring(sys: ts.System): ts.System {
if (!sys.watchFile || !this.options?.watch) {
return sys;
}

const watchFile = sys.watchFile;
return {
...sys,
readFile: (path, encoding) => {
if (path.endsWith('package.json') && !this.watchers.has(path)) {
this.watchers.set(
path,
watchFile(path, this.onPackageJsonWatchChange.bind(this), 3_000)
);
}

return sys.readFile(path, encoding);
}
};
}

private onPackageJsonWatchChange(path: string, onWatchChange: ts.FileWatcherEventKind) {
const dir = dirname(path);
const projectService = this.lsDocumentContext.projectService;
const packageJsonCache = projectService?.packageJsonCache;
const normalizedPath = projectService?.toPath(path);

if (onWatchChange === ts.FileWatcherEventKind.Deleted) {
this.watchers.get(path)?.close();
this.watchers.delete(path);
packageJsonCache?.delete(normalizedPath);
} else {
packageJsonCache?.addOrUpdate(normalizedPath);
}

forAllServices((service) => {
service.onPackageJsonChange(path);
});
if (!path.includes('node_modules')) {
return;
}

setTimeout(() => {
this.updateSnapshotsInDirectory(dir);
const realPath =
this.tsSystem.realpath &&
this.getCanonicalFileName(normalizePath(this.tsSystem.realpath?.(dir)));

// pnpm
if (realPath && realPath !== dir) {
this.updateSnapshotsInDirectory(realPath);
const realPkgPath = join(realPath, 'package.json');
forAllServices((service) => {
service.onPackageJsonChange(realPkgPath);
});
}
}, 500);
}

private updateSnapshotsInDirectory(dir: string) {
this.globalSnapshotsManager.getByPrefix(dir).forEach((snapshot) => {
this.globalSnapshotsManager.updateTsOrJsFile(snapshot.filePath);
});
}
}
96 changes: 1 addition & 95 deletions packages/language-server/src/plugins/typescript/SnapshotManager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import { TextDocumentContentChangeEvent } from 'vscode-languageserver';
import { createGetCanonicalFileName, GetCanonicalFileName, normalizePath } from '../../utils';
import { EventEmitter } from 'events';
import { FileMap } from '../../lib/documents/fileCollection';
import { dirname } from 'path';

type SnapshotChangeHandler = (fileName: string, newDocument: DocumentSnapshot | undefined) => void;

Expand All @@ -18,20 +17,10 @@ export class GlobalSnapshotsManager {
private emitter = new EventEmitter();
private documents: FileMap<DocumentSnapshot>;
private getCanonicalFileName: GetCanonicalFileName;
private packageJsonCache: PackageJsonCache;

constructor(
private readonly tsSystem: ts.System,
watchPackageJson = false
) {
constructor(private readonly tsSystem: ts.System) {
this.documents = new FileMap(tsSystem.useCaseSensitiveFileNames);
this.getCanonicalFileName = createGetCanonicalFileName(tsSystem.useCaseSensitiveFileNames);
this.packageJsonCache = new PackageJsonCache(
tsSystem,
watchPackageJson,
this.getCanonicalFileName,
this.updateSnapshotsInDirectory.bind(this)
);
}

get(fileName: string) {
Expand Down Expand Up @@ -94,16 +83,6 @@ export class GlobalSnapshotsManager {
removeChangeListener(listener: SnapshotChangeHandler) {
this.emitter.off('change', listener);
}

getPackageJson(path: string) {
return this.packageJsonCache.getPackageJson(path);
}

private updateSnapshotsInDirectory(dir: string) {
this.getByPrefix(dir).forEach((snapshot) => {
this.updateTsOrJsFile(snapshot.filePath);
});
}
}

export interface TsFilesSpec {
Expand Down Expand Up @@ -267,76 +246,3 @@ export class SnapshotManager {
}

export const ignoredBuildDirectories = ['__sapper__', '.svelte-kit'];

class PackageJsonCache {
constructor(
private readonly tsSystem: ts.System,
private readonly watchPackageJson: boolean,
private readonly getCanonicalFileName: GetCanonicalFileName,
private readonly updateSnapshotsInDirectory: (directory: string) => void
) {
this.watchers = new FileMap(tsSystem.useCaseSensitiveFileNames);
}

private readonly watchers: FileMap<ts.FileWatcher>;

private packageJsonCache = new FileMap<
{ text: string; modifiedTime: number | undefined } | undefined
>();

getPackageJson(path: string) {
if (!this.packageJsonCache.has(path)) {
this.packageJsonCache.set(path, this.initWatcherAndRead(path));
}

return this.packageJsonCache.get(path);
}

private initWatcherAndRead(path: string) {
if (this.watchPackageJson) {
this.tsSystem.watchFile?.(path, this.onPackageJsonWatchChange.bind(this), 3_000);
}
const exist = this.tsSystem.fileExists(path);

if (!exist) {
return undefined;
}

return this.readPackageJson(path);
}

private readPackageJson(path: string) {
return {
text: this.tsSystem.readFile(path) ?? '',
modifiedTime: this.tsSystem.getModifiedTime?.(path)?.valueOf()
};
}

private onPackageJsonWatchChange(path: string, onWatchChange: ts.FileWatcherEventKind) {
const dir = dirname(path);

if (onWatchChange === ts.FileWatcherEventKind.Deleted) {
this.packageJsonCache.delete(path);
this.watchers.get(path)?.close();
this.watchers.delete(path);
} else {
this.packageJsonCache.set(path, this.readPackageJson(path));
}

if (!path.includes('node_modules')) {
return;
}

setTimeout(() => {
this.updateSnapshotsInDirectory(dir);
const realPath =
this.tsSystem.realpath &&
this.getCanonicalFileName(normalizePath(this.tsSystem.realpath?.(dir)));

// pnpm
if (realPath && realPath !== dir) {
this.updateSnapshotsInDirectory(realPath);
}
}, 500);
}
}
4 changes: 3 additions & 1 deletion packages/language-server/src/plugins/typescript/features/CompletionProvider.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,9 @@ export class CompletionsProviderImpl implements CompletionsProvider<CompletionEn
offset,
{
...userPreferences,
triggerCharacter: validTriggerCharacter
triggerCharacter: validTriggerCharacter,
// Just in case, if there is a parser error. Force ts to not use the cache
triggerKind: tsDoc.parserError ? undefined : completionContext?.triggerKind
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this mean we got no advantage from this change in broken states, e.g when being in the middle of a template completion like <Comp?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was wrong. The incompletion cache requires another internal API. And it seems only rarely to trigger. So I think it's not worth the effort to add it now. Maybe we can add it later or add it to Volar when we switch.

In the module resolution "node", TypeScript only loads 100 of them and marks the completion as incomplete. That's when the incomplete cache can be handy. The completion is reused and only resolves more completion label detail, the faded module specifier text after the completion label. But In most of the cases I tested, It only marks it as incomplete in global completion, and most of the time, the first letter can already narrow the number down.

In module resolution node16+ and bundler, TypeScript will check all the module specifiers in auto import. It is probably to filter out auto import blocked by the export map. And create-svelte recently switched to bundler by default. That explains why, all of a sudden, many people started to complain about slow completion.

In the case of <Comp. Do you remember why we'll reuse the completion in case of triggerKind === CompletionTriggerKind?TriggerForIncompleteCompletions? It seems like we'll reuse the completion in this situation before asking ts for a new list. Is that the intended behaviour?

Copy link
Member

@dummdidumm dummdidumm Jan 11, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it was intentional because completion is rather expensive in this situation and if people type one character after the other they should be able to narrow down the existing completions.
Is this resulting in undesired behavior now?

Re "TS checks this more thoroughly now" - is there some inefficiency you've seen on the TS side which we could report upstream? Or is it unavoidable additional work that needs to be done to be more correct with the export conditions?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's mostly unavoidable. If there is a cache, it at least only happens the first time and after various caches are invalidated. #2244 might have something to optimize on the TypeScript side, but I think It is an edge case. I'm not sure if there is something less extreme and also having performance issues.

},
formatSettings
);
Expand Down
3 changes: 2 additions & 1 deletion packages/language-server/src/plugins/typescript/module-loader.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,8 @@ export function createSvelteModuleLoader(
resolveModuleNames,
resolveTypeReferenceDirectiveReferences,
mightHaveInvalidatedResolutions,
clearPendingInvalidations
clearPendingInvalidations,
getModuleResolutionCache: () => tsModuleCache
};

function resolveModuleNames(
Expand Down
Loading