Add g:ycm_start_autocmd option

[wincent]

PR Prelude

Thank you for working on YCM! :)

Please complete these steps and check these boxes (by putting an x inside
the brackets) before filing your PR:

• I have read and understood YCM's CONTRIBUTING document.
• I have read and understood YCM's CODE_OF_CONDUCT document.
• I have included tests for the changes in my PR. If not, I have included a
  rationale for why I haven't.
• I understand my PR may be closed if it becomes obvious I didn't
  actually perform all of these steps.

Why this change is necessary and useful

Profiling with vim --startuptime shows that YCM is responsible for about 250ms of the overall 500ms that Vim spends starting up on my machine. By switching from "VimEnter" to "CursorHold,CursorHoldI" as a trigger for calling youcompleteme#Enable, we get almost all of that back, leading to an approximately 50% speed-up and a very perceptible improvement in responsiveness.

The trade-off here is that YCM won't get fully initialized until Vim has been idle for 'updatetime' seconds (2s in my set-up, which apparently YCM itself sets; before YCM runs though, the default of 4s is probably active). As not everybody may like this trade-off, I've made all this configurable via an option.

Test plan: Manually tested with --startuptime and the settings shown in the documentation (no automated tests yet because testing VimScript changes automatically is hard, like it says in the CONTRIBUTING doc; wanting some feedback on whether this would be the sort of thing that you'd want to include before I go further with it).

---

This change is 

[codecov-io]

Current coverage is 84.44% (diff: 100%)

Merging #2454 into master will not change coverage

@@             master      #2454   diff @@
==========================================
  Files            17         17          
  Lines          1903       1903          
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
  Hits           1607       1607          
  Misses          296        296          
  Partials          0          0          

Powered by Codecov. Last update 39be8b1...69bcb5b

[Valloric]

Thanks for sending a PR!

I'm not necessarily against the change you're making, but I am against exposing it as an option. We already have waaaay too many of them and are extremely wary about adding more.

This change, if it works well, should just be something we do for everyone. Given that, it's a fairly scary change that has the potential to break stuff. So we need to be careful here. What possible drawbacks do you see of doing this for everyone? Having YCM be unavailable for the first 2 seconds might be worth the startup gains...

@micbou @puremourning @vheon Thoughts?

[wincent]

Having YCM be unavailable for the first 2 seconds might be worth the startup gains...

One thing to note is that it won't be 2 seconds but 4 seconds given the default 'updatetime'. (YCM won't drop it from 4s to 2s until after youcompleteme#Enable() runs.) In my own .vimrc I set 'updatetime' to 2000 to avoid this. We might want to consider moving that part out of youcompleteme#Enable() and into the non-autoloaded part of the plug-in; it should be immeasurably fast.

[Valloric]

What we're ultimately trying to do is start YCM startup asynchronously. I know latest Vim has some async APIs, so we might want to detect the presence of these and use them for faster startup. If they're not there, go with the old blocking approach.

The it becomes a "simple" issue of upgrading your Vim to get better startup perf. The benefit is that we wouldn't have any artificial delay from updatetime on when startup actually triggers.

[vheon]

I'm not a big fan of this change. Not for the change itself but rather because if we start doing this kind of changes I'm afraid that users with slightly different needs will ask to tweak this again, leading to more user options for "not so used functionalities". Instead what I would prefer is to let the user decide when to load YCM and be done with it. Since #2084 YCM is ready to be lazy-loaded. This means that for this specific use case (load YCM on the first CursrorHold,CursorHoldI a user of vim-plug for example will just have to do this in his vimrc:

Plug 'Valloric/YouCompleteMe', { 'on': [] }

augroup load_ycm
  autocmd!
  autocmd CursorHold, CursorHoldI * call plug#load('YouCompleteMe')
                                \ | autocmd! load_ycm
augroup END

or a user with the latest vim can use the packages, dropping YCM in pack/*/opt/ and have this in its vimrc instead:

augroup load_ycm
  autocmd!
  autocmd CursorHold, CursorHoldI * :packadd YouCompleteMe
                                \ | autocmd! load_ycm

augroup END

What we're ultimately trying to do is start YCM startup asynchronously. I know latest Vim has some async APIs, so we might want to detect the presence of these and use them for faster startup. If they're not there, go with the old blocking approach.

For me this is the proper way to solve the startup problem.

PS: @wincent I'm a fan of your vim screencasts, keep'em coming 😛 👍

[wincent]

Good points, @vheon. :packadd is a good solution, and in fact I'm using that to lazy-load NERDTree (which is similarly poorly behaved, eagerly loading a bunch of autoloaded functionality).

The reason I made this PR is because there is clearly an intent in the implementation to move loading out of the critical startup path by default, requiring no additional action from the user. (I ended up adding an option only because it constituted a behavior change and so I thought it might make it more likely to be accepted.)

I'm happy for this to be closed, but I think perhaps the entire autocmd VimEnter line should be replaced with a straight-up call:

diff --git a/plugin/youcompleteme.vim b/plugin/youcompleteme.vim
index 0ac381e..cafb013 100644
--- a/plugin/youcompleteme.vim
+++ b/plugin/youcompleteme.vim
@@ -129,12 +129,15 @@ let g:ycm_goto_buffer_command =
 let g:ycm_disable_for_files_larger_than_kb =
       \ get( g:, 'ycm_disable_for_files_larger_than_kb', 1000 )
 
-" On-demand loading. Let's use the autoload folder and not slow down vim's
-" startup procedure.
-if has( 'vim_starting' ) " loading at startup
-  augroup youcompletemeStart
-    autocmd!
-    autocmd VimEnter * call youcompleteme#Enable()
-  augroup END
-else " manual loading with :packadd
-  call youcompleteme#Enable()
-endif
+call youcompleteme#Enable()
 
 " This is basic vim plugin boilerplate
 call s:restore_cpo()

(not a real patch; edited in the browser.)

[Valloric]

I'm happy for this to be closed, but I think perhaps the entire autocmd VimEnter line should be replaced with a straight-up call

Could you elaborate what that would buy us?

@vheon I did forget about the packadd feature, thanks for pointing that out. I also agree with @wincent's point that we should probably be doing some sort of async loading by default if we can get away with it without impacting functionality.

[wincent]

Could you elaborate what that would buy us?

The presence of the autocmd, the vim_starting check, and the comment:

" On-demand loading. Let's use the autoload folder and not slow down vim's
" startup procedure.

Are all misleading, because they don't have the desired effect.

I think you should either make deferred loading work automatically, or document how the user can achieve it (ie. via autocmd CursorHold, CursorHoldI * :packadd YouCompleteMe etc). The third alternative (ie. the current implementation) doesn't really seem to have a reason to exist.

[vheon]

The third alternative (ie. the current implementation) doesn't really seem to have a reason to exist.

Actually there was a reason for this which was that had to be sure to be executed after Ultisnips and with VimEnter we were sure about this, but since #2321 I think you're right and we can call youcompleteme#Enable() right away.

[Valloric]

I think you should either make deferred loading work automatically, or document how the user can achieve it (ie. via autocmd CursorHold, CursorHoldI * :packadd YouCompleteMe etc). The third alternative (ie. the current implementation) doesn't really seem to have a reason to exist.

I can't really disagree with this. :)

[vheon]

@wincent so at this point we either close this or if you want (and have the time) you can change this to the "patch" you proposed at #2454 (comment), otherwise I'll do it without problem 👍

If you decide to change this and you want to change the docs, don't touch the youcompleteme.txt file but change the README.md because we generate the former using the latter.

[wincent]

Sorry about the delay, @vheon. Closing this in favor of #2473.