Lazy calculation of expensive file explaining diagnsotics and some caching to be used to share the diagnostic data #58398

sheetalkamat · 2024-05-01T23:55:23Z

Store information about file explaining diagnostics to be generated only when program diagnostics are asked. We store this already if it's the diagnostic while we are creating program graph, we now store other compiler options errors that explain file include reasons as well as lazy diagnostics to be generated and then actually populate it when program diagnostics are queried.
In diagnostic explaining the reason why file is in program, we are caching details as well as related info that can be reused when there are multiple diagnostics for same file. The cache is cleared after population of program diagnostics is complete so doesn't stay beyond the computation of these diagnostics.

Probably fixes

cc: @jakebailey
As discussed offline few days ago the real issue is that typescript-eslint is passing in canonical file path of tsconfig and that generates lot of errors in program because of forceConsistentCasingInFileNames being defaulting to true in later versions of typescript. I have typescript-eslint/typescript-eslint#9042 for tsconfig issue we discussed on wednesday but i dont know if there are any other eg source files or anything that are having these issues as i didnt get to look into code that deeply

Here are some perf numbers with the repro in #58011 (comment) where API is used with and without config file name lower casing. Special thanks to @reduckted to give us this repro so we can actually verify this.

When diagnostics are queried (runs emitFilesAndReportErrors) after creating watch program:

	tsc (5.4.5)	eslint (5.4.5)	delta (5.4.5)	tsc (fix)	eslint (fix)	delta (fix)	tsc (delta with fix)	eslint (delta with fix)
Files:	4843	4843		4843	4843
Memory used:	313744K	1366218K	335.46%	334185K	363474K	8.76%	6.52%	-73.40%
Program time:	9.38s	31.98s	240.94%	9.16s	9.18s	0.22%	-2.35%	-71.29%
Total time:	11.22s	33.40s	197.68%	10.50s	10.72s	2.10%	-6.42%	-67.90%

Detailed Extended Diagnostics

	tsc (5.4.5)	eslint (5.4.5)	delta (5.4.5)	tsc (fix)	eslint (fix)	delta (fix)	tsc (delta with fix)	eslint (delta with fix)
Files:	4843	4843		4843	4843
Lines of Library:	38041	38041		38310	38310
Lines of Definitions:	174189	174189		174189	174189
Lines of TypeScript:	32561	32561		32561	32561
Lines of JavaScript:	30	30		30	30
Lines of JSON:	0	0		0	0
Lines of Other:	0	0		0	0
Identifiers:	201753	201753		201976	201976
Symbols:	140060	140060		140227	140227
Types:	2272	2272		2273	2273
Instantiations:	168	168		168	168
Memory used:	313744K	1366218K	335.46%	334185K	363474K	8.76%	6.52%	-73.40%
Assignability cache size:	24	24		24	24
Identity cache size:	0	0		0	0
Subtype cache size:	0	0		0	0
Strict subtype cache size:	0	0		0	0
I/O Read time:	0.98s	1.16s		0.91s	0.93s
Parse time:	1.61s	2.00s		1.50s	1.47s
ResolveModule time:	1.34s	1.51s		1.25s	1.16s
ResolveTypeReference time:	0.03s	0.03s		0.03s	0.03s
ResolveLibrary time:	0.06s	0.04s		0.05s	0.05s
Program time:	9.38s	31.98s	240.94%	9.16s	9.18s	0.22%	-2.35%	-71.29%
Bind time:	0.61s	0.67s		0.61s	0.67s
Check time:	1.22s	0.74s		0.72s	0.87s
printTime time:	0.00s	0.00s		0.00s	0.00s
Emit time:	0.01s	0.01s		0.00s	0.00s
Total time:	11.22s	33.40s	197.68%	10.50s	10.72s	2.10%	-6.42%	-67.90%

When diagnostics are not queried which is what es-lint does as per @jakebailey

	tsc (5.4.5)	eslint (5.4.5)	delta (5.4.5)	tsc (fix)	eslint (fix)	delta (fix)	tsc (delta with fix)	eslint (delta with fix)
Files:	4843	4843		4843	4843
Memory used:	290762K	1352409K	365.13%	324293K	320025K	-1.32%	11.53%	-76.34%
Program time:	9.63s	26.77s	177.99%	9.31s	9.60s	3.11%	-3.32%	-64.14%
Total time:	10.30s	27.50s	166.99%	9.89s	10.13s	14.31s	41.26%	-63.16%

Detailed Extended Diagnostics

	tsc (5.4.5)	eslint (5.4.5)	delta (5.4.5)	tsc (fix)	eslint (fix)	delta (fix)	tsc (delta with fix)	eslint (delta with fix)
Files:	4843	4843		4843	4843
Lines of Library:	38041	38041		38310	38310
Lines of Definitions:	174189	174189		174189	174189
Lines of TypeScript:	32561	32561		32561	32561
Lines of JavaScript:	30	30		30	30
Lines of JSON:	0	0		0	0
Lines of Other:	0	0		0	0
Identifiers:	201753	201753		201976	201976
Symbols:	140029	140029		140196	140196
Types:	89	89		88	88
Instantiations:	0	0		0	0
Memory used:	290762K	1352409K	365.13%	324293K	320025K	-1.32%	11.53%	-76.34%
Assignability cache size:	0	0		0	0
Identity cache size:	0	0		0	0
Subtype cache size:	0	0		0	0
Strict subtype cache size:	0	0		0	0
I/O Read time:	0.97s	0.99s		0.93s	0.99s
Parse time:	1.74s	1.66s		1.51s	1.59s
ResolveModule time:	1.42s	1.22s		1.26s	1.20s
ResolveTypeReference time:	0.03s	0.03s		0.03s	0.03s
ResolveLibrary time:	0.05s	0.03s		0.06s	0.05s
Program time:	9.63s	26.77s	177.99%	9.31s	9.60s	3.11%	-3.32%	-64.14%
Bind time:	0.67s	0.73s		0.58s	0.54s
Total time:	10.30s	27.50s	166.99%	9.89s	10.13s	14.31s	41.26%	-63.16%

sheetalkamat · 2024-05-02T22:51:43Z

@typescript-bot pack this

typescript-bot · 2024-05-02T22:51:45Z

Starting jobs; this comment will be updated as builds start and complete.

Command	Status	Results
`pack this`	✅ Started	✅ Results

typescript-bot · 2024-05-02T22:56:23Z

Hey @sheetalkamat, I've packed this into an installable tgz. You can install it for testing by referencing it in your package.json like so:

{
    "devDependencies": {
        "typescript": "https://typescript.visualstudio.com/cf7ac146-d525-443c-b23c-0d58337efebc/_apis/build/builds/161598/artifacts?artifactName=tgz&fileId=3F3D97042F3CD53F19137FD5481087CF59977AC9681B5F1C2DEDE0CAE6673D6E02&fileName=/typescript-5.5.0-insiders.20240502.tgz"
    }
}

and then running npm install.

There is also a playground for this build and an npm module you can use via "typescript": "npm:@typescript-deploys/pr-build@5.5.0-pr-58398-2".;

jakebailey

Behavior seems correct, but a few small comments.

src/compiler/program.ts

src/compiler/types.ts

src/compiler/program.ts

…ady processed

kronodeus · 2024-06-28T01:12:08Z

@sheetalkamat I am experiencing an error with these changes in my project when using getCombinedCodeFix:

TypeError: Cannot read properties of undefined (reading 'push')                                                                                                                                                                                                                                               
                                                                                                                                                                                                                                             
      at addLazyProgramDiagnosticExplainingFile (../../node_modules/.pnpm/@ts-morph+common@0.24.0/node_modules/@ts-morph/common/dist/typescript.js:126723:41)                                                                                
      at checkSourceFilesBelongToPath (../../node_modules/.pnpm/@ts-morph+common@0.24.0/node_modules/@ts-morph/common/dist/typescript.js:126142:11)                                                                                          
      at ../../node_modules/.pnpm/@ts-morph+common@0.24.0/node_modules/@ts-morph/common/dist/typescript.js:124558:37                                                                                                                         
      at getCommonSourceDirectory (../../node_modules/.pnpm/@ts-morph+common@0.24.0/node_modules/@ts-morph/common/dist/typescript.js:117872:53)
      at Object.getCommonSourceDirectory2 [as getCommonSourceDirectory] (../../node_modules/.pnpm/@ts-morph+common@0.24.0/node_modules/@ts-morph/common/dist/typescript.js:124553:31)
      at getDeclarationEmitOutputFilePath (../../node_modules/.pnpm/@ts-morph+common@0.24.0/node_modules/@ts-morph/common/dist/typescript.js:20116:119)
      at getOutputPathsFor (../../node_modules/.pnpm/@ts-morph+common@0.24.0/node_modules/@ts-morph/common/dist/typescript.js:117787:96)
      at transformRoot (../../node_modules/.pnpm/@ts-morph+common@0.24.0/node_modules/@ts-morph/common/dist/typescript.js:115914:62)
      at transformation (../../node_modules/.pnpm/@ts-morph+common@0.24.0/node_modules/@ts-morph/common/dist/typescript.js:117457:14)
      at transformRoot (../../node_modules/.pnpm/@ts-morph+common@0.24.0/node_modules/@ts-morph/common/dist/typescript.js:117480:71)
      at transformNodes (../../node_modules/.pnpm/@ts-morph+common@0.24.0/node_modules/@ts-morph/common/dist/typescript.js:117465:70)
      at getDeclarationDiagnostics (../../node_modules/.pnpm/@ts-morph+common@0.24.0/node_modules/@ts-morph/common/dist/typescript.js:115644:18)
      at ../../node_modules/.pnpm/@ts-morph+common@0.24.0/node_modules/@ts-morph/common/dist/typescript.js:125420:14
      at runWithCancellationToken (../../node_modules/.pnpm/@ts-morph+common@0.24.0/node_modules/@ts-morph/common/dist/typescript.js:125122:14)
      at getDeclarationDiagnosticsForFileNoCache (../../node_modules/.pnpm/@ts-morph+common@0.24.0/node_modules/@ts-morph/common/dist/typescript.js:125418:12)
      at getAndCacheDiagnostics (../../node_modules/.pnpm/@ts-morph+common@0.24.0/node_modules/@ts-morph/common/dist/typescript.js:125429:20)
      at getDeclarationDiagnosticsWorker (../../node_modules/.pnpm/@ts-morph+common@0.24.0/node_modules/@ts-morph/common/dist/typescript.js:125415:12)
      at getDeclarationDiagnosticsForFile (../../node_modules/.pnpm/@ts-morph+common@0.24.0/node_modules/@ts-morph/common/dist/typescript.js:125438:48)
      at getDiagnosticsHelper (../../node_modules/.pnpm/@ts-morph+common@0.24.0/node_modules/@ts-morph/common/dist/typescript.js:125070:44)
      at Object.getDeclarationDiagnostics2 [as getDeclarationDiagnostics] (../../node_modules/.pnpm/@ts-morph+common@0.24.0/node_modules/@ts-morph/common/dist/typescript.js:125108:14)
      at getDiagnostics (../../node_modules/.pnpm/@ts-morph+common@0.24.0/node_modules/@ts-morph/common/dist/typescript.js:151176:18)
      at eachDiagnostic (../../node_modules/.pnpm/@ts-morph+common@0.24.0/node_modules/@ts-morph/common/dist/typescript.js:151162:23)
      at Object.getAllCodeActions (../../node_modules/.pnpm/@ts-morph+common@0.24.0/node_modules/@ts-morph/common/dist/typescript.js:154119:5)
      at Object.getAllFixes (../../node_modules/.pnpm/@ts-morph+common@0.24.0/node_modules/@ts-morph/common/dist/typescript.js:151148:65)
      at Object.getCombinedCodeFix (../../node_modules/.pnpm/@ts-morph+common@0.24.0/node_modules/@ts-morph/common/dist/typescript.js:149433:31)

Unfortunately I am not exactly sure how to reproduce this issue outside of my codebase, but it seems there is some code path that calls addLazyProgramDiagnosticExplainingFile (which calls push on lazyProgramDiagnosticExplainingFile assuming it is defined) after updateAndGetProgramDiagnostics is called, which sets it to undefined.

Here is the call stack that hits updateAndGetProgramDiagnostics which sets the array to undefined before the above call stack:

updateAndGetProgramDiagnostics
getProgramDiagnostics
getSemanticDiagnosticsForFile
getDiagnosticsHelper
Object.getSemanticDiagnostics
getDiagnostics
eachDiagnostic
getAllCodeActions
Object.getAllFixes
Object.getCombinedCodeFix

This is occurring in 5.5.2

kronodeus · 2024-06-28T01:43:26Z

In codeFixProvider.ts#getDiagnostics I see we are getting semantic diagnostics before getting declaration diagnostics. As you can see in the stack traces above, the code path which sets lazyProgramDiagnosticExplainingFile to undefined is reached while getting semantic diagnostics while the code path which attempts to push to it is reached while getting declaration diagnostics. The change to get declaration diagnostics after semantic diagnostics came from this commit.

My hunch is that in order to reproduce this issue you'd need to request code fixes for a file that has both semantic diagnostics AND declaration diagnostics. The error doesn't occur if I set "declaration": false and "composite": false in my tsconfig.json because that causes this if-statement to evaluate to false and skip declaration diagnostics.

sheetalkamat · 2024-06-28T15:56:50Z

Please send the tsserver log so we can investigate

sheetalkamat · 2024-06-28T19:59:47Z

Ok I have the repro .. will look into this

sheetalkamat added 3 commits May 1, 2024 16:37

Add some tests

3a29c66

Store less data for file preprocessing diagnostics

a35bf99

Populate program diagnostics on demand

4f16ff5

typescript-bot assigned sheetalkamat May 1, 2024

typescript-bot added Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels May 1, 2024

sheetalkamat added 3 commits May 1, 2024 17:10

Remove duplicate reasons

f5a6bf4

Stricter types

3653901

Caching file explaining diagnostic chain and reusing

c8e1180

sheetalkamat changed the title ~~[wip] on demand program diagnostics population~~ Lazy calculation of expensive file explaining diagnsotics and some caching to be used to share the diagnostic data May 3, 2024

This was linked to issues May 3, 2024

Increased memory usage when creating a watch program that includes local JavaScript files in node_modules #58011

Closed

OOM typescript, mac m1+. #58274

Closed

sheetalkamat marked this pull request as ready for review May 3, 2024 19:29

sheetalkamat requested review from andrewbranch and jakebailey May 3, 2024 19:29

sheetalkamat mentioned this pull request May 3, 2024

fix: do not pass tsconfig canonical file name to typescript API to get program details for config file typescript-eslint/typescript-eslint#9042

Merged

3 tasks

Release program on close

3eb6754

jakebailey mentioned this pull request May 6, 2024

Ensure equal FileIncludeReasons are not duplicitively added #58352

Closed

jakebailey reviewed May 6, 2024

View reviewed changes

src/compiler/program.ts Outdated Show resolved Hide resolved

src/compiler/types.ts Outdated Show resolved Hide resolved

src/compiler/program.ts Show resolved Hide resolved

src/compiler/program.ts Outdated Show resolved Hide resolved

Fix typo

7f56cc5

sheetalkamat force-pushed the forceConsistentFileNames branch from adfa25e to 78c42a7 Compare May 6, 2024 18:15

Instead of checking each reason, check if references in file are alre…

4e58b5f

…ady processed

sheetalkamat force-pushed the forceConsistentFileNames branch from 78c42a7 to 4e58b5f Compare May 6, 2024 18:42

sheetalkamat requested a review from jakebailey May 6, 2024 19:22

jakebailey approved these changes May 6, 2024

View reviewed changes

sheetalkamat merged commit fd81d04 into main May 6, 2024
28 checks passed

sheetalkamat deleted the forceConsistentFileNames branch May 6, 2024 23:30

stackyism mentioned this pull request Jun 18, 2024

Typescript process is consuming much memory and preventing suggestions at editor #58820

Closed

sheetalkamat mentioned this pull request Jun 28, 2024

Delay the calculation of common source root if it would be needed when calculation dts files #59070

Merged

zachstence mentioned this pull request Jul 30, 2024

Out of memory during build evidence-dev/evidence#2282

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Lazy calculation of expensive file explaining diagnsotics and some caching to be used to share the diagnostic data #58398

Lazy calculation of expensive file explaining diagnsotics and some caching to be used to share the diagnostic data #58398

sheetalkamat commented May 1, 2024 •

edited

Loading

sheetalkamat commented May 2, 2024

typescript-bot commented May 2, 2024 •

edited

Loading

typescript-bot commented May 2, 2024 •

edited

Loading

jakebailey left a comment

kronodeus commented Jun 28, 2024 •

edited

Loading

kronodeus commented Jun 28, 2024

sheetalkamat commented Jun 28, 2024

sheetalkamat commented Jun 28, 2024

Lazy calculation of expensive file explaining diagnsotics and some caching to be used to share the diagnostic data #58398

Lazy calculation of expensive file explaining diagnsotics and some caching to be used to share the diagnostic data #58398

Conversation

sheetalkamat commented May 1, 2024 • edited Loading

sheetalkamat commented May 2, 2024

typescript-bot commented May 2, 2024 • edited Loading

typescript-bot commented May 2, 2024 • edited Loading

jakebailey left a comment

Choose a reason for hiding this comment

kronodeus commented Jun 28, 2024 • edited Loading

kronodeus commented Jun 28, 2024

sheetalkamat commented Jun 28, 2024

sheetalkamat commented Jun 28, 2024

sheetalkamat commented May 1, 2024 •

edited

Loading

typescript-bot commented May 2, 2024 •

edited

Loading

typescript-bot commented May 2, 2024 •

edited

Loading

kronodeus commented Jun 28, 2024 •

edited

Loading