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:

The language service builds a new Program for main, eventually pulling files from dep into main’s program.
For dep’s SourceFile lookups, DocumentRegistry returns entries with file.resolvedModules already populated.
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:

TypeScript/src/compiler/program.ts

Lines 1067 to 1075 in 3b502f4

    
           function resolveModuleNamesReusingOldState(moduleNames: string[], containingFile: string, file: SourceFile) { 
        
               if (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)); 
        
               } 
        
               const oldSourceFile = oldProgram && oldProgram.getSourceFile(containingFile); 
        
               if (oldSourceFile !== file && file.resolvedModules) {

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:

TypeScript/src/testRunner/compilerRunner.ts

Lines 231 to 238 in 3b502f4

    
           this.result = Compiler.compileFiles( 
        
               this.toBeCompiled, 
        
               this.otherFiles, 
        
               this.harnessSettings, 
        
               /*options*/ tsConfigOptions, 
        
               /*currentDirectory*/ this.harnessSettings.currentDirectory, 
        
               testCaseContent.symlinks 
        
           );

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!

Ryan Cavanaugh (@RyanCavanaugh)

i have had chat in past about this with Ryan Cavanaugh (@RyanCavanaugh) that we could this as part of some option/flag where user opt in to say anything that is already resolved persists across compilations as we had talked about getting this for 4.1.

The work to get that involves:

New flag (name which Daniel Rosenwasser (@DanielRosenwasser) can help with once we have clear idea of what it should do.. we had not discussed if this should be any resolution that persists or only correctly resolved resolutions persists.. (i think later makes more sense but haven't had chance evaluate that)
Handling this in in program creation, resolution caching (Resolution cache that handles editor and watch scenarios), tsbuildinfo to have this information serialized and read and yet reserve the sharing of resolutions (this is one of the biggest thing that needs to happen for any use in incremental/watch scenarios since we want to preserve the optimizations we have for sharing resolutions and being able to invalidate and watch them)

Sheetal Nandi (@sheetalkamat)

Thank you Sheetal Nandi (@sheetalkamat) and Daniel Rosenwasser (@DanielRosenwasser) for looking into this for us.

Thanks from me as well. Appreciate the quick response yesterday and glad this was already on the radar.

It sounds like there's a larger scope version of this where resolution cache is persisted to disk between tsc/tsserver restarts. I'm excited for that and understand why it would require a feature flag.

In the meantime I want to drop a quick thought. I think there's a potential short-term, lower payoff alternative without a flag. Program can likely trust the resolutions it sees in a SourceFile with higher confidence if the language service reference count is greater than 1. In that case there's already another portion of tsserver using those resolutions. If those resolutions are outdated, it'd at least be consistently outdated.

TypeScript/src/services/documentRegistry.ts

Lines 104 to 111 in 7838b01

    
           interface DocumentRegistryEntry { 
        
               sourceFile: SourceFile; 
        
               // The number of language services that this source file is referenced in.   When no more 
        
               // language services are referencing the file, then the file can be removed from the 
        
               // registry. 
        
               languageServiceRefCount: number; 
        
           }

Although it may not be worth exploring that if the end goal for 4.1.0 is to implement the larger scope, higher payoff version.

I think there's a potential short-term, lower payoff alternative without a flag. Program can likely trust the resolutions it sees in a SourceFile with higher confidence if the language service reference count is greater than 1.

Gave this an attempt and failed on a test that remapped @ref/a in a compilerOptions.paths change. Ignore my suggestion above because it doesn't handle this case.

  15) unittests:: tsbuild:: watchMode:: program updates
       tsc-watch and tsserver works with project references
         invoking when references are already built
           on transitive references
             when config files are in side by side folders
               edit on config file
                 with tsserver:

      AssertionError: Configured project: /user/username/projects/transitiveReferences/c/tsconfig.json:: actual files: expected to contain /user/username/projects/transitiveReferences/nrefs/a.d.ts, actual keys: /a/lib/lib.d.ts,/user/username/projects/transitiveReferences/a/index.ts,/user/username/projects/transitiveReferences/b/index.ts,/user/username/projects/transitiveReferences/refs/a.d.ts,/user/username/projects/transitiveReferences/c/index.ts,/user/username/projects/transitiveReferences/c/tsconfig.json: expected false to be true
      + expected - actual

      -false
      +true

Navigation Menu

Repository navigation

Can Module Resolution Cache Usage Be Improved? #40356

5 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

	function resolveModuleNamesReusingOldState(moduleNames: string[], containingFile: string, file: SourceFile) {
	if (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));
	}

	const oldSourceFile = oldProgram && oldProgram.getSourceFile(containingFile);
	if (oldSourceFile !== file && file.resolvedModules) {

	this.result = Compiler.compileFiles(
	this.toBeCompiled,
	this.otherFiles,
	this.harnessSettings,
	/options/ tsConfigOptions,
	/currentDirectory/ this.harnessSettings.currentDirectory,
	testCaseContent.symlinks
	);

	interface DocumentRegistryEntry {
	sourceFile: SourceFile;

	// The number of language services that this source file is referenced in. When no more
	// language services are referencing the file, then the file can be removed from the
	// registry.
	languageServiceRefCount: number;
	}

Can Module Resolution Cache Usage Be Improved? #40356

Description

Activity

sheetalkamat commented on Sep 2, 2020

ericanderson commented on Sep 3, 2020

gluxon commented on Sep 3, 2020

gluxon commented on Sep 8, 2020

gluxon commented on Sep 25, 2020

5 remaining items

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions