-
Notifications
You must be signed in to change notification settings - Fork 12.6k
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
Improve the performance of isolatedDeclarations quickfix #58722
Conversation
It turns out that `typeChecker.getEmitResolver()` can be expensive, so avoid calling it unless necessary. Hopefully this fixes the performance issue (bullet microsoft#1) of microsoft#58426
These are not related to the perf issue, but I noticed them while editing this file
@microsoft-github-policy-service agree company="Google" |
This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise. |
Weird, how does this end up being expensive? Is it recalculating diagnostics? Something else? Do you have a profile before/after? You'll want to resend that CLA comment again, I don't think it understands edits. I also modified your PR description as the language it contained would have closed the linked issue (when it shouldn't in full yet). |
Yeah, I assume it's recalculating diagnostics, as the method called inside The full before profile is vscode-profile-2024-05-29-16-18-13.cpuprofile After this fix, the quickfix in my example projects didn't have perceptible latency, so I didn't try to grab an after profile. @microsoft-github-policy-service agree company="Google" |
Oops, I think I uploaded the wrong before profile. Darn reverse chronological. The useful ones are these three: |
Thank you @blickly for jumping on this 👍 |
* upstream/main: (37 commits) Added NoTruncation flag to completions (microsoft#58719) Clone node to remove location even when it has been modified if needed (microsoft#58706) Properly account for `this` argument in intersection apparent type caching (microsoft#58677) Fix: Include Values of Script Extensions for Unicode Property Value Expressions in Regular Expressions (microsoft#58615) In `reScanSlashToken` use `charCodeChecked` not `codePointChecked` (microsoft#58727) Shorten error spans for errors reported on constructor declarations (microsoft#58061) Mark file as skips typechecking if it contains ts-nocheck (microsoft#58593) Fixed an issue with broken `await using` declarations in `for of` loops (microsoft#56466) Do not expand type references in keyof and index access (microsoft#58715) Improve the performance of isolatedDeclarations quickfix (microsoft#58722) Unwrap `NoInfer` types when narrowing (microsoft#58292) Recover from type reuse errors by falling back to inferred type printing (microsoft#58720) Fixing self import (microsoft#58718) Enable JS emit for noCheck and noCheck for transpileModule (microsoft#58364) Revert PR 55371 (microsoft#58702) Update dependencies (microsoft#58639) Fix baselines after PR 58621 (microsoft#58705) Do not infer `yield*` type from contextual `TReturn` (microsoft#58621) `await using` normative changes (microsoft#58624) Handling statements from a known source file (microsoft#58679) ...
Thanks! I also wanted to to thank @h-joo for helping to answer many of my questions and get me up to speed on the codebase. |
It turns out that
typeChecker.getEmitResolver()
can be expensive, so avoid calling it unless necessary.Hopefully helps #58426, in particularly the performance issue (bullet 1)