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

Use CursorHold instead of Cursor Moved (performance) #126

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

Conversation

chr4
Copy link

@chr4 chr4 commented May 7, 2020

I've noticed massive performance issues when using CursorMoved, fixed it for me by switching to CursorHold.

@chr4 chr4 force-pushed the chore/use-cursor-hold branch from 6895de0 to e8920fc Compare May 7, 2020 08:52
@ap
Copy link
Owner

ap commented May 7, 2020

Thank you for the patch. This too, I’m afraid, I won’t be taking… 😐

The plugin used to use CursorHold a long time ago. I did a lot of work optimising it and limiting the amount of work that it does during updating, then switched it to CursorMoved. Using CursorHold doesn’t much fix the performance problem so much as just hide it, by making the plugin laggy to update. You scroll around and colours don’t show, then you settle down and shortly after they pop in. I hate that…

What size is your Vim window? What machine are you on?

@chr4
Copy link
Author

chr4 commented May 7, 2020

It makes a huge difference on my machine regarding startup time (I've tested this on macOS Catalina and Ubuntu 18.04, both with neovim):

Using --startuptime with bootstrap.css

diff move.txt hold.txt | grep vim-css
< 153.673  002.377  002.377: sourcing ~/.config/nvim/bundle/vim-css-color/autoload/css_color.vim
< 1891.508  000.814  000.814: sourcing ~/.config/nvim/bundle/vim-css-color/syntax/colornames/basic.vim
< 1894.072  004.576  003.762: sourcing ~/.config/nvim/bundle/vim-css-color/syntax/colornames/extended.vim
< 1894.431  1743.615  1736.661: sourcing ~/.config/nvim/bundle/vim-css-color/after/syntax/css.vim
> 088.510  001.061  001.061: sourcing ~/.config/nvim/bundle/vim-css-color/autoload/css_color.vim
> 089.201  000.318  000.318: sourcing ~/.config/nvim/bundle/vim-css-color/syntax/colornames/basic.vim
> 090.807  002.051  001.733: sourcing ~/.config/nvim/bundle/vim-css-color/syntax/colornames/extended.vim
> 091.068  003.743  000.631: sourcing ~/.config/nvim/bundle/vim-css-color/after/syntax/css.vim

Using this bench fix (tried just calculating in all vim-css-color files), the difference is this:

864.848 # CursorMoved
5.982   # CursorHold

I get your problem with hiding vs. fixing - at least for me the seconds long startup time felt unbearable, and I preferred the Hold approach, which I didn't really notice. But this is just personal taste I guess.

Edit: My vim window size is ~130x70 colums/lines.

@ap
Copy link
Owner

ap commented May 7, 2020

at least for me the seconds long startup time felt unbearable

Yeah, obviously. It’s very weird that CursorHold affects performance during sourcing, though. And in fact, for me, opening that file yields this:

138.197  002.699  002.699: sourcing /Users/ap/.vim/pack/bundle/start/css-color/autoload/css_color.vim
143.842  000.644  000.644: sourcing /Users/ap/.vim/pack/bundle/start/css-color/syntax/colornames/basic.vim
148.396  005.329  004.685: sourcing /Users/ap/.vim/pack/bundle/start/css-color/syntax/colornames/extended.vim
148.582  013.228  005.200: sourcing /Users/ap/.vim/pack/bundle/start/css-color/after/syntax/css.vim

I.e. effectively instant startup. Certainly nothing like what you are seeing… Thing is, I am using regular Vim 8.1 (on Mac). So… is NeoVim doing something really silly?

@chr4
Copy link
Author

chr4 commented May 7, 2020

It's a little better with vim 8.1 - but still factor 10 slower:

diff move.txt hold.txt | grep vim-css
< 075.024  001.508  001.508: sourcing ~/.config/nvim/bundle/vim-css-color/autoload/css_color.vim
< 852.235  000.129  000.129: sourcing ~/.config/nvim/bundle/vim-css-color/syntax/colornames/basic.vim
< 853.153  001.194  001.065: sourcing ~/.config/nvim/bundle/vim-css-color/syntax/colornames/extended.vim
< 853.316  779.958  777.256: sourcing ~/.config/nvim/bundle/vim-css-color/after/syntax/css.vim
> 082.854  001.904  001.904: sourcing ~/.config/nvim/bundle/vim-css-color/autoload/css_color.vim
> 083.737  000.205  000.205: sourcing ~/.config/nvim/bundle/vim-css-color/syntax/colornames/basic.vim
> 086.239  002.909  002.704: sourcing ~/.config/nvim/bundle/vim-css-color/syntax/colornames/extended.vim
> 086.515  005.791  000.978: sourcing ~/.config/nvim/bundle/vim-css-color/after/syntax/css.vim

Maybe it's some weird cross-issue with another plugin, I'll investigate that.

@chr4
Copy link
Author

chr4 commented May 7, 2020

Found something: This is dependent on the option set foldmethod=syntax. As soon as I comment out this in my config, it is fast again:

diff move.txt hold.txt | grep vim-css
< 085.125  001.085  001.085: sourcing ~/.config/nvim/bundle/vim-css-color/autoload/css_color.vim
< 086.513  000.154  000.154: sourcing ~/.config/nvim/bundle/vim-css-color/syntax/colornames/basic.vim
< 087.865  001.630  001.476: sourcing ~/.config/nvim/bundle/vim-css-color/syntax/colornames/extended.vim
< 088.069  004.161  001.446: sourcing ~/.config/nvim/bundle/vim-css-color/after/syntax/css.vim
> 083.431  001.043  001.043: sourcing ~/.config/nvim/bundle/vim-css-color/autoload/css_color.vim
> 084.026  000.231  000.231: sourcing ~/.config/nvim/bundle/vim-css-color/syntax/colornames/basic.vim
> 085.532  001.862  001.632: sourcing ~/.config/nvim/bundle/vim-css-color/syntax/colornames/extended.vim
> 085.732  003.473  000.567: sourcing ~/.config/nvim/bundle/vim-css-color/after/syntax/css.vim

@ap
Copy link
Owner

ap commented May 7, 2020

Oooh, yeah. That makes a giant difference for me as well.

And I can now also see that if your view of the file has almost everything folded away most of the time, then you wouldn’t care about the lagginess of using CursorHold – the stuff that would be highlighted laggily isn’t even visible 99% of the time, so who cares. So for you, CursorHold isn’t just a crutch, it’s a pretty reasonable solution. Hmm.

I want to see if it’s possible to fix this properly, though. It should be.

So. The first obvious issue is that when I open bootstrap.css under fdm=syntax, the topmost and bottommost lines of the file in my 60-line Vim windows are anywhere from ~150 to ~1350 lines apart across the entire file. The plugin has to do far more work on every cursor movement than my lines setting would seem to indicate. Maybe that’s all that needs fixing?

So this patch limits parsing to only unfolded lines:

diff --git i/autoload/css_color.vim w/autoload/css_color.vim
index f72d598352b..0cb673b9d48 100644
--- i/autoload/css_color.vim
+++ w/autoload/css_color.vim
@@ -224,7 +224,7 @@ function! s:parse_screen()
        let b:css_color_upd = upd
        let left = max([ wv.leftcol - 15, 0 ])
        let width = &columns * 4
-       call filter( range( line('w0'), line('w$') ), 'substitute( strpart( getline(v:val), col([v:val, left]), width ), b:css_color_pat, ''\=s:create_syn_match()'', ''g'' )' )
+       call filter( range( line('w0'), line('w$') ), '( 0 > foldclosed(v:val) ) && substitute( strpart( getline(v:val), col([v:val, left]), width ), b:css_color_pat, ''\=s:create_syn_match()'', ''g'' )' )
 endfunction

 """"""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""

This change alone makes a huge difference while scrolling around the file initially. So that’s good.

However, it doesn’t change the startup delay at all. ☹️

It’s also still moderately painful to open a fold which contains a so far unparsed color. Adding new syntax highlights under foldmethod=syntax with a large file is just inherently slow… Switching to CursorHold helps hide the hiccup, but that uses the updatetime setting for the delay, and the default for that is 4 seconds, which is not a good experience when used for rendering purposes. Unfortunately the main purpose of updatetime is to govern the viminfo file write delay, so setting updatetime to something more rendering-appropriate like 0.1 seconds is not a good idea either. And there is no real way to do timeouts in VimL, at least not in vanilla Vim… maybe I can fake something with the job support in Vim 8, I don’t know. I have no obvious idea for where to go with this yet.

For now I next want to investigate whether it’s possible to somehow postpone the installation of the autocommands until after the file has finished opening. Theoretically that should at least solve the startup time issue…

@chr4
Copy link
Author

chr4 commented May 8, 2020

Thanks for digging into this!

I'm actually not really using fdm=syntax to be honest. I can live with having it commented out.
I never conciously experienced the lag issues you're describing, but I'm not handling files with a lot of color syntax involved, maybe that's a reason why I never really bothered.

Applying your patch actually increases the startup delay for me:

084.546  001.213  001.213: sourcing ~/.config/nvim/bundle/vim-css-color/autoload/css_color.vim
7680.773  000.214  000.214: sourcing ~/.config/nvim/bundle/vim-css-color/syntax/colornames/basic.vim
7683.025  002.596  002.382: sourcing ~/.config/nvim/bundle/vim-css-color/syntax/colornames/extended.vim
7683.231  7600.039  7596.230: sourcing ~/.config/nvim/bundle/vim-css-color/after/syntax/css.vim

I'm going to just deactivate foldmethod=syntax as a workaround now. If you decide that it's too much efford to fix this, maybe consider adding a small note to README that this doesn't work well with foldmethod=syntax?

Let me know if I can test more things to help out!

@ap
Copy link
Owner

ap commented May 9, 2020

Applying your patch actually increases the startup delay for me:

Is that a tenfold increase…!?

If you decide that it's too much efford to fix this

It’s not. I don’t know whether it’s possible to make make it perform acceptably, with robust code – but if possible then I want to make it so.

And I just saw that Vim 8 does have a timer_start() and related functions; not sure how I missed that before. I’ll have to try it out to see if I guess right, but I’m speculating that installing the syntax highlights asynchronously might hide the painful hiccups enough to make things feel smooth(ish).

I’m a weirdo who doesn’t like telling users they have to live on the bleeding edge just to make my own life easier, so I’d also want to structure the plugin such that it can be async and smooth on Vim 8 but continues to work synchronously and no worse than before on Vim 7. (Not least because that’s also necessary to support Vim 8 versions compiled without timer support.)

@chr4
Copy link
Author

chr4 commented May 10, 2020

Is that a tenfold increase…!?

Seems like it. I was using neovim when benchmarking this, fyi.

Really apprechiating your effords here to fix this and also keep backwards compatibility. Good luck! Let me know if I can test something out, I'd be glad to help!

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