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(treesitter): async parsing #22420

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

lewis6991
Copy link
Member

@lewis6991 lewis6991 commented Feb 26, 2023

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.

@justinmk
Copy link
Member

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

@lewis6991
Copy link
Member Author

lewis6991 commented Feb 27, 2023

This is great, does it still also make sense to max out at 'redrawtime'? #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?

@justinmk
Copy link
Member

'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.

@lewis6991 lewis6991 force-pushed the feat/async_parsing branch 16 times, most recently from 7f758f3 to 4d78b73 Compare February 28, 2023 15:12
@lewis6991 lewis6991 force-pushed the feat/async_parsing branch 8 times, most recently from 5832f83 to f3194d2 Compare April 30, 2023 14:48
@lewis6991 lewis6991 force-pushed the feat/async_parsing branch 7 times, most recently from dde64ff to 2701b96 Compare May 1, 2023 09:33
@lewis6991 lewis6991 force-pushed the feat/async_parsing branch 3 times, most recently from dc2c9d9 to 6387ddd Compare May 8, 2023 09:50
@lewis6991
Copy link
Member Author

lewis6991 commented May 15, 2023

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.

@dumblob

This comment was marked as off-topic.

@cathaysia

This comment has been minimized.

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

Successfully merging this pull request may close these issues.

None yet

8 participants