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
Skip parsing JSDoc in .ts files #52466
base: main
Are you sure you want to change the base?
Conversation
Just an experiment for now. Current test failures: - all fourslash tests of jsdoc fail; the parser should produce jsdoc for tsserver. - Spurious unused-type errors because `@link` isn't parsed and treated as a usage. - ohhh no, `@this` is mistakenly interpreted in .ts files: thisInFunctionCall.
@typescript-bot perf test this |
Heya @sandersn, I've started to run the perf test suite on this PR at 5d585f6. You can monitor the build here. Update: The results are in! |
@sandersn Here they are:
CompilerComparison Report - main..52466
System
Hosts
Scenarios
TSServerComparison Report - main..52466
System
Hosts
Scenarios
StartupComparison Report - main..52466
System
Hosts
Scenarios
Developer Information: |
Holy moly, we spend that much time on JSDoc? |
Yes |
Yep. Yes indeed. |
also delete comment in checker, that's for a different experiment
@typescript-bot perf test this |
Heya @sandersn, I've started to run the perf test suite on this PR at 3d5d2a6. You can monitor the build here. Update: The results are in! |
This looks great, hah. Do the parser time savings in our benches correspond, percentage-wise, with how much text is jsdoc, or does jsdoc have an outsized parse time impact? I ask because, if the later, it may still be worth also looking at if we can make jsdoc parsing faster for the sake of LS operations that actually need the jsdoc. |
Uhhh, not really? Correlation is 27%, which is not high, although xstate is kind of an outlier and if you drop it the correlation is 66%. It's definitely worth investigating jsdoc parsing. Edit: There are only 50,000 characters of comments in xstate so I looked at all of them. There's nothing interesting there, unless we are pathologically bad at parsing the one or two markdown links. I think it's the perf tests being wonky, so let's see what the second run results are like. |
@sandersn Here they are:
CompilerComparison Report - main..52466
System
Hosts
Scenarios
TSServerComparison Report - main..52466
System
Hosts
Scenarios
StartupComparison Report - main..52466
System
Hosts
Scenarios
Developer Information: |
@weswigham maybe you're right about the isTsserver check. The speedup is gone in the commit with the suspicious version of the check. |
@typescript-bot perf test this |
Heya @sandersn, I've started to run the perf test suite on this PR at ebcbd11. You can monitor the build here. Update: The results are in! |
@sandersn Here they are:
CompilerComparison Report - main..52466
System
Hosts
Scenarios
TSServerComparison Report - main..52466
System
Hosts
Scenarios
StartupComparison Report - main..52466
System
Hosts
Scenarios
Developer Information: |
Just an experiment for now. JSDoc is still parsed
@link
or@see
An alternative would make the parser parse lazily, and instead have the checker skip checking in the above conditions. After thinking about it, I'm not sure that's much better.