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

Treesitter performance (tracking issue) #22426

Open
2 of 9 tasks
lewis6991 opened this issue Feb 27, 2023 · 10 comments
Open
2 of 9 tasks

Treesitter performance (tracking issue) #22426

lewis6991 opened this issue Feb 27, 2023 · 10 comments
Labels
performance issues reporting performance problems treesitter

Comments

@lewis6991
Copy link
Member

lewis6991 commented Feb 27, 2023

Problem

Our treesitter implementation could be more efficient to overcome many known issues with performance. Specifically around large files.



Other

Files for benchmarking:

@lewis6991 lewis6991 added bug issues reporting wrong behavior performance issues reporting performance problems treesitter and removed bug issues reporting wrong behavior labels Feb 27, 2023
@clason
Copy link
Member

clason commented Feb 27, 2023

Another issue is incremental querying, which may help with expensive injection predicates as in nvim-treesitter/nvim-treesitter#2996?

@lewis6991
Copy link
Member Author

lewis6991 commented Feb 27, 2023

This will be partially resolved by just running the queries less in #22394.

@clason
Copy link
Member

clason commented Mar 8, 2023

"Bad" files for performance testing: https://raw.githubusercontent.com/torvalds/linux/master/drivers/gpu/drm/amd/include/asic_reg/bif/bif_5_1_sh_mask.h
https://raw.githubusercontent.com/torvalds/linux/master/drivers/gpu/drm/amd/include/asic_reg/dcn/dcn_3_2_0_sh_mask.h

(cpp header files consisting of, respectively, 33k and 220k injections of cpp into preprocessor macros)

@clason

This comment was marked as resolved.

@lewis6991
Copy link
Member Author

lewis6991 commented Aug 12, 2023

Idea from chat: Highlighter should not parse off-screen injections (should help with those files).

#24647 is now merged which should improve the performance of large files with lots of injections.

@MKrbm

This comment has been minimized.

@tomtomjhj
Copy link
Sponsor Contributor

Partially run the query for rendered lines.

This would require separate queries for non-combined injection and combined injection (or some other significant API changes). Combined injection query should be run on the entire source because the invisble part of the region affects the visible part, just like the root language. So the partial query execution (using start and stop params of iter_matches) should be applied only to non-combined injections. However, currently there is no way to iterate only the matches that have some specific metadata. The simpliest solution I can think of is to have two separate query files for combined and non-combined ones.

@lewis6991
Copy link
Member Author

lewis6991 commented Dec 25, 2023

Non-combined is the norm, so if combined is detected such optimisations should just be disabled.

I think adding additional states could over complicate things for not a lot of return and running the query twice would just be worse over all.

@tomtomjhj
Copy link
Sponsor Contributor

tomtomjhj commented Dec 25, 2023

Partially run the query for rendered lines.

Another issue with this is that it's difficult to make injected region parsing incremental. This requires some sort of persistent ID for each region and the corresponding tree so that the old tree can be used when parsing the corresponding region. The current implementation of LanguageTree simply uses the index of the region in the whole list of regions captured by the last execution of the injection query, which is frequently invalidated. Unfortunately, treesitter doesn't seem to provide such a functionality. Using the equality of the region value could be an option, but that looks inefficient.

Even if there were some ID mechanism that allows incremental parsing of injected regions, I think it would be still inefficient because all those regions should be adjusted after each edit. So the parser would need to discard trees (e.g., with LRU cache) to balance the cost of maintaining the existing trees and parsing the new regions from scratch.

Update: see #26827

@lewis6991
Copy link
Member Author

lewis6991 commented Dec 25, 2023

Helix apparently already does this so I suggest you look at their implementation.

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

No branches or pull requests

4 participants