Repository: TypeScript
Issue: Can Module Resolution Cache Usage Be Improved?
Labels: Suggestion,In Discussion,Needs Investigation,Rescheduled

**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
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](https://github.com/gluxon/ts-module-resolution-cache-testing), 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!