Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 9 commits into from May 6, 2024

Conversation

sheetalkamat
Copy link
Member

@sheetalkamat sheetalkamat commented May 1, 2024

  • 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%

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

@typescript-bot pack this

@typescript-bot
Copy link
Collaborator

typescript-bot commented May 2, 2024

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

Command Status Results
pack this ✅ Started ✅ Results

@typescript-bot
Copy link
Collaborator

typescript-bot commented May 2, 2024

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/[email protected]".;

Copy link
Member

@jakebailey jakebailey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Behavior seems correct, but a few small comments.

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
@sheetalkamat sheetalkamat merged commit fd81d04 into main May 6, 2024
28 checks passed
@sheetalkamat sheetalkamat deleted the forceConsistentFileNames branch May 6, 2024 23:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug
Projects
None yet
3 participants