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

Autocmd for remote plugin on VimEnter is slow #5728

Open
chemzqm opened this Issue Dec 7, 2016 · 28 comments

Comments

Projects
None yet
6 participants
@chemzqm
Copy link
Contributor

chemzqm commented Dec 7, 2016

  • nvim --version: NVIM v0.2.0-44-g8089292 Build type: Release
  • Vim (version: ) behaves differently? No
  • Operating system/version: MacOS
  • Terminal name/version: iTerm2

Actual behaviour

Autocmd registered from remote plugin could cost ~150ms on startup, even with sync=False flag.

Expected behaviour

I thought is should be no more than 10ms when using rpcnotify

Steps to reproduce using nvim -u NORC

Create a simple remote plugin in rplugin/python3/my.py

    import neovim

    @neovim.plugin
    class Limit(object):
        def __init__(self, vim):
            self.vim = vim

       @neovim.autocmd('VimEnter', pattern='*', sync=False)
        def on_enter(self):
            return

Add this plugin to runtimepath

Run :UpdateRemotePlugin

use command:

nvim --startuptime tmp.txt

The time used by VimEnter autocommands would be increased more than 150ms.

I tried with sync=True, and it even worse.

Since I just want to start a server on VimEnter, I changed the code to use jobstart and normal VimEnter autocmd, the time of VimEnter autocommands increased no more than 10ms

@chemzqm chemzqm changed the title Autocmd for remote plugin is slow Autocmd for remote plugin on VimEnter is slow Dec 7, 2016

@mhinz

This comment has been minimized.

Copy link
Member

mhinz commented Dec 7, 2016

See :h g:python_host_prog and the following options, especially the skip_check ones.

sync has nothing to do with the startup time of the Python host.

@justinmk

This comment has been minimized.

Copy link
Member

justinmk commented Dec 7, 2016

Why does g:python_host_skip_check exist? What happens if user has g:python_host_skip_check=1 but it's a lie (i.e. user doesn't have neovim python module, or it's wrong, or whatever)?

@justinmk justinmk added this to the 0.3 milestone Dec 7, 2016

@mhinz

This comment has been minimized.

Copy link
Member

mhinz commented Dec 7, 2016

It prevents shelling out to python -c 'import sys; print(sys.version_info[0])' (simplified) and always assumes "the Python installation is working".

If neither g:python_host_prog nor g:python_host_skip_check are set, there could be multiple calls to exepath() and system().

And the same again for Python 3.

provider#pythonx#Detect() could need some love, though.

@chemzqm

This comment has been minimized.

Copy link
Contributor Author

chemzqm commented Dec 7, 2016

@mhinz I have these lines in my vimrc:

let g:python_host_skip_check=1
let g:python_host_prog = '/usr/local/bin/python'
let g:python3_host_skip_check=1
let g:python3_host_prog = '/usr/local/bin/python3'

But it doesn't help too much:

  • Without autocmd of VimEnter

      181.817  064.025: VimEnter autocommands
    
  • With VimEnter autocmd and python host skip check

      341.260  229.980: VimEnter autocommands
    
  • With VimEnter autocmd and without python host skip check

      322.555  210.953: VimEnter autocommands
    

    even a litte faster 😕

  • With VimEnter autocmd and without python host skip check and using sync=True

      331.298  220.584: VimEnter autocommands
    

seems nothing different, but could be much slower when more code inside python handler.

@mhinz

This comment has been minimized.

Copy link
Member

mhinz commented Dec 7, 2016

Maybe that's just the time starting a Python process costs in general? Maybe there are ways to improve the startup performance in python-client, but apart from that I don't see how to improve this further.

That happens only once, though. You can check that after starting nvim, there will be a Python child process that acts as the host.

Maybe there's a better way to achieve what you're trying to do?

@chemzqm

This comment has been minimized.

Copy link
Contributor Author

chemzqm commented Dec 7, 2016

@mhinz I've tried to add VimEnter autocmd to more plugins , the startup time didn't make any noticeable difference, so I think you're right.

@justinmk

This comment has been minimized.

Copy link
Member

justinmk commented Dec 7, 2016

What good is g:python_host_skip_check doing then? Would be nice to remove that option if possible.

@mhinz

This comment has been minimized.

Copy link
Member

mhinz commented Dec 7, 2016

@justinmk

Since "having a working installation" is specific to the system anyway, we could probably drop it in favor of just having g:python_host_prog.

I could refactor runtime/autoload/provider/pythonx.vim, if you want.

@chemzqm Could you gist the complete startup log (without g:python... set)?

CC @bfredl

@chemzqm

This comment has been minimized.

Copy link
Contributor Author

chemzqm commented Dec 7, 2016

@mhinz https://gist.github.com/chemzqm/faa1f6bac2eddc3c15026c1109a8e69f

It looks like the python process start would be asynchronous if there's no VimEnter autocmd of remote plugin

@justinmk

This comment has been minimized.

Copy link
Member

justinmk commented Dec 8, 2016

@mhinz 👍

@bfredl

This comment has been minimized.

Copy link
Member

bfredl commented Dec 8, 2016

At least g:python3_host_skip_check should be implied by g:python3_host_prog (if the user specifies an executable, it should be trusted), but I won't mind if it was completely removed.

@justinmk

This comment has been minimized.

Copy link
Member

justinmk commented Dec 8, 2016

@bfredl That's what I was thinking too. g:python3_host_prog can serve the purpose of g:python3_host_skip_check (if any). And g:python3_host_skip_check and be removed from the docs and silently ignored.

@chemzqm

This comment has been minimized.

Copy link
Contributor Author

chemzqm commented Dec 17, 2016

@mhinz It only need 1ms to start process, but it cost more than 150ms to check if the process is running ok in https://github.com/neovim/neovim/blob/master/runtime/autoload/provider/pythonx.vim#L35

I've profiled with:

  let start = reltime()
  try
    let channel_id = jobstart(args, s:job_opts)
    echo reltimestr(reltime(start))
    if rpcrequest(channel_id, 'poll') ==# 'ok'
      echo reltimestr(reltime(start))
      return channel_id
    endif

Couldn't we check that in async, for example, use timer_start to check the status?

@Shougo

This comment has been minimized.

Copy link
Contributor

Shougo commented Dec 18, 2016

@mhinz It only need 1ms to start process, but it cost more than 150ms to check if the process is running ok in https://github.com/neovim/neovim/blob/master/runtime/autoload/provider/pythonx.vim#L35

Yes, it is the same overhead for me.

@Shougo

This comment has been minimized.

Copy link
Contributor

Shougo commented Jun 14, 2017

I have tested my.py VimEnter.

087.841  076.407: VimEnter autocommands

It is 76ms in master.
Unfortunately, #5856 does not work. So I cannot test the optimization.

@chemzqm

This comment has been minimized.

Copy link
Contributor Author

chemzqm commented Jun 14, 2017

I was AFK for some time, will look into this issue again later

@Shougo

This comment has been minimized.

Copy link
Contributor

Shougo commented Jul 5, 2017

#5856 does not fix the problem.
I think the wait is needed for VimEnter.

@bfredl

This comment has been minimized.

Copy link
Member

bfredl commented Jul 5, 2017

#6743/#6742 will fix the problem.

@Shougo

This comment has been minimized.

Copy link
Contributor

Shougo commented Jul 5, 2017

#6743/#6742 will fix the problem.

Why it fixes the problem?
The remote plugins are loaded immediately?

@bfredl

This comment has been minimized.

Copy link
Member

bfredl commented Jul 5, 2017

No, it simply enables to do the host initialization in a completely async fashion. So autocmd("VimEnter", sync=False) will check if the host is ready, if not, it will be delayed until ChannelInfo event. Of course sync=True will still be slow.

Moreover we should enable the same for legacy script hosts but it will require plugin/user intervention (made up names, probably hasn't dedicated autocmd):

if has("nvim")
  RequireHost python-legacy
  autocmd HostLoaded python-legacy call CodeUsingPython()
else
  call CodeUsingPython()
end

func CodeUsingPython()
  py import whatever
  ...
endfunc

or maybe even

  HostAsync python-legacy call CodeUsingPython()
@Shougo

This comment has been minimized.

Copy link
Contributor

Shougo commented Jul 5, 2017

Ah, OK. It seems better.

When #6743 #6742 is merged?

#5856 is not needed?

@bfredl

This comment has been minimized.

Copy link
Member

bfredl commented Jul 6, 2017

#6742 will take some time, #5856 can be merged in the meanwhile if it is noticeable improvement.

@Shougo

This comment has been minimized.

Copy link
Contributor

Shougo commented Jul 6, 2017

OK. I think #5856 improves the performance.

@StillStealSteel

This comment has been minimized.

Copy link

StillStealSteel commented Jul 19, 2018

What is the status of this issue?
Currently I have 750+ms added by VimEnter autocommands to the startup time.

And with disabled deoplete only 32ms.
@Shougo

@chemzqm

This comment has been minimized.

Copy link
Contributor Author

chemzqm commented Jul 20, 2018

@StillStealSteel here is a workaround:

diff --git a/runtime/autoload/provider.vim b/runtime/autoload/provider.vim
index dc24e801d..754f1cc79 100644
--- a/runtime/autoload/provider.vim
+++ b/runtime/autoload/provider.vim
@@ -4,18 +4,25 @@
 "
 " Returns a valid channel on success
 function! provider#Poll(argv, orig_name, log_env) abort
-  let job = {'rpc': v:true, 'stderr_buffered': v:true}
+  let job = {'rpc': v:true, 'stderr_buffered': v:true, 'on_stderr': funcref('s:OnError')}
   try
     let channel_id = jobstart(a:argv, job)
-    if channel_id > 0 && rpcrequest(channel_id, 'poll') ==# 'ok'
+    if channel_id > 0
       return channel_id
     endif
   catch
     echomsg v:throwpoint
     echomsg v:exception
-    for row in get(job, 'stderr', [])
-      echomsg row
-    endfor
   endtry
   throw remote#host#LoadErrorForHost(a:orig_name, a:log_env)
 endfunction
+
+function! s:OnError(id, data, event)
+  if !empty(a:data)
+    echohl Error
+    for row in a:data
+      echomsg row
+    endfor
+    echohl None
+  endif
+endfunction

But the user will miss the errors on startup or :UpdateRemotePlugins since it's async, :message could be used to show the errors.

I think it would be useful if neovim could provide async rpcrequest that accepts callback to deal with the response.

@StillStealSteel

This comment has been minimized.

Copy link

StillStealSteel commented Jul 20, 2018

@chemzqm Will test it lately. Thank you.
If it works is there any chance that this would be added in the master?
Yes i understand that startup errors for majority of users is more important, but maybe as an option that turn off by default? Or its completely temporal hack?
Nothing can be done on deoplete side? As i understand it's python provider that is slow, right?

@StillStealSteel

This comment has been minimized.

Copy link

StillStealSteel commented Jul 20, 2018

@chemzqm Thank you. It works like a charm.

@Shougo

This comment has been minimized.

Copy link
Contributor

Shougo commented Jul 22, 2018

Nothing can be done on deoplete side? As i understand it's python provider that is slow, right?

It is not related to deoplete.
deoplete already implements async initialization.

gibfahn added a commit to gibfahn/dot that referenced this issue Jan 6, 2019

vimrc: update statusline, undo, surround, indentation
- Add lightline (pretty statusbar)
- Change Gundo to Mundo (updated fork).
- Add vim-textobj-line (`il` and `al` line motions).
- Add vim-sleuth to automatically work out file indentation.
- Improve surround workaround [0][].
- Delay enabling deoplete as nvim startup is slow [1][]
- Fix $MYVIMRC sourcing with lightline [2][].

[0]: https://vi.stackexchange.com/questions/16565/reclaim-cursor-shape-in-operator-pending-mode-neovim-while-using-vim-surround
[1]: neovim/neovim#5728
[2]: https://github.com/itchyny/lightline.vim/pull/103/files
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.