Skip to content

Cache file existence in synchronizeHostData #48942

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

Closed
wants to merge 1 commit into from
Closed

Cache file existence in synchronizeHostData #48942

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
44 changes: 38 additions & 6 deletions src/services/services.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -995,12 +995,16 @@ namespace ts {
// the set of scripts handled by the host changes.
class HostCache {
private fileNameToEntry: ESMap<Path, CachedHostFileInformation>;
private fileExistsCache: ESMap<string, boolean>;
private directoryExistsCache: undefined | ESMap<string, boolean>;
private currentDirectory: string;

constructor(private host: LanguageServiceHost, getCanonicalFileName: GetCanonicalFileName) {
constructor(private host: LanguageServiceHost, getCanonicalFileName: GetCanonicalFileName, cacheDirectoryExists: boolean) {
// script id => script index
this.currentDirectory = host.getCurrentDirectory();
this.fileNameToEntry = new Map();
this.fileExistsCache = new Map();
this.directoryExistsCache = cacheDirectoryExists ? new Map() : undefined;

// Initialize the list with the root file names
const rootFileNames = host.getScriptFileNames();
Expand Down Expand Up @@ -1030,6 +1034,22 @@ namespace ts {
return entry;
}

public fileExists(fileName: string): boolean | undefined {
return this.fileExistsCache.get(fileName);
}

public setFileExists(fileName: string, exists: boolean): void {
this.fileExistsCache.set(fileName, exists);
}

public directoryExists(directoryName: string): boolean | undefined {
return this.directoryExistsCache?.get(directoryName);
}

public setDirectoryExists(directoryName: string, exists: boolean): void {
this.directoryExistsCache?.set(directoryName, exists);
}

public getEntryByPath(path: Path): CachedHostFileInformation | undefined {
return this.fileNameToEntry.get(path);
}
Expand Down Expand Up @@ -1367,7 +1387,7 @@ namespace ts {
}

// Get a fresh cache of the host information
let hostCache: HostCache | undefined = new HostCache(host, getCanonicalFileName);
let hostCache: HostCache | undefined = new HostCache(host, getCanonicalFileName, /*cacheDirectoryExists*/ !!host.directoryExists);
const rootFileNames = hostCache.getRootFileNames();
const newSettings = host.getCompilationSettings() || getDefaultCompilerOptions();
const hasInvalidatedResolution: HasInvalidatedResolution = host.hasInvalidatedResolution || returnFalse;
Expand Down Expand Up @@ -1410,9 +1430,7 @@ namespace ts {
readFile,
getSymlinkCache: maybeBind(host, host.getSymlinkCache),
realpath: maybeBind(host, host.realpath),
directoryExists: directoryName => {
return directoryProbablyExists(directoryName, host);
},
directoryExists,
getDirectories: path => {
return host.getDirectories ? host.getDirectories(path) : [];
},
Expand Down Expand Up @@ -1492,11 +1510,25 @@ namespace ts {
}

function fileExists(fileName: string): boolean {
// Check this before the file contents cache because it saves computing the Path
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason you are not using changeCompilerHostLikeToUseCache

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 coded it up that way, but found it confusing because of the interaction with HostCache. changeCompilerHostLikeToUseCache covers

  1. readFile, which is already handled, slightly differently, by HostCache
  2. fileExists
  3. directoryExists
  4. createDirectory, which would not be used
  5. writeFile, which would not be used
  6. getSourceFile, which the caller has to decide to hook up or not and overlaps with HostCache

Some of the nuances, e.g. readFile vs readFileWithCache, felt like they would be confusing in this specialized context.

I believe the HostCache is also used in a few places that consume the language service host, rather than the compiler host.

If you feel strongly about it, I can try to rewrite it in a way that eliminates HostCache altogether, in favor of mutating the host directly.

Copy link
Member

Choose a reason for hiding this comment

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

I am just wondering if we can get rid of host cache and use our standard method so that if any additions or changes we do in future are not missed? Looked at fileExists , readFile etc in services and its used for duration of program creation so would prefer to use compilerHost and changeCompilerHostLikeToUseCache instead of maintaining our own caches..

// and also covers non-existence
const cached = hostCache?.fileExists(fileName);
if (cached !== undefined) return cached;
const path = toPath(fileName, currentDirectory, getCanonicalFileName);
const entry = hostCache && hostCache.getEntryByPath(path);
return entry ?
const exists = entry ?
!isString(entry) :
(!!host.fileExists && host.fileExists(fileName));
hostCache?.setFileExists(fileName, exists);
return exists;
}

function directoryExists(directoryName: string): boolean {
const cached = hostCache?.directoryExists(directoryName);
if (cached !== undefined) return cached;
const exists = directoryProbablyExists(directoryName, host);
hostCache?.setDirectoryExists(directoryName, exists);
return exists;
}

function readFile(fileName: string) {
Expand Down