-
Notifications
You must be signed in to change notification settings - Fork 80
Use CursorHold instead of Cursor Moved (performance) #126
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
Conversation
6895de0
to
e8920fc
Compare
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?
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.
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?
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.
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
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...
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!
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.)
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!
I've noticed massive performance issues when using
CursorMoved
, fixed it for me by switching toCursorHold
.