Skip to content
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

Tree-sitter Performance #43

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

WillLillis
Copy link
Contributor

@WillLillis WillLillis commented Jan 22, 2024

Here's a first pass on the tree-sitter performance changes I asked about in #39. I brought in the lsp_textdocument crate to help manage in-memory copies of the buffers being edited. Using this crate, text edits can now be communicated and applied incrementally, as opposed to copying over the entire document every time a change is made. I also co-located a tree-sitter tree in the global text store map so that each document now has a corresponding tree. With this in place, the trees can now be maintained incrementally alongside the in-memory buffers. Edits are applied eagerly to each tree (inside handle_didChange()), but the tree is only re-parsed lazily as needed (in get_position_from_lsp_completion()).

I also swapped out the protocol-related structs defined in lsp/src/handle.rs with their equivalents from the lsp_types crate, as the crate's structs have some extra information by more closely matching the lsp spec. Some of this extra info was needed to interface with the lsp_textdocument crate, but using the crate's types should also make things easier to maintain overall.

I made some assumptions about how this should be implemented, so if there needs to be any changes/ refactoring I'm happy to work through that :)

Apologies that this PR is a bit on the long side, but the tree-sitter changes had to touch a couple different parts of the code base.

Closes #39

@ThePrimeagen
Copy link
Owner

i am scared to merge this :)

i'll have to pull it into my editor and really consider this

@ThePrimeagen
Copy link
Owner

also, merge confwicts pweeze

@WillLillis
Copy link
Contributor Author

WillLillis commented Feb 11, 2024

Many complexity spirit demon maybe hiding in this PR, but Tom (a genius, btw) said LGTM :)

In all seriousness though, if this isn't it I'm happy to refactor/ start fresh. If it would help to break this into smaller PRs that's fine too. Really appreciate you taking a look!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tree-sitter perf
2 participants