Add JSDoc parsing#225
Conversation
(As far as I can tell)
|
Performance: (numbers below) In the normal mode, empty.ts is 17.7% slower, checker is the same, dom.generated.d.ts is 1.3% faster and Herebyfile.mjs is 0.4% slower. None of these are actually parsing JSDoc, so I don't know why empty.ts is so much slower. Maybe it's overhead from initialising the jsdocCache. In always-parse mode, everything is slower. The interesting ones are the large ones: checker.ts and dom.generated.d.ts. checker.ts doesn't have much jsdoc and is only 2% slower. The DOM has JSDoc on nearly every declaration and is 58% slower. My initial opinion is that this isn't worth optimising now because the language service isn't ready; the common case is that jsdoc isn't parsed at all, and parsing is a small part of take taken to boot. In the normal mode: In the old-style, always-parse mode: I used the command |
|
Indeed making jsdocCache initialisation lazy make empty.ts only slow down by 2.4% and use 4.9% more memory. |
|
Retesting but setting But, |
Also 1. Default to JSDocParsingModeAll everywhere else. 2. Change tests to explicitly use JSDocParsingModeAll. 2. Delete unused parseSourceFile wrapper in program.go.
|
I think 4% cost is reasonable for JSDoc parsing -- at least initially. I have some non-backward-compatible ideas for how to make it faster, but it's already cheaper than JSDoc parsing in Strada. |
| result.ScriptKind = p.scriptKind | ||
| result.Flags |= p.sourceFlags | ||
| result.SetJSDocCache(p.jsdocCache) | ||
| p.jsdocCache = nil |
There was a problem hiding this comment.
I would remove this; duplicated above and not related to the source file itself, but the memory retention of the Parser.
| result.LanguageVersion = p.languageVersion | ||
| result.LanguageVariant = p.languageVariant | ||
| result.ScriptKind = p.scriptKind | ||
| result.Flags |= p.sourceFlags |
There was a problem hiding this comment.
Is there a chance that the source flags differ when we reparse for await?
|
Nathan and I looked at this on a call; there were a couple of bugs (not caused by this PR) that prevented things from working, notably that we never set the ScriptKind on the scanner. That plus some performance improvements leave us at: aka no difference for a tsc run, so that's great. I think there is sill some performance to be gained when parsing JSDoc but that's not strictly required now that we got over the hurdle of a plain tsc run. |
| return token == ast.KindGreaterThanToken || tokenIsIdentifierOrKeyword(token) | ||
| } | ||
|
|
||
| func getJSDocCommentRanges(f *ast.NodeFactory, commentRanges []ast.CommentRange, node *ast.Node, text string) []ast.CommentRange { |
There was a problem hiding this comment.
Note: this would be better as an iterator, but it's not performant enough until Go 1.24 is released.
|
A quick run of TS 5.7.3 on dom.generated.ts shows a 30% slowdown between ParseNone and ParseAll. I don't think we measured that number after the last round of performance fixes, but that's an initial target at least. That's in a file where 49% of characters are in JSDoc. I didn't measure the increase in number of nodes. I'm going to merge this now and re-run my AST tests afterwards -- the overhead of branch management is way lower that way. |
|
AST tests are still good. |
2 participants
This PR adds JSDoc parsing. It builds on my previous PRs so it doesn't touch a lot of files for such a large feature. Specifically, the main AST still doesn't have a jsdoc property. Instead, the parser has a
jsdocCachemap that uses node as keys for jsdoc.Like Strada,
@seeor@link.jsdocDiagnostics, so that JS files without checkJS can ignore them.Some notes:
@seeor@link.parseJSDocTypeNameWithNamespace. In Corsa, it always returns a name, whereas in Strada it could return undefined, sincetypedef/enum/callback's names were optional.@immediateproposal in 2024.@typedef. I copied the Strada behaviour to Corsa even though it's wrong.scanner *Scanner -> s *Scannerin methods, sorry.I'll get some performance numbers and post them shortly. I don't expect this to be cheap because the fundamental algorithm hasn't changed from Strada.