Skip to content

Conversation

@SevereOverfl0w
Copy link
Contributor

For users using an "autocomplete" package of some kind (so completion on
every key press), a slow completion function adds noticeable lag to
typing. The use of searchpairpos really slows down the current context
implementation.

Since nvim-0.5, there's a treesitter package included. Treesitter can
incrementally parse the tree, and offers a very fast way to find the
root node.

I've benchmarked before & after using the clip library and get_complete_context goes from 6.244431 to 0.027365 self time (I will include the profile in a comment, as I cannot upload a file to a PR).

I've implemented this such that existing vim users will experience no degradation, that's required gymnastics in defining s:get_complete_context. To keep s:get_complete_context very fast, I've avoided conditionals which won't change while vim is running.

@SevereOverfl0w
Copy link
Contributor Author

SevereOverfl0w commented Oct 16, 2021

profile.zip

Apparently I could, but for some reason there's no upload button! Uploaded here anyway.

The benchmark script I used was:

34
normal! o

for s:item in split(';; sjdkfj kfjksdjfk jksdfjksdjf djksfjksdjfksjdf jsdkfjskdfjksd jfksdjf ksdjfksdjf sdjfksd fksjdfk jdsfkjd', '\zs')
    call feedkeys('a' . s:item . "\<C-x>\<C-o>", 'tx')
    sleep 50m
endfor

And to run it I opened vim and did:

profile start /tmp/filename
profile func *complete*
source benchmark.vim
profile stop

The function I tested this within is 54 lines in total, within a file that is 346 lines long. I originally noticed this problem in a file that was ~500 lines long when editing a function of around ~100 lines towards the end of the file.

@tpope
Copy link
Owner

tpope commented Oct 16, 2021

First question: Why autoload/_fireplace.lua/require'_fireplace'? Is this a convention?

@SevereOverfl0w
Copy link
Contributor Author

There's no way to mark code "private" in lua, _ is a convention for that (similar to python).

@tpope
Copy link
Owner

tpope commented Oct 17, 2021

That answers half the question. Why autoload/? It's not autoloaded. And Neovim seems to prefer lua/.

@SevereOverfl0w
Copy link
Contributor Author

@tpope It is in lua, I assumed you typo'd autoload. Apologies.

@tpope
Copy link
Owner

tpope commented Oct 17, 2021

My bad, could have sworn I saw autoload/_fireplace.lua in the diff.

If the function changes, how do I reload it? I think ideally, reloading autoload/fireplace.vim would also reload the lua file.

Can we go ahead and structure this in a way that isn't limited to a single function?

@SevereOverfl0w
Copy link
Contributor Author

I can make that change, no problem!

https://github.com/nanotee/nvim-lua-guide has tips for developing against nvim + lua. https://github.com/nanotee/nvim-lua-guide#reloading-cached-modules explains reloading. I was just restarting nvim personally, it's fast!

@SevereOverfl0w
Copy link
Contributor Author

I've made that change. I've also fixed a bug I noticed where if you started a new form the treesitter AST needs to be updated. I've re-run the benchmark, and the numbers are approximately the same.

@SevereOverfl0w SevereOverfl0w force-pushed the faster-complete branch 2 times, most recently from 66172c9 to 034e39f Compare October 27, 2021 19:27
@SevereOverfl0w SevereOverfl0w requested a review from tpope October 27, 2021 19:27
For users using an "autocomplete" package of some kind (so completion on
every key press), a slow completion function adds noticeable lag to
typing.  The use of searchpairpos really slows down the current context
implementation.

Since nvim-0.5, there's a treesitter package included.  Treesitter can
incrementally parse the tree, and offers a very fast way to find the
root node.
Comment on lines +196 to +223
" Find toplevel form
" If cursor is on start parenthesis we don't want to find the form
" If cursor is on end parenthesis we want to find the form
let [line1, col1] = searchpairpos('(', '', ')', 'Wrnb', g:fireplace#skip)
let [line2, col2] = searchpairpos('(', '', ')', 'Wrnc', g:fireplace#skip)

if (line1 == 0 && col1 == 0) || (line2 == 0 && col2 == 0)
return ""
endif
if (line1 == 0 && col1 == 0) || (line2 == 0 && col2 == 0)
return ""
endif

if line1 == line2
let expr = getline(line1)[col1-1 : col2-1]
else
let expr = getline(line1)[col1-1 : -1] . ' '
\ . join(getline(line1+1, line2-1), ' ')
\ . getline(line2)[0 : col2-1]
endif
if line1 == line2
let expr = getline(line1)[col1-1 : col2-1]
else
let expr = getline(line1)[col1-1 : -1] . ' '
\ . join(getline(line1+1, line2-1), ' ')
\ . getline(line2)[0 : col2-1]
endif

" Calculate the position of cursor inside the expr
if line1 == line('.')
let p = col('.') - col1
else
let p = strlen(getline(line1)[col1-1 : -1])
\ + strlen(join(getline(line1 + 1, line('.') - 1), ' '))
\ + col('.')
endif
" Calculate the position of cursor inside the expr
if line1 == line('.')
let p = col('.') - col1
else
let p = strlen(getline(line1)[col1-1 : -1])
\ + strlen(join(getline(line1 + 1, line('.') - 1), ' '))
\ + col('.')
endif

return strpart(expr, 0, p) . ' __prefix__ ' . strpart(expr, p)
return strpart(expr, 0, p) . ' __prefix__ ' . strpart(expr, p)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like you botched the indent here.

Comment on lines +23 to +24
local nrow1,ncol1,nline2,ncol2 = root_node:range()
local crow,ccol = unpack(vim.api.nvim_win_get_cursor(0))
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you give these commas some breathing room, along with the = up top?

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.

2 participants