search

denoland / deno

A modern runtime for JavaScript and TypeScript.
https://deno.com
MIT License
96.64k stars 5.34k forks source link

lsp: replace code lens navigation tree with swc "collector" #11032

Open kitsonk opened 3 years ago

kitsonk commented 3 years ago

Currently, the TypeScript code lenses for implementations and references use the TypeScript "navigation tree", which is a light-weight AST, to identify positions in the code that might be "interesting" to lazily ask the TypeScript language server about at a future point.

Getting a navigation tree is one of the slower processes with the language server, and is several orders of magnitude slower than visiting an swc parsed AST. In fact, we actually don't currently cache/reuse an swc parsed AST for a file across the language server, reparsing a file a few times depending on the types of requests to a language server.

I recently refactored the code for the code lenses to make them a little bit more self contained. The logic for walking the "navigation tree" AST is in lsp::code_lens::collect_tsc and I implemented with test code lens using the "collector" pattern in lsp::code_lens::collect_test which is in my opinion the way we should refactor the TypeScript code lenses.

Also, caching the swc parse of the module in the document info would be a good thing I think. That might take a bit more structuring of code. It is used in dependency analysis of the document as well as the test code lens and also use by the linting (though I don't know how easy it would be to send it to deno_lint without a bigger refactor of one or the other).

This is the refactor I had in my mind @dsherret that I mentioned to you. Let me know if you want to work on it, if not, we can always add it to my backlog. 😄

kitsonk commented 2 years ago

@dsherret if you don't feel too strongly about this one, I would be interested in taking this one back up. I think it could dramatically improve the performance of the LSP.

dsherret commented 2 years ago

@kitsonk I was thinking about working on it in January/Feb after the bulk of the node compatibility improvements. Feel free to take a look sooner if you'd like.

dsherret commented 2 years ago

I've been looking at the LSP code more over the past few days and eliminating the NavigationTree would be very nice. Right now there's a race condition if we were to make the code concurrent in that an out of date navigation tree might end up being stored in a document that it wasn't created from. This could be eliminated by implementing this feature and it would reduce our mutation points.

kitsonk commented 2 years ago

To fix that race condition, we need to supply the LSP version on the call and have it come back in the response and abandon it. But the bigger win would be cancelling in flight requests, but that is a bit of a challenge.