-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Add resolveLibrary method on hosts and store resolvedLibraries in program so that resolutions can be reused #53877
Conversation
924030f
to
d7cfc01
Compare
d7cfc01
to
d171bb6
Compare
d171bb6
to
fc8c4da
Compare
@typescript-bot perf test this |
Heya @andrewbranch, I've started to run the perf test suite on this PR at fc8c4da. You can monitor the build here. Update: The results are in! |
@andrewbranch Here they are:
CompilerComparison Report - main..53877
System
Hosts
Scenarios
TSServerComparison Report - main..53877
System
Hosts
Scenarios
StartupComparison Report - main..53877
System
Hosts
Scenarios
Developer Information: |
IIRC module resolution is reported as part of parse time in the perf results? If so, looks like a win. |
@typescript-bot perf test faster |
Heya @jakebailey, I've started to run the abridged perf test suite on this PR at fc8c4da. You can monitor the build here. Update: The results are in! |
@andrewbranch @jakebailey do we want to report library resolution times in |
Also for now made the API internal and can make it public if really needed. |
@jakebailey Here they are:Comparison Report - main..53877
System
Hosts
Scenarios
Developer Information: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we want to report library resolution times in --extendedDiagnostics? like module resolution times and type ref resolution times. I thought it wouldnt be much and hence not needed but open to adding it if you think that would be helpful to track.
If it's easy, then I feel like it can't hurt, but, it might not be worth it, yeah.
With this PR:
resolvedLibraries
which contains the@typescript/libxxx
resolution and the actual file used as lib so that resolutions are not re-done just to answer if the file is "lib" (c5dc1a0 though this one is only maintaining the actual lib file used the next commit: fc8c4da modifies to store the resolution to so it can be reused)resolveModuleNameLiterals
orresolveTypeReferenceDirectiveReferences
we addresolveLibrary
method on all hosts and correspondinghasInvalidatedLibResolutions
to determine if lib resolution is valid. This enables reuse of the library resolution and caching it across program boundaries. This also fixes to use different module resolution cache for library resolutions since its the cache with different options than module resolution. fc8c4daFixes #52759
Fixes #52707