Skip to content

Use job API for system() calls in GUI Vim to avoid overhead #1325

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.

Sign up for GitHub

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

Merged
junegunn merged 3 commits into from
Feb 18, 2026
Merged

Use job API for system() calls in GUI Vim to avoid overhead #1325

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
cf2d899
Use job API for system() calls in GUI Vim to avoid overhead
jgb Feb 18, 2026
87dfcb2
Avoid busy-wait in s:system_job() polling loop
jgb Feb 18, 2026
28dfd36
Reduce sleep in s:system_job() polling loop to 1ms
jgb Feb 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 35 additions & 11 deletions plug.vim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ let s:mac_gui = has('gui_macvim') && has('gui_running')
let s:is_win = has('win32')
let s:nvim = has('nvim-0.2') || (has('nvim') && exists('*jobwait') && !s:is_win)
let s:vim8 = has('patch-8.0.0039') && exists('*job_start')
let s:shell_error = 0
if s:is_win && &shellslash
set noshellslash
let s:me = resolve(expand('<sfile>:p'))
Expand Down Expand Up @@ -170,7 +171,7 @@ function! s:git_origin_branch(spec)

" The command may not return the name of a branch in detached HEAD state
let result = s:lines(s:system('git symbolic-ref --short HEAD', a:spec.dir))
return v:shell_error ? '' : result[-1]
return s:shell_error ? '' : result[-1]
endfunction

if s:is_win
Expand Down Expand Up @@ -1097,7 +1098,7 @@ function! s:checkout(spec)
let credential_helper = s:disable_credential_helper() ? '-c credential.helper= ' : ''
let output = s:system(
\ 'git '.credential_helper.'fetch --depth 999999 && git checkout '.plug#shellescape(sha).' --', a:spec.dir)
let error = v:shell_error
let error = s:shell_error
endif
return [output, error]
endfunction
Expand Down Expand Up @@ -1303,23 +1304,23 @@ function! s:update_finish()
let tag = spec.tag
if tag =~ '\*'
let tags = s:lines(s:system('git tag --list '.plug#shellescape(tag).' --sort -version:refname 2>&1', spec.dir))
if !v:shell_error && !empty(tags)
if !s:shell_error && !empty(tags)
let tag = tags[0]
call s:log4(name, printf('Latest tag for %s -> %s', spec.tag, tag))
call append(3, '')
endif
endif
call s:log4(name, 'Checking out '.tag)
let out = s:system('git checkout -q '.plug#shellescape(tag).' -- 2>&1', spec.dir)
let error = v:shell_error
let error = s:shell_error
endif
if !error && filereadable(spec.dir.'/.gitmodules') &&
\ (s:update.force || has_key(s:update.new, name) || s:is_updated(spec.dir))
call s:log4(name, 'Updating submodules. This may take a while.')
let out .= s:bang('git submodule update --init --recursive'.s:submodule_opt.' 2>&1', spec.dir)
let error = v:shell_error
endif
let msg = s:format_message(v:shell_error ? 'x': '-', name, out)
let msg = s:format_message(error ? 'x': '-', name, out)
if error
call add(s:update.errors, name)
call s:regress_bar()
Expand Down Expand Up @@ -1480,7 +1481,7 @@ function! s:spawn(name, spec, queue, opts)
endif
else
let job.lines = s:lines(call('s:system', has_key(a:opts, 'dir') ? [argv, a:opts.dir] : [argv]))
let job.error = v:shell_error != 0
let job.error = s:shell_error != 0
let job.running = 0
endif
endfunction
Expand Down Expand Up @@ -2334,6 +2335,22 @@ function! s:with_cd(cmd, dir, ...)
return printf('%s %s %s %s', cd, plug#shellescape(a:dir, {'script': script, 'shell': &shell}), sep, a:cmd)
endfunction

function! s:system_job(cmd) abort
let tmp = tempname()
let job = job_start(['/bin/sh', '-c', a:cmd], {
\ 'out_io': 'file',
\ 'out_name': tmp,
\ 'err_io': 'out',
\})
while job_status(job) ==# 'run'
sleep 1m
endwhile
Comment on lines +2345 to +2347
Copy link
Copy Markdown
Owner

@junegunn junegunn Feb 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it acceptable to do this without any sleeps? Is this a common practice when using job_status?

Copy link
Copy Markdown
Contributor Author

@jgb jgb Feb 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point! added a sleep 10m to avoid hijacking the cpu (I'm a long time vim user, but little vimscript experience)

Copy link
Copy Markdown
Owner

@junegunn junegunn Feb 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Copy Markdown
Owner

@junegunn junegunn Feb 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we should reduce 10m to something like 1m.

Copy link
Copy Markdown
Contributor Author

@jgb jgb Feb 18, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Owner

@junegunn junegunn Feb 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Copy link
Copy Markdown
Contributor Author

@jgb jgb Feb 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 :)

let s:shell_error = job_info(job).exitval
let result = filereadable(tmp) ? join(readfile(tmp, 'b'), "\n") : ''
silent! call delete(tmp)
return result
endfunction

function! s:system(cmd, ...)
let batchfile = ''
try
Expand All @@ -2343,7 +2360,9 @@ function! s:system(cmd, ...)
" but it cannot set the working directory for the command.
" Assume that the command does not rely on the shell.
if has('nvim') && a:0 == 0
return system(a:cmd)
let ret = system(a:cmd)
let s:shell_error = v:shell_error
return ret
endif
let cmd = join(map(copy(a:cmd), 'plug#shellescape(v:val, {"shell": &shell, "script": 0})'))
if s:is_powershell(&shell)
Expand All @@ -2358,7 +2377,12 @@ function! s:system(cmd, ...)
if s:is_win && type(a:cmd) != s:TYPE.list
let [batchfile, cmd] = s:batchfile(cmd)
endif
return system(cmd)
if s:vim8 && has('gui_running') && !s:is_win
return s:system_job(cmd)
endif
let ret = system(cmd)
let s:shell_error = v:shell_error
return ret
finally
let [&shell, &shellcmdflag, &shellredir] = [sh, shellcmdflag, shrd]
if s:is_win && filereadable(batchfile)
Expand All @@ -2369,7 +2393,7 @@ endfunction

function! s:system_chomp(...)
let ret = call('s:system', a:000)
return v:shell_error ? '' : substitute(ret, '\n$', '', '')
return s:shell_error ? '' : substitute(ret, '\n$', '', '')
endfunction

function! s:git_validate(spec, check_branch)
Expand Down Expand Up @@ -2412,7 +2436,7 @@ function! s:git_validate(spec, check_branch)
\ 'git', 'rev-list', '--count', '--left-right',
\ printf('HEAD...origin/%s', origin_branch)
\ ], a:spec.dir)), '\t')
if v:shell_error || len(ahead_behind) != 2
if s:shell_error || len(ahead_behind) != 2
let err = "Failed to compare with the origin. The default branch might have changed.\nPlugClean required."
else
let [ahead, behind] = ahead_behind
Expand Down Expand Up @@ -2562,7 +2586,7 @@ function! s:upgrade()

try
let out = s:system(['git', 'clone', '--depth', '1', s:plug_src, tmp])
if v:shell_error
if s:shell_error
return s:err('Error upgrading vim-plug: '. out)
endif

Expand Down