Use job API for system() calls in GUI Vim to avoid overhead#1325
Use job API for system() calls in GUI Vim to avoid overhead#1325junegunn merged 3 commits intojunegunn:masterfrom
Conversation
In GUI Vim (gvim, MacVim), each system() call incurs significant overhead due to command prompt window creation, making operations like PlugDiff ~5x slower than in terminal Vim or Neovim. Add s:system_job() which uses job_start with file-based output to run commands without that overhead, and use it in s:system() when running in GUI Vim with job support. Since v:shell_error is read-only in Vim 9, introduce s:shell_error which is set by both code paths and used at all call sites. Fixes junegunn#1312
| while job_status(job) ==# 'run' | ||
| endwhile |
There was a problem hiding this comment.
Is it acceptable to do this without any sleeps? Is this a common practice when using job_status?
There was a problem hiding this comment.
good point! added a sleep 10m to avoid hijacking the cpu (I'm a long time vim user, but little vimscript experience)
There was a problem hiding this comment.
Thanks for addressing the comment. I've been away from VimScript development for a while, so I also am not so familar with the latest best practices.
Did adding this affect the total execution time? What was your experience like?
There was a problem hiding this comment.
I wonder if we should reduce 10m to something like 1m.
There was a problem hiding this comment.
I just did some benchmarks:
without the sleep: 0.478
with 1m sleep: 0.599
with 10m sleep: 1.338
original code: 5.785
None of the options have any discernible impact on the cpu usage (I've got about 107 plugins).
If you decide we do need a sleep, I vote for the 1m sleep.
There was a problem hiding this comment.
Thanks! +1 on 1m.
Last time I checked, PlugDiff was much slower in GVim than in terminal Vim. But for some reason, when I tried it again after installing MacVim again on my system, the difference was not significant. But anyway, the patch definitely improves the performance.
PlugDiff execution time for me:
- Terminal Vim: 2.75
- MacVim: 4.28 -> 3.20 (10m) -> 2.49 (1m)
There was a problem hiding this comment.
done, sleep 1m seems a good compromise, on my system with gvim I'm going from 5.78 seconds to 0.59 seconds, pretty sweet :)
Add sleep 10m to the job_status polling loop so it yields the CPU instead of spinning at 100% while waiting for the job to finish.
|
Merged, thanks! |
2 participants
In GUI Vim (gvim, MacVim), each system() call incurs significant overhead due to command prompt window creation, making operations like PlugDiff ~5x slower than in terminal Vim or Neovim.
Add s:system_job() which uses job_start with file-based output to run commands without that overhead, and use it in s:system() when running in GUI Vim with job support.
Since v:shell_error is read-only in Vim 9, introduce s:shell_error which is set by both code paths and used at all call sites.
Tested in vim / gvim 9.1.2103 and neovim 0.11.6
PlugDiff in gvim is now as fast as in vim/neovim.
Fixes #1312