-
Notifications
You must be signed in to change notification settings - Fork 42
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
Extremely expensive highlighting patterns #240
Comments
Yes, this parser is definitely at the high end for query parsing. I'd previously investigated this (and found improvements) in tree-sitter/tree-sitter#1578 and tree-sitter/tree-sitter#1589, but haven't revisited to see if there are simplifications to the grammar. The Go parser isn't an entirely fair comparison, I think, because that Thank you for isolating it down to just those patterns -- this makes me wonder if it has something to do with the tricks I do with aliased nodes to disambiguate function calls properly in the presence of all of the other language constructs. I will try to find some time to spin up my benchmarks again and see what can be optimized. Out of curiosity, why are you using a build with compiler optimizations turned off? How commonly do you do this, and is it feasible to simply use the build with compiler optimizations? |
Holy hell the analysis and work you put into those optimization bugs is amazing. I cannot take credit for finding those particular patterns. I just suggested to someone else they start commenting out patterns to see if some are to blame, and they found them. Soooo, if you incorporate a parser using SPM, it inherits a lot of the build settings for whatever your final target will be. If you use this in a standard Xcode environment, it will be debug (with zero optimizations) for debug builds. This is not ideal, I know. Pre-built binaries make sense for many reasons, so I'm trying to make that happen now. But, it's a lower-priority thing for me at the moment. |
Actually, it just occurred to me that the ABI 14 improvements aren't going to kick in here, since |
That would be awesome! |
Ok, I found one that is possibly an infinite loop in
Any ideas? It's related to the "body" pattern. Removing that allows this to terminate. |
It wasn't really fair to put this on you anyways. I've opened an issue with the runtime, and I should now experiment to see if ABI 13 makes a difference. But, not sure I can get to that in the near-term. |
Whether or not it's fair, it's certainly an interesting problem :) Interestingly, my desktop can run To me there are two questions:
|
Huh. Ok, well it's great to know that it isn't an ABI regression. |
Hey @alex-pinkus, thanks for sharing this project 🙏 I was wondering if there's an update on this issue? I came here when I noticed that highlighting a tiny Swift script takes 1.52s on my machine, whereas highlighting a similarly sized Python script takes 0.02s. (I am by no means an expert on parsing and syntax highlighting, so I hope this is not an unfair comparison.) |
@augustwester Does highlighting take that long, or is it preparing the highlighting query? This bug is about the latter. Also, are you running in Debug? Tree-sitter's parsers and runtime are extremely sensitive to compiler optimizations. |
@mattmassicotte It's building the query! I'm new to tree-sitter, so I wouldn't know how to verify this for the tree-sitter CLI, but I'm currently using SwiftTreeSitter in my app and—from there—have noticed that building the query is very slow (upwards of 8s compared to 7ms for tree-sitter-go). I'm not sure which optimization level I'm using when running tree-sitter-swift from the tree-sitter CLI. (How do I check this?) All I can tell you is that setting the Clang optimization flag to |
@augustwester Ok, so then, yes, this is what this bug is all about. I'm the author of SwiftTreeSitter - glad you are finding it useful! One thing I do want to do is update to the very latest tree-sitter release in SwiftTreeSitter. I don't have a good reason to believe that will address this, but I also don't want to fall to far behind and who knows? SPM packages build with the same level as your final target. So if you are just running your app, it's almost certainly debug. I'm actually not that familiar with the CLI, so I don't actually know what it's doing or how it works. But, I've found I need to modify some of the contents of highlights.scm to reduce the preparation time to something reasonable. The details for that are up above. But, you should still be prepared for query prep to be a slow process, I'm afraid. Go isn't a great comparison because the grammar is vastly simpler. |
@mattmassicotte That's awesome, I'm finding it very useful 👍 Fair enough about Go, I don't really have much insight into what makes a grammar complex or not. Maybe C# would be a better comparison? In that case, SwiftTreeSitter builds the query in 1.2s vs. 8s for Swift. Anyway, I don't wanna beat a dead horse here. Just wanted to follow up, since tree-sitter-swift is an outlier in this respect. |
Definitely worth following up. This class of problems (expensive query prep time) is something I'd really love for the tree-sitter maintainers to take a look at. tree-sitter/tree-sitter#1916 is the bug I filed there. |
I have just started learnig Swift and I have been setting it up with Neovim. Is there work around for this? |
This bug is really about the initial query processing - a one-time cost. So, if you are seeing a long initial delay before indentation calculations work, then yes, this could be the issue. But if you are seeing persistent lag even after correct behavior this is probably not related. |
It’s hard to describe exactly what the lag feels like but the cases I notice it are a. When first opening a swift file is the longest. Even with a file only 134 lines but takes a good 500ms+ to open. Interesting when scroling in switching back to the buffer from another buffer it feels fine. A note I have quite a few plugins that use treesitter including indent-blankline, nvim-ufo, guess-indent, nvim-treesitter-textobjects. I really do prefer the highlighting (and other features) I get when using treesitter so I am really hoping there is something that can be done to fix this. |
A could be caused by this issue. I don't think any of the other phenomena could. But, I have found that tree-sitter also has many other performance issues which are parser-dependent. Simple grammars go faster, and Swift is definitely not a simple grammar. Regardless, these are definitely issues on the tree-sitter runtime side. |
Intersting I was able to run Update! Got it without a autocmd. Setting up nvim-treesitter with this config gits it disabled only on swift. File open times are sitll a bit slow but at least the lag is gone when editing. Amazing!
|
Ok so then this is entirely related to the performance of the indentation system. Would take more work to understand where the problem is coming from, but I think we can be pretty confident it is not related to this bug. |
Just wondering, has there been any progress on this issue? Disabling indenting fixed the issue of there being lag on a n initial newline entry, but if I say for example hold down enter, it lags a lot when entering newlines, even with syntax and indenting turned off. However with tree-sitter-swift totally uninstalled it's extremely smooth |
No. But, I'm pretty sure we have established that this issue isn't at all a factor in poor indentation performance. My gut is that this is a problem within the client editor itself. |
In general, swift's highlights.scm is really expensive for the tree-sitter runtime to validate. It takes many seconds with compiler optimizations off. In comparison, Go's highlights, which are comparable in complexity, to my eyes anyways, take less than 1 second.
There are two patterns in particular that are bad.
(call_expression ; foo.bar.baz(): highlight the baz() (navigation_expression (navigation_suffix (simple_identifier) @function)))
I'm not sure I understand why, particularly the second. But, I figured I'd open an issue because this parser's highlights are such an outliner. Perhaps it has something to do with the grammar?
The text was updated successfully, but these errors were encountered: