Check if path is excluded via watch options before adding watchers during watchFailedLookupLocationOfNonRelativeModuleResolutions

[timse]

🔍 Search Terms

finishCachingPerDirectoryResolution
failedLookupLocation

✅ Viability Checklist

• This wouldn't be a breaking change in existing TypeScript/JavaScript code
• This wouldn't change the runtime behavior of existing JavaScript code
• This could be implemented without emitting different JS based on the types of the expressions
• This isn't a runtime feature (e.g. library functionality, non-ECMAScript syntax with JavaScript output, new syntax sugar for JS, etc.)
• This isn't a request to add a new utility type: https://github.com/microsoft/TypeScript/wiki/No-New-Utility-Types
• This feature would agree with the rest of our Design Goals: https://github.com/Microsoft/TypeScript/wiki/TypeScript-Design-Goals

⭐ Suggestion

System

Typescript 5.4

Context

We have a rather large Typescript project, with both a lot of internal as well as external dependendencies.
I have more recently started looking into the rather sluggish initial "startup times" in our IDEs.
By "startup time" I mean the initial parsing etc. before intellisense etc. can kick in and show type information.

Activating tracing and checking times from VSCode the big block of finishCachingPerDirectoryResolution at the end immediatelly caught my eye.
Depending on the amount of dependencies a package/project has this can take upwards of 10 seconds.
After further investigation it really boils down to the initial call to watchFailedLookupLocationOfResolution (

TypeScript/src/compiler/resolutionCache.ts

Lines 1146 to 1165 in 0dda037

	function watchFailedLookupLocationOfResolution(resolution: ResolutionWithFailedLookupLocations) {
	Debug.assert(!!resolution.files?.size);
	const { failedLookupLocations, affectingLocations, alternateResult } = resolution;
	if (!failedLookupLocations?.length && !affectingLocations?.length && !alternateResult) return;
	if (failedLookupLocations?.length || alternateResult) resolutionsWithFailedLookups.add(resolution);
	let setAtRoot = false;
	if (failedLookupLocations) {
	for (const failedLookupLocation of failedLookupLocations) {
	setAtRoot = watchFailedLookupLocation(failedLookupLocation, setAtRoot);
	}
	}
	if (alternateResult) setAtRoot = watchFailedLookupLocation(alternateResult, setAtRoot);
	if (setAtRoot) {
	// This is always non recursive
	setDirectoryWatcher(rootDir, rootPath, /*packageDir*/ undefined, /*packageDirPath*/ undefined, /*nonRecursive*/ true);
	}
	watchAffectingLocationsOfResolution(resolution, !failedLookupLocations?.length && !alternateResult);
	}

)

In particular this loop:

TypeScript/src/compiler/resolutionCache.ts

Lines 1155 to 1157 in 0dda037

	for (const failedLookupLocation of failedLookupLocations) {
	setAtRoot = watchFailedLookupLocation(failedLookupLocation, setAtRoot);
	}

is the main problem as for some reason, having many external dependencies can mean this array can grow beyond 100k entries.

Solutioning

First try

The first thing I was wondering is if I could simply use the watchOptions.excludeDirectories to avoid having so many watchers being set for at times irrelevant directories (irrelevant in the sense that i can guarantee they wont ever be needed to be watched), however this setting is simply ignored at this point.

Second try

I added a isIgnoredByWatchOptions inside the above mentioned for loop to avoid calling watchFailedLookupLocation for a location that should be ignored anyway. This did filter out calls, however did not make things faster.
To simply verify anything i added a !failedLookupLocation.includes('node_modules') to the check before calling isIgnoredByWatchOptions which brought down times roughly ~90%.
So i dug deeper into isIgnoredByWatchOptions and it looks like it is creating a rather expensive matcher every time it gets called, even if it basically gets called 100k times with the same matcher relevant parameters but a different path to check against.

Third try

I used the available memoizeCached to create a cached version of the regex creating found in the first lines of matchesExcludeWorker (

TypeScript/src/compiler/commandLineParser.ts

Lines 4010 to 4022 in 0dda037

	export function matchesExcludeWorker(
	pathToCheck: string,
	excludeSpecs: readonly string[] | undefined,
	useCaseSensitiveFileNames: boolean,
	currentDirectory: string,
	basePath?: string,
	): boolean {
	const excludePattern = getRegularExpressionForWildcard(excludeSpecs, combinePaths(normalizePath(currentDirectory), basePath), "exclude");
	const excludeRegex = excludePattern && getRegexFromPattern(excludePattern, useCaseSensitiveFileNames);
	if (!excludeRegex) return false;
	if (excludeRegex.test(pathToCheck)) return true;
	return !hasExtension(pathToCheck) && excludeRegex.test(ensureTrailingDirectorySeparator(pathToCheck));
	}

)

const getCachedExcludeMatcher = memoizeCached((excludeSpecs, basePath, useCaseSensitiveFileNames) => {
	const usefulSpecs = filter(excludeSpecs, (spec) => !invalidDotDotAfterRecursiveWildcard(spec))
	const excludePattern = getRegularExpressionForWildcard(usefulSpecs, basePath, "exclude");
  const excludeRegex = excludePattern && getRegexFromPattern(excludePattern, useCaseSensitiveFileNames);
	return excludeRegex;
}, new MatchExcludeCache());

using this cached "excludeMatcher" approach together with watchOptions.excludeDirectories and checking isIgnoredByWatchOptions in the failedLookupLocations loop brought down time about 75% from ~8 seconds to 2 seconds.

Ask

My question is:
a) On a very basic level - why can there by so many (100k+) failed lookup locations and how can that be avoided
b) Is it ok to check if a failed lookup location is marked as "excluded" from watch options and check for that before watching it
c) Are you ok for me to raise a PR with the above solutioning to reduce time for the watcher setup?

📃 Motivating Example

Watcher setup time for larger projects has been reduced resulting in faster tsserver load times in IDEs

💻 Use Cases

1. What do you want to use this for?
   improve ide setup performance
2. What shortcomings exist with current approaches?
   the current creating of matchers is creating performance issues when called a lot
3. What workarounds are you using in the meantime?
   n/a