Can Module Resolution Cache Usage Be Improved?

TypeScript Version: 4.1.0-dev.20200902

Search Terms:

• module resolution cache
• resolveModuleNamesReusingOldState
• module resolution performance

Expected behavior:
Module resolution cache for files from a different Program is reused by resolveModuleNamesReusingOldState.

Actual behavior:
resolvedModules is always recalculated when a new Program is created.

I'm looking at a specific section of src/compiler/program.ts and see a place module resolution cache is available but unused. Utilizing this module resolution cache in my project reduced load time from ~18,768ms to ~7774ms in a specific (but common) scenario.

Suppose packages dep and main are loaded one after the other in tsserver, and main depends on dep. In this case:

1. The language service builds a new Program for main, eventually pulling files from dep into main’s program.
2. For dep’s SourceFile lookups, DocumentRegistry returns entries with file.resolvedModules already populated.
3. Unfortunately the guard in resolveModuleNamesReusingOldState ignores this and always recalculates file.resolvedModules when a new Program is constructed.

I'd like help in determining whether the resolvedModules recalculation in (3) is always necessary. Here's the guard in question for reference:

https://github.com/microsoft/TypeScript/blob/3b502f4ec1a44c18f7e272bc7a86fe82af7704e7/src/compiler/program.ts#L1067-L1075

I applied an extremely naive patch to play with:

diff --git a/src/compiler/program.ts b/src/compiler/program.ts
index d2810a857a..c9cf14bd8c 100644
--- a/src/compiler/program.ts
+++ b/src/compiler/program.ts
@@ -1065,7 +1065,9 @@ namespace ts {
         }
 
         function resolveModuleNamesReusingOldState(moduleNames: string[], containingFile: string, file: SourceFile) {
-            if (structuralIsReused === StructureIsReused.Not && !file.ambientModuleNames.length) {
+            const everyModuleNameIsResolved = moduleNames.every(moduleName => file.resolvedModules?.get(moduleName));
+
+            if (!everyModuleNameIsResolved && structuralIsReused === StructureIsReused.Not && !file.ambientModuleNames.length) {
                 // If the old program state does not permit reusing resolutions and `file` does not contain locally defined ambient modules,
                 // the best we can do is fallback to the default logic.
                 return resolveModuleNamesWorker(moduleNames, containingFile, /*reusedNames*/ undefined, getResolvedProjectReferenceToRedirect(file.originalFileName));

With this patch and running tsserver against an experimental repo, I saw:

• A ~2x performance improvement loading packages/b/src/b.ts after packages/a/src/a.ts. From ~8751ms to ~3866ms. (Packages a and b are small but both depend on c which has 50,000 files.)
• But lots of module resolution tests failing. I haven’t gone through all of them yet, but believe this is mostly because of the mock testing environment pre-compiling test cases. Compiling files before the test is ran causes the test to no-op and fail baseline reference tracing comparison: https://github.com/microsoft/TypeScript/blob/3b502f4ec1a44c18f7e272bc7a86fe82af7704e7/src/testRunner/compilerRunner.ts#L231-L238

I'd like to provide a pull request but curious for initial thoughts before doing so. It's possible I'm misunderstanding something about module resolution that makes this performance optimization not doable. Is that the case?

Thanks in advance!