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

feat: cache textobjects created from a ts query #692

Closed
wants to merge 1 commit into from

Conversation

gepbird
Copy link

@gepbird gepbird commented Sep 23, 2024

Problem

When the user opens a file with this plugin installed (most notably latex, dart, zig and rust files), a slow ts.get_query will get called, that can take 3500ms for a latex file on an older machine. The reason for this is tree-sitter is trying to do some aggressive initial analysis of the query (eg. latex/textobjects.scm).

Neovim acknowledged that this function call is slow and memoized it, however it doesn't get cached for too long, as the cache gets garbage collected. For me this cache gets cleared after opening 2-3 files, so it's not that useful.

This PR's solution

This PR puts computed queries in a cache which lives as long as neovim is open.

Further work

A proper upstream fix would be the best, but this performance issue is present for months and I don't see any progress on it.
In order to persist the cache between neovim sessions, we could store the cache somewhere in probably ~/.local/state/nvim. And for cache invalidation, we could also store the hash of the .scm files and invalidate the cache when the hash changes.

Other issues and PRs

Helps a lot with #461, #627.
Upstream issue this PR is working around: tree-sitter/tree-sitter#973.
In neovim a similar caching system was implemented in neovim/neovim#24222, but got replaced with the garbage collected memoization in neovim/neovim#25201.
Other plugins are resorting to this kind of workaround, for example noice.

@clason
Copy link
Collaborator

clason commented Sep 23, 2024

Thank you, but the master branch is basically frozen; any development now happens on the main branch which will become the default (and which already contains caching).

@clason
Copy link
Collaborator

clason commented Sep 23, 2024

(And if you add a cache, you need to make sure it gets invalidated on changes!)

@clason clason closed this Sep 23, 2024
@gepbird
Copy link
Author

gepbird commented Sep 23, 2024

@clason Thanks for the info. I just tried loading a dart file on main and it's instant, even for the first try, very impressive work! Latex may be a little slower if not the same speed, or at least I can feel the sometimes fast, sometimes slower loading that I noticed with neovim's garbage collected memoize.

How stable is the main branch? Since it's not the default I assume most users and distro package maintainers shouldn't switch yet, is there some tracking issue?

(Since AFAIK the query won't change during runtime, I deliberately didn't invalidate it.)

@clason
Copy link
Collaborator

clason commented Sep 24, 2024

It's not stable yet; there's some regressions and some more work to be done that could be breaking (see open PRs labeled NEXT). No ETA but we hope those will get resolved in a timely manner. It's volunteer work, though, so impossible to plan.

@gepbird gepbird deleted the get-query-cache branch September 30, 2024 06:25
@ribru17
Copy link
Member

ribru17 commented Nov 23, 2024

Note that this will likely be helped greatly by neovim/neovim#30227, as it will allow the query.get memoization to use a strong table and be manually invalidated only when necessary (e.g. after query.set)

@gepbird
Copy link
Author

gepbird commented Nov 23, 2024

Thanks for your work in upstream and linking it here, that will also help other plugins :)

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.

3 participants