neovim icon indicating copy to clipboard operation
neovim copied to clipboard

feat(treesitter): async parsing

Open lewis6991 opened this issue 1 year ago • 9 comments

Idea

Use TSParser:set_timeout to set the maximum time to parse each event-loop iteration. If the timeout is reach, parsing will be deferred and resumed at a later point in the event loop. This should result in less editor blocking and lag, especially during the initial load of a buffer.

  • TSParser:parse() will come in two variants:

    TSParser:parse(opts) -- synchronous, returns trees and changes directly
    TSParser:parse(opts, callback) -- asynchronous, returns trees and changes as args to the callback
    

To enable:

vim.api.nvim_create_autocmd('FileType', {
  callback = function()
    local lang = vim.treesitter.language.get_lang(vim.bo.filetype)
    if lang and pcall(vim.treesitter.language.add, lang) then
      -- vim.treesitter.start()  -- sync
      vim.treesitter.start(nil, nil, { timeout = 1 }) -- async
    end
  end,
})

Notes

  • As the synchronous version of parse will still be supported, this'll mean any plugins calling the synchronous version will now block the UI instead of highlighter, so they will all have to adapt to the async version in order to not negate the benefits of this PR.

  • How does 'redrawtime' fit into this? Or does it not?

  • How do we handle subsequent calls to parse whilst one is in-progress? For subsequent calls, don't run the parse again, but save the callback and call it when the currently running parse completes. If a synchronous parse is requested, we cancel the async one in flight.

lewis6991 avatar Feb 26 '23 17:02 lewis6991

This is great, does it still also make sense to max out at 'redrawtime'? #19812

justinmk avatar Feb 27 '23 13:02 justinmk

This is great, does it still also make sense to max out at 'redrawtime'? https://github.com/neovim/neovim/pull/19812

There will be two timeouts to consider:

  1. The maximum segment time of a parse.
  2. The total time of the parse.

I think 'redrawtime' correlates to 1) as this is the time we will block for. Atm I've left 2) unbound so that parse is always guaranteed to return a result.

With that said, the default value of 'redrawtime' is 2 seconds, so it might make more sense to link that to 2).

Or maybe conflating this with 'redrawtime' isn't the right choice?

lewis6991 avatar Feb 27 '23 13:02 lewis6991

'redrawtime' is 2 seconds, so it might make more sense to link that to 2).

That's what I was thinking... the total time should have an upper limit, it's equivalent to the user hitting ctrl-c.

justinmk avatar Feb 27 '23 14:02 justinmk

Status update

This PR sort of works as is, however I've found that Treesitter's parser timeout doesn't work when trying to parse the big linux file.

I'm fairly confident this is a Treesitter issue, but in order to get any insight into what's happening I've raised #23638 so we can see Treesitters internal logging.

lewis6991 avatar May 15 '23 14:05 lewis6991

The treesitter logging PR got merged some time ago.

Did anyone managed to distill some more info from the logs shedding some light on the issue with the big linux file?

dumblob avatar Jun 29 '23 19:06 dumblob

If we have a function like Qt's processEvents, then we can process events when parse file. It should be simplifer?

cathaysia avatar Apr 24 '24 04:04 cathaysia

I'm fairly confident this is a Treesitter issue, but in order to get any insight into what's happening I've raised https://github.com/neovim/neovim/pull/23638 so we can see Treesitters internal logging.

Now that this is merged, is there any additional insight?

Regarding

There will be two timeouts to consider:

  1. The maximum segment time of a parse.
  2. The total time of the parse.

I agree that a) 1. is the most important to tackle because it directly ties to input latency, b) redrawtime is appropriate for 2. and not 1., and c) 2. may become moot if 1. is implemented (when would you not want to finish a parse if it is "free" (not blocking)?)

Unfortunately, b) implies that we may need a new option (unless we can find a objectively optimal value -- good luck...). My suggestion would be to not make this a global option but expose this through a new {opts} table to vim.treesitter.start (not necessarily in this PR).

clason avatar May 25 '24 10:05 clason

Hi @lewis6991, if it helps any I spent some time rebasing this on latest master at https://github.com/ribru17/neovim/tree/async_parsing_rebase. It still doesn't seem like all edge cases are handled but I believe the rebase has fullest fidelity to the original changes (there were a lot of conflicts).

ribru17 avatar Jul 27 '24 22:07 ribru17

Thanks, I've pulled in those changed.

FYI I have no immediate plans to continue this, so feel free to pick this up if you want to.

lewis6991 avatar Jul 29 '24 09:07 lewis6991