Run :make, :grep, etc. asynchronously as a Job. #1241

splinterofchaos · 2014-09-25T16:48:26Z

Not sure if this is the right way to do it, or even if it's a good thing to do, but these changes significantly improve the performance of the :make and :grep families by running them as Jobs instead of redirecting to an outfile then reading from it. I used this tiny vim script to test the performance:

function! s:grep_job()
  grep color **.c
endf

command DoIt call s:grep_job()

and ran it with $ build/bin/nvim -S grep.vim --cmd 'profile start grep.profile' --cmd 'profile! file grep.vim' -c 'DoIt'

The profiled time for the line grep color **.c:

count  total (s)   self (s)
    1   0.413358   0.162599   grep color **.c

and with this PR:

count  total (s)   self (s)
    1   0.179551   0.114989   grep color **.c

Less than half the run time! Although I have found the gains vary with the size of the output, it's never slower.

TODO:

Fix test failures. (a5ad879)
Check for bugs (There are encoding errors, but that's not relevant to this PR. (Run :make, :grep, etc. asynchronously as a Job. #1241 (comment)))
Protect against partial reads (f7f6ee7)
qf_init_ext() compiles several regex programs every time it's run. We can improve performance by saving the program and using it on the next run, but proving that skipping that massive code block is safe may be tricky. Doesn't seem to improve performance enough.
Update obsolete documentation in runtime/.

splinterofchaos · 2014-09-25T16:53:10Z

src/nvim/quickfix.c

-  sprintf((char *)cmd, "%s%s%s", (char *)p_shq, (char *)eap->arg,
-      (char *)p_shq);
-  if (*p_sp != NUL)
-    append_redir(cmd, len, p_sp, fname);


The fastest output I ever got was by, instead of removing these, using NULL for the callbacks and running mkfifo() here, but that certainly isn't OS-independent.

justinmk · 2014-09-25T17:56:28Z

Very nice. What happens if user initiates :grep while another :grep is still running?

splinterofchaos · 2014-09-25T19:02:00Z

Far as I know, that can't happen because I call job_wait() almost immediately after spawning it. Only things that should be running in parallel are the external program and the callback.

However, if the callback itself runs in parallel, we're in trouble. @tarruda, am I correct in my assumption that a job won't run the same callback twice in parallel? If not, is there a way to wait on the previous run to finish?

tarruda · 2014-09-25T21:34:01Z

However, if the callback itself runs in parallel, we're in trouble. @tarruda, am I correct in my assumption that a job won't run the same callback twice in parallel? If not, is there a way to wait on the previous run to finish?

When you call job_wait, it will focus the event loop on the job so other callbacks can't run. Also, Nvim is single-threaded so it's not possible to have another callback is running in parallel.

You might want to look if os_system works for these use cases, because it already wraps job_start/job_wait in a synchronous API for calling commands through the shell.

splinterofchaos · 2014-09-25T21:40:50Z

I was able to make this stop truncating lines due to the callback being sent partial reads, but lines greater than 1024 characters are being truncated still, whereas with the original code, newlines would be inserted, but I'm not sure why. (Perhaps that's a quirk of fgets().)

Also, the bytes seem to have been run through some conversion sort of because I get fenÃªtres instead of fenêtres.

You might want to look if os_system works for these use cases,

That's where I stole this code from! :)
os_system() isn't quite as fast as what I have written because it waits for the program to finish before proceeding, but it was faster than the old redirection method when I tested it.

justinmk · 2014-09-25T21:58:30Z

I get fenÃªtres instead of fenêtres.

Sounds like the encoding issue I was concerned about here: #1008

splinterofchaos · 2014-09-26T03:23:14Z

I get fenÃªtres instead of fenêtres.

Sounds like the encoding issue I was concerned about here: #1008

Not so sure any more. I had been copying the quick-fix window to a new buffer, saving as a file, then diffing that file against one generated by the master branch to ensure consistency. It looks right in the quick-fix window, and after yank/pasting into the new buffer--it's only upon loading the file that I see "Ãª" instead of "ê".

splinterofchaos · 2014-09-26T16:27:32Z

Ok, all of my TODO items finished. Functionally, the :make and :grep families differ a little this PR.

Errors written to stderr will be grouped together instead of interspersed with that written to stdout. Actually, I think stdout and stderr should be treated differently so the quickfix window doesn't fill up with a bunch of "Entering directory"/"Leaving ..."/"X is a directory"/"etc..." lines, but that's not what vim does and I don't have a good solution right now.

Lines above 1023 characters would be split up, previously, but that's because, when reading from a file, we call fgets() into a buffer of that size. Now, they get truncated by STRLCPY(), which IMO, means less garbage.

Output is no longer duplicated to stdout. This might be more a bad thing because on long :greps it might look like vim is doing nothing. Since this may case slowdown, I'm hesitant to re-add this feature.

splinterofchaos · 2014-09-27T21:08:46Z

I just discovered #473 and I think that implementing this in terms of a solution for that would be cleaner, so I'm marking this WIP until then.

tarruda · 2014-10-01T08:06:55Z

I just discovered #473 and I think that implementing this in terms of a solution for that would be cleaner, so I'm marking this WIP until then.

Thats great, thanks for giving it a shot.

Replace the shell redirection pattern by spawning a job and sending qf_init_ext() the output through a callback instead of a file through qf_init(). This lets us build and update a quick-fix list while "make"/"grep" runs.

The "make error file" never gets used, running through a Job, but keep the option in place because a user may reference it with an old .vimrc.

Sometimes the RStream only contains part of a line because its buffer maxed out or it didn't get the whole message on the first read. Maintain partial line buffers to keep this data intact between calls and separate stdout from stdin.

:make and :grep do not write to an error file.

justinmk · 2023-07-12T18:05:01Z

Implementing :vimgrep asynchronously using multiprocess is explored here: [WIP] Multiprocessing Feature #9943
Async :make, :grep, would be compelling but needs careful treatment.
- Could be done using a "provider" approach or some other hook, to allow implementation in Lua.
- https://github.com/tpope/vim-dispatch would be a good reference.
This PR removes 'makeef' but it is still relevant? grep/make: remove 'shellpipe', always read from pipe for 'makeef' #20890

splinterofchaos reviewed Sep 25, 2014
View reviewed changes

splinterofchaos force-pushed the ex_make-job branch from f7f6ee7 to 4252198 Compare September 26, 2014 16:13

splinterofchaos changed the title ~~Run :make, :grep, etc. asynchronously as a Job.~~ [RFC] Run :make, :grep, etc. asynchronously as a Job. Sep 26, 2014

splinterofchaos mentioned this pull request Sep 26, 2014

JobActivity: incomplete reads may cause corrupted v:job_data #1243

Closed

splinterofchaos changed the title ~~[RFC] Run :make, :grep, etc. asynchronously as a Job.~~ [WIP] Run :make, :grep, etc. asynchronously as a Job. Sep 27, 2014

splinterofchaos mentioned this pull request Oct 3, 2014

[RFC] Open os_call_shell() to sending to stdin/reading callbacks. #1263

Closed

splinterofchaos mentioned this pull request Oct 31, 2014

[RDY] Refactor os_call_shell to use os_system #1365

Merged

Scott Prager added 5 commits November 1, 2014 15:17

ex_make: Run :make/grep as Job.

b85426e

Replace the shell redirection pattern by spawning a job and sending qf_init_ext() the output through a callback instead of a file through qf_init(). This lets us build and update a quick-fix list while "make"/"grep" runs.

ex_make: Remove 'makeef' functionality.

e1c9164

The "make error file" never gets used, running through a Job, but keep the option in place because a user may reference it with an old .vimrc.

ex_make: Guard from partial reads.

2b2b14f

Sometimes the RStream only contains part of a line because its buffer maxed out or it didn't get the whole message on the first read. Maintain partial line buffers to keep this data intact between calls and separate stdout from stdin.

ex_make: Update documentation.

b01c252

:make and :grep do not write to an error file.

memory: implement xmemrchr()

1d0668b

justinmk added performance performance, latency, cpu/memory usage refactor changes that are not features or bugfixes enhancement labels Nov 11, 2014

splinterofchaos force-pushed the ex_make-job branch from 4252198 to 1d0668b Compare March 27, 2015 13:36

justinmk added the job-control OS processes, spawn label Apr 2, 2018

dundargoc removed the WIP label Feb 7, 2023

dundargoc changed the title ~~[WIP] Run :make, :grep, etc. asynchronously as a Job.~~ Run :make, :grep, etc. asynchronously as a Job. Apr 2, 2023

justinmk closed this Jul 12, 2023

justinmk mentioned this pull request May 21, 2024

asynchronous :make, :grep, etc #28881

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Run :make, :grep, etc. asynchronously as a Job. #1241

Run :make, :grep, etc. asynchronously as a Job. #1241

Uh oh!

splinterofchaos commented Sep 25, 2014

Uh oh!

splinterofchaos Sep 25, 2014

Uh oh!

justinmk commented Sep 25, 2014

Uh oh!

splinterofchaos commented Sep 25, 2014

Uh oh!

tarruda commented Sep 25, 2014

Uh oh!

splinterofchaos commented Sep 25, 2014

Uh oh!

justinmk commented Sep 25, 2014

Uh oh!

splinterofchaos commented Sep 26, 2014

Uh oh!

splinterofchaos commented Sep 26, 2014

Uh oh!

splinterofchaos commented Sep 27, 2014

Uh oh!

tarruda commented Oct 1, 2014

Uh oh!

justinmk commented Jul 12, 2023 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Run :make, :grep, etc. asynchronously as a Job. #1241

Run :make, :grep, etc. asynchronously as a Job. #1241

Uh oh!

Conversation

splinterofchaos commented Sep 25, 2014

Uh oh!

splinterofchaos Sep 25, 2014

Choose a reason for hiding this comment

Uh oh!

justinmk commented Sep 25, 2014

Uh oh!

splinterofchaos commented Sep 25, 2014

Uh oh!

tarruda commented Sep 25, 2014

Uh oh!

splinterofchaos commented Sep 25, 2014

Uh oh!

justinmk commented Sep 25, 2014

Uh oh!

splinterofchaos commented Sep 26, 2014

Uh oh!

splinterofchaos commented Sep 26, 2014

Uh oh!

splinterofchaos commented Sep 27, 2014

Uh oh!

tarruda commented Oct 1, 2014

Uh oh!

justinmk commented Jul 12, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

justinmk commented Jul 12, 2023 •

edited

Loading