Skip to content

Multifaceted approach to performantly enabling fileExists outside of the synchronize step in the emit host #25107

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

Merged
merged 5 commits into from
Jun 22, 2018
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
9 changes: 8 additions & 1 deletion src/compiler/program.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) } : {}),
};
}
Expand Down
14 changes: 7 additions & 7 deletions src/services/services.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1171,13 +1171,13 @@ namespace ts {
}

// Get a fresh cache of the host information
let 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;
}

Expand All @@ -1204,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);
}
Expand Down Expand Up @@ -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
Expand All @@ -1260,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));
Expand All @@ -1278,11 +1278,11 @@ namespace ts {
}

function getOrCreateSourceFileByPath(fileName: string, path: Path, _languageVersion: ScriptTarget, _onError?: (message: string) => void, shouldCreateNewSourceFile?: boolean): SourceFile | undefined {
Debug.assert(hostCache !== undefined);
Copy link
Member

Choose a reason for hiding this comment

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

I think you do want to assert that hostCache !== undefined. That is getOrCreateSourceFileByPath should be called only when hostCache is not 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.
const hostFileInformation = hostCache.getOrCreateEntryByPath(fileName, path);
const hostFileInformation = hostCache && hostCache.getOrCreateEntryByPath(fileName, path);
if (!hostFileInformation) {
return undefined;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
/// <reference path="fourslash.ts" />

// @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([]);