From f1523d2bb83520f06f90a3c3de8f20f8ca4c012b Mon Sep 17 00:00:00 2001 From: Wesley Wigham Date: Wed, 20 Jun 2018 13:46:39 -0700 Subject: [PATCH 1/5] Multifaceted approach to performantly enabling fileExists outside of the synchronize step in the emit host --- src/compiler/program.ts | 9 +++++- src/services/services.ts | 29 ++++++++++++++++--- ...ypesDeclarationDiagnosticsNoServerError.ts | 14 +++++++++ 3 files changed, 47 insertions(+), 5 deletions(-) create mode 100644 tests/cases/fourslash/importTypesDeclarationDiagnosticsNoServerError.ts diff --git a/src/compiler/program.ts b/src/compiler/program.ts index 3289271e7c6df..fb29466aeeca0 100644 --- a/src/compiler/program.ts +++ b/src/compiler/program.ts @@ -1239,7 +1239,14 @@ namespace ts { (fileName, data, writeByteOrderMark, onError, sourceFiles) => host.writeFile(fileName, data, writeByteOrderMark, onError, sourceFiles)), isEmitBlocked, readFile: f => host.readFile(f), - fileExists: f => host.fileExists(f), + fileExists: f => { + // Use local caches + const path = toPath(f); + if (getSourceFileByPath(path)) return true; + if (contains(missingFilePaths, path)) return false; + // Before falling back to the host + return host.fileExists(f); + }, ...(host.directoryExists ? { directoryExists: f => host.directoryExists!(f) } : {}), }; } diff --git a/src/services/services.ts b/src/services/services.ts index 24feaac17c629..6832cbc753d57 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -869,7 +869,7 @@ namespace ts { private _compilationSettings: CompilerOptions; private currentDirectory: string; - constructor(private host: LanguageServiceHost, getCanonicalFileName: GetCanonicalFileName) { + constructor(private host: LanguageServiceHost, private getCanonicalFileName: GetCanonicalFileName) { // script id => script index this.currentDirectory = host.getCurrentDirectory(); this.fileNameToEntry = createMap(); @@ -892,6 +892,28 @@ namespace ts { return this.host.getProjectReferences && this.host.getProjectReferences(); } + /** + * Returns a copy of the HostCache where there are no cached script snapshots, + * just their paths/existence. + */ + public thinCopy() { + const cache = new HostCache(this.host, this.getCanonicalFileName); + const entries = this.fileNameToEntry.keys(); + for (let { done, value: key } = entries.next(); !done; { done, value: key } = entries.next()) { + const existing = this.fileNameToEntry.get(key); + if (!existing) continue; + this.fileNameToEntry.set(key, typeof existing !== "string" ? { + hostFileName: existing.hostFileName, + version: existing.version, + get scriptSnapshot(): IScriptSnapshot { + return Debug.fail("Thin cache does not support actually retrieving snapshots! This callstack is being executed outside of a context within which file contents are available!"); + }, + scriptKind: existing.scriptKind + } : existing); + } + return cache; + } + private createEntry(fileName: string, path: Path) { let entry: CachedHostFileInformation; const scriptSnapshot = this.host.getScriptSnapshot(fileName); @@ -1171,7 +1193,7 @@ namespace ts { } // Get a fresh cache of the host information - let hostCache = new HostCache(host, getCanonicalFileName); + let hostCache: HostCache = new HostCache(host, getCanonicalFileName); const rootFileNames = hostCache.getRootFileNames(); const hasInvalidatedResolution: HasInvalidatedResolution = host.hasInvalidatedResolution || returnFalse; @@ -1246,7 +1268,7 @@ namespace ts { // hostCache is captured in the closure for 'getOrCreateSourceFile' but it should not be used past this point. // It needs to be cleared to allow all collected snapshots to be released - hostCache = undefined!; + hostCache = hostCache.thinCopy(); // The thin copy does not cache any snapshots, just their paths, so is relatively safe to hold onto longer-term // We reset this cache on structure invalidation so we don't hold on to outdated files for long; however we can't use the `compilerHost` above, // Because it only functions until `hostCache` is cleared, while we'll potentially need the functionality to lazily read sourcemap files during @@ -1278,7 +1300,6 @@ namespace ts { } function getOrCreateSourceFileByPath(fileName: string, path: Path, _languageVersion: ScriptTarget, _onError?: (message: string) => void, shouldCreateNewSourceFile?: boolean): SourceFile | undefined { - Debug.assert(hostCache !== undefined); // The program is asking for this file, check first if the host can locate it. // If the host can not locate the file, then it does not exist. return undefined // to the program to allow reporting of errors for missing files. diff --git a/tests/cases/fourslash/importTypesDeclarationDiagnosticsNoServerError.ts b/tests/cases/fourslash/importTypesDeclarationDiagnosticsNoServerError.ts new file mode 100644 index 0000000000000..78ad677a57c01 --- /dev/null +++ b/tests/cases/fourslash/importTypesDeclarationDiagnosticsNoServerError.ts @@ -0,0 +1,14 @@ +/// + +// @declaration: true +// @Filename: node_modules/foo/index.d.ts +////export function f(): I; +////export interface I { +//// x: number; +////} +// @Filename: a.ts +////import { f } from "foo"; +////export const x = f(); + +goTo.file(1); +verify.getSemanticDiagnostics([]); From 6d1554353ae4435f3b63703626b186ffeb41a2b8 Mon Sep 17 00:00:00 2001 From: Wesley Wigham Date: Wed, 20 Jun 2018 15:19:25 -0700 Subject: [PATCH 2/5] make cache undefinable and handle correctly --- src/services/services.ts | 36 +++++++----------------------------- 1 file changed, 7 insertions(+), 29 deletions(-) diff --git a/src/services/services.ts b/src/services/services.ts index 6832cbc753d57..3fb73e04b53fa 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -869,7 +869,7 @@ namespace ts { private _compilationSettings: CompilerOptions; private currentDirectory: string; - constructor(private host: LanguageServiceHost, private getCanonicalFileName: GetCanonicalFileName) { + constructor(private host: LanguageServiceHost, getCanonicalFileName: GetCanonicalFileName) { // script id => script index this.currentDirectory = host.getCurrentDirectory(); this.fileNameToEntry = createMap(); @@ -892,28 +892,6 @@ namespace ts { return this.host.getProjectReferences && this.host.getProjectReferences(); } - /** - * Returns a copy of the HostCache where there are no cached script snapshots, - * just their paths/existence. - */ - public thinCopy() { - const cache = new HostCache(this.host, this.getCanonicalFileName); - const entries = this.fileNameToEntry.keys(); - for (let { done, value: key } = entries.next(); !done; { done, value: key } = entries.next()) { - const existing = this.fileNameToEntry.get(key); - if (!existing) continue; - this.fileNameToEntry.set(key, typeof existing !== "string" ? { - hostFileName: existing.hostFileName, - version: existing.version, - get scriptSnapshot(): IScriptSnapshot { - return Debug.fail("Thin cache does not support actually retrieving snapshots! This callstack is being executed outside of a context within which file contents are available!"); - }, - scriptKind: existing.scriptKind - } : existing); - } - return cache; - } - private createEntry(fileName: string, path: Path) { let entry: CachedHostFileInformation; const scriptSnapshot = this.host.getScriptSnapshot(fileName); @@ -1193,13 +1171,13 @@ namespace ts { } // Get a fresh cache of the host information - let hostCache: HostCache = new HostCache(host, getCanonicalFileName); + let hostCache: HostCache | undefined = new HostCache(host, getCanonicalFileName); const rootFileNames = hostCache.getRootFileNames(); const hasInvalidatedResolution: HasInvalidatedResolution = host.hasInvalidatedResolution || returnFalse; // If the program is already up-to-date, we can reuse it - if (isProgramUptoDate(program, rootFileNames, hostCache.compilationSettings(), path => hostCache.getVersion(path), fileExists, hasInvalidatedResolution, !!host.hasChangedAutomaticTypeDirectiveNames)) { + if (isProgramUptoDate(program, rootFileNames, hostCache.compilationSettings(), path => hostCache!.getVersion(path), fileExists, hasInvalidatedResolution, !!host.hasChangedAutomaticTypeDirectiveNames)) { return; } @@ -1226,7 +1204,7 @@ namespace ts { readFile(fileName) { // stub missing host functionality const path = toPath(fileName, currentDirectory, getCanonicalFileName); - const entry = hostCache.getEntryByPath(path); + const entry = hostCache && hostCache.getEntryByPath(path); if (entry) { return isString(entry) ? undefined : getSnapshotText(entry.scriptSnapshot); } @@ -1268,7 +1246,7 @@ namespace ts { // hostCache is captured in the closure for 'getOrCreateSourceFile' but it should not be used past this point. // It needs to be cleared to allow all collected snapshots to be released - hostCache = hostCache.thinCopy(); // The thin copy does not cache any snapshots, just their paths, so is relatively safe to hold onto longer-term + hostCache = undefined!; // We reset this cache on structure invalidation so we don't hold on to outdated files for long; however we can't use the `compilerHost` above, // Because it only functions until `hostCache` is cleared, while we'll potentially need the functionality to lazily read sourcemap files during @@ -1282,7 +1260,7 @@ namespace ts { function fileExists(fileName: string): boolean { const path = toPath(fileName, currentDirectory, getCanonicalFileName); - const entry = hostCache.getEntryByPath(path); + const entry = hostCache && hostCache.getEntryByPath(path); return entry ? !isString(entry) : (!!host.fileExists && host.fileExists(fileName)); @@ -1303,7 +1281,7 @@ namespace ts { // The program is asking for this file, check first if the host can locate it. // If the host can not locate the file, then it does not exist. return undefined // to the program to allow reporting of errors for missing files. - const hostFileInformation = hostCache.getOrCreateEntryByPath(fileName, path); + const hostFileInformation = hostCache && hostCache.getOrCreateEntryByPath(fileName, path); if (!hostFileInformation) { return undefined; } From a8b1a7dafe8a39667105f2ec96ee9e7b23a15b95 Mon Sep 17 00:00:00 2001 From: Wesley Wigham Date: Wed, 20 Jun 2018 15:20:31 -0700 Subject: [PATCH 3/5] Remove unneeded cast --- src/services/services.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/services/services.ts b/src/services/services.ts index 3fb73e04b53fa..f9825e7b86629 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -1246,7 +1246,7 @@ namespace ts { // hostCache is captured in the closure for 'getOrCreateSourceFile' but it should not be used past this point. // It needs to be cleared to allow all collected snapshots to be released - hostCache = undefined!; + hostCache = undefined; // We reset this cache on structure invalidation so we don't hold on to outdated files for long; however we can't use the `compilerHost` above, // Because it only functions until `hostCache` is cleared, while we'll potentially need the functionality to lazily read sourcemap files during From 6eaea18f431cf52cbdd868229bd73949ec8d5591 Mon Sep 17 00:00:00 2001 From: Wesley Wigham Date: Wed, 20 Jun 2018 19:00:05 -0700 Subject: [PATCH 4/5] Readd assert --- src/services/services.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/services/services.ts b/src/services/services.ts index f9825e7b86629..abfc7a701a678 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -1278,6 +1278,7 @@ namespace ts { } function getOrCreateSourceFileByPath(fileName: string, path: Path, _languageVersion: ScriptTarget, _onError?: (message: string) => void, shouldCreateNewSourceFile?: boolean): SourceFile | undefined { + Debug.assert(hostCache !== undefined); // The program is asking for this file, check first if the host can locate it. // If the host can not locate the file, then it does not exist. return undefined // to the program to allow reporting of errors for missing files. From 8930cf3dcbee2d589ac46e8ceb79b53e6e05022f Mon Sep 17 00:00:00 2001 From: Wesley Wigham Date: Wed, 20 Jun 2018 19:01:32 -0700 Subject: [PATCH 5/5] More useful failure messager --- src/services/services.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/services/services.ts b/src/services/services.ts index abfc7a701a678..c1e7469fd2d56 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -1278,7 +1278,7 @@ namespace ts { } function getOrCreateSourceFileByPath(fileName: string, path: Path, _languageVersion: ScriptTarget, _onError?: (message: string) => void, shouldCreateNewSourceFile?: boolean): SourceFile | undefined { - Debug.assert(hostCache !== undefined); + Debug.assert(hostCache !== undefined, "getOrCreateSourceFileByPath called after typical CompilerHost lifetime, check the callstack something with a reference to an old host."); // The program is asking for this file, check first if the host can locate it. // If the host can not locate the file, then it does not exist. return undefined // to the program to allow reporting of errors for missing files.