Create modern jobs api for simple processes #2195

lervag · 2021-10-06T21:06:54Z

NB! This PR is not finished yet and is not ready for testing.

I want to remove the old process.vim module and switch it with a new jobs.vim module that relies on the jobstart (nvim) and job_start (Vim) functions. I believe this may be useful in many cases, and perhaps give a better experience on Windows especially (cf. e.g. #1885).

lervag · 2021-10-07T10:31:42Z

This is now more or less ready for testing. @clason, I would be happy if you could test this on MacOS. I believe it should work more or less exactly as before (you should see no difference), so testing would only be to look for inconsistencies or errors.

I would also be glad if anyone could test this on Windows.

lervag · 2021-10-07T10:33:08Z

I'll be so bold as to ask some of the recent issue contributors that seem to use Windows: @habamax @bongbang

habamax · 2021-10-07T10:35:45Z

Sure, will check it, @lervag

habamax · 2021-10-07T10:43:55Z

regarding #1885, I am experiencing the same delay for initial <C-x><C-o> (~30 ~40 seconds for my doc). Not sure if you were to fix it with this PR too.

Other than that it works the same for me. Will continue using it and let you know if there are issues.

lervag · 2021-10-07T10:51:59Z

regarding #1885, I am experiencing the same delay for initial <C-x><C-o> (~30 ~40 seconds for my doc). Not sure if you were to fix it with this PR too.

Well, actually, I was hoping that this might resolve the delay, at least in part. :\

Other than that it works the same for me. Will continue using it and let you know if there are issues.

That's good to hear, though! Are you on Vim or neovim?

habamax · 2021-10-07T10:53:01Z

Another note, after I VimtexClearCache All, it takes < 2 secs to show the completion.

Tried with another new document it is < 2 seconds. :)

Not sure why first time was so long.

habamax · 2021-10-07T10:53:34Z

That's good to hear, though! Are you on Vim or neovim?

Latest nightly vim (gVim)

lervag · 2021-10-07T10:54:12Z

it takes < 2 secs to show the completion.

That sounds much better than 30 - 40 seconds. Or do I misunderstand anything?

habamax · 2021-10-07T10:54:47Z

That sounds much better than 30 - 40 seconds. Or do I misunderstand anything?

Now with cleared cache it is < 2 seconds, all consequent calls are instant.

lervag · 2021-10-07T10:57:00Z

How is this compared to the current master branch? Is the master branch still slow (as in O(10) seconds)?

habamax · 2021-10-07T10:59:47Z

How is this compared to the current master branch? Is the master branch still slow (as in O(10) seconds)?

Current master is about 10 seconds for cleared cache.

lervag · 2021-10-07T11:11:05Z

Cool; so, if I understand correctly, this PR should improve the experience for Windows users.

I'm pushing another update now, then I'll wait for some more feedback before I merge. This does change an essential part, so I believe it is sensible to avoid rushing the merge...

clason · 2021-10-07T11:31:06Z

This is now more or less ready for testing. @clason, I would be happy if you could test this on MacOS. I believe it should work more or less exactly as before (you should see no difference), so testing would only be to look for inconsistencies or errors.

I did a brief test, and everything seems to work as intended; I haven't done a proper side-by-side comparison, but it's certainly not slower and quite probably faster! (I'm on an M1 at the moment, so everything feels very snappy already ;))

lervag · 2021-10-07T11:33:18Z

Great, thanks for testing! I don't think it should be much faster, except perhaps on Windows where I've removed some silly hacks. But I hope it will be more robust, and there may be some performance improvements due to more consistent code.

lervag · 2021-10-08T10:22:29Z

I'm tempted to merge this. Feel free to dissuade me, I'll wait until later today in any case.

Use jobstart (neovim) or job_start (Vim) instead of the more "old fashioned" system() and ":!". refer: #2195

lervag · 2021-10-08T11:23:01Z

Thanks; I've merged this, then. :)

lervag · 2021-10-08T20:08:13Z

I've also pushed quite a few updates that further improve on this. I've tested quite thoroughly on my side with neovim and Vim, and I believe things should still work well on MacOS and Windows. However, I am curious to hear feedback on this, especially from the Windows side (@habamax).

lervag · 2021-10-08T20:20:52Z

Btw, some very crude benchmarking from my side:

Neovim (hyperfine -M 3 make)

commit	Timing (s)	Note
`fb7c6d5`	48.187 ± 0.221	Latest version
`f40305e`	49.068 ± 0.079
`ef3b014`	48.965 ± 0.031	This PR merged
`ef3b014`	49.217 ± 0.213
`aaecdbb`	48.282 ± 0.189	Before this PR

Vim (MYVIM="vim -T dumb --not-a-term -n" hyperfine -M 3 make)

commit	Timing (s)	Note
`fb7c6d5`	51.026 ± 0.138	Latest version
`f40305e`	52.302 ± 0.138
`ef3b014`	52.239 ± 0.204	This PR merged
`ef3b014`	52.208 ± 0.153
`aaecdbb`	51.368 ± 0.097	Before this PR

The commands are run inside VIMTEX/test; hyperfine is here. Again, this is very crude and does not really say very much. Between aaecdbb and fb7c6d5 there are also added several new tests, for instance, so in a way one should expect the run time to increase. I believe these tests may (perhaps) be better than nothing for justifying a claim that the improvements are improvements, especially considering that I think they strongly improve the robustness of the code.

bongbang · 2021-10-09T05:55:30Z

Sorry for being late to the party. I got the email rather late to begin with and was not quite savvy enough with git and Github to download a branch for testing. (I normally just let Vundle handle the updates.)

For future reference (so that I can do my part), is the following roughly what I should do?

cd into vimfiles/bundle
git clone https://github.com/lervag/vimtex/ --branch <branch> --single-branch vimtex-test
Find a handy way to switch back and forth between two versions of VimTeX (maybe you can tell me).

Thank you, Karl Lervåg , for ever diligent improvements. So far, I haven't noticed much difference in the latest version except perhaps one bug that I've been meaning to report may have already be solved.

habamax · 2021-10-09T06:21:55Z

@lervag well, now it takes more time with cleared cache (~ 30 secs) to show the completion for the same document :)

But when I save it under other path/name -- it is again "fast enough"

I wonder if it is possible to run background job once tex file is loaded to "construct" what is behind that cache.

lervag · 2021-10-09T09:42:59Z

@bongbang

Sorry for being late to the party.

No problem!

For future reference (so that I can do my part), is the following roughly what I should do?

Yes, more or less. I would do it like this:

cd /path/to/vimtex
# e.g. ~/.vim/bundle/vimtex

# Pull latest updates, including all branches
git pull --all

# Checkout the test branch
git checkout <branch>

# Checkout back to master
git checkout master

This way, you don't need to work with two copies of the code. git checkout as such serves as a handy way to switch between two versions.

Thank you, Karl Lervåg , for ever diligent improvements. So far, I haven't noticed much difference in the latest version except perhaps one bug that I've been meaning to report may have already be solved.

Great, I'm glad to hear this might have solved a bug, and I was not really expecting to hear of any noticable differences. Thank you for testing! And for the kind words :)

lervag · 2021-10-09T09:48:38Z

@habamax

well, now it takes more time with cleared cache (~ 30 secs) to show the completion for the same document :)

Yes, I was actually sort of expecting this. For some reason, it seems the system(...) call is much quicker than the job_start(...) call. However, I also believe the "improvement" in the previous version was faulty, because I added a manual wait for the job_start with a timeout of 5 seconds. When timing out, the system call would essentially return nothing. Thus, this did not fix anything, it only "treated the symptom".

But when I save it under other path/name -- it is again "fast enough"

What do you mean here? "save under other path/name"?

I wonder if it is possible to run background job once tex file is loaded to "construct" what is behind that cache.

Perhaps. Could I propose that you again open a new issue for this? I'm sorry about the inconvenience, but I now know much more about what actually happens, and so we could continue the discussion and possibly find a way to fix it.

One thing I'm thinking: Are you perhaps using WSL 2? Could it have something to do with the slowness of accessing files outside the linux partition?

habamax · 2021-10-09T10:32:22Z

But when I save it under other path/name -- it is again "fast enough"

What do you mean here? "save under other path/name"?

:w ~/docs/anotherpath/anothername.tex then switch to it and do all the same

I wonder if it is possible to run background job once tex file is loaded to "construct" what is behind that cache.

Perhaps. Could I propose that you again open a new issue for this? I'm sorry about the inconvenience, but I now know much more about what actually happens, and so we could continue the discussion and possibly find a way to fix it.

Sure will create it.

One thing I'm thinking: Are you perhaps using WSL 2? Could it have something to do with the slowness of accessing files outside the linux partition?

Well, that is another issue I might create. For this particular case I have used gVim (and in general for tex I use it).

I also have WSL1 where completion works "instantly", but the issue is that compilation is not working there, vimtex can't find latemxk executable:

lervag · 2021-10-09T14:00:12Z

Sure will create it.

Great, let's continue this discussion there, then.

wip: create modern jobs api for simple processes

253830b

lervag mentioned this pull request Oct 6, 2021

Reduce delay of completion #1885

Closed

lervag added 2 commits October 7, 2021 12:28

feat: swap process.vim with jobs.vim

d7359ae

fix: minor error

310df4d

test: adjusted test

53f7b86

feat(jobs): only collect output if desired

462f56f

lervag added 5 commits October 7, 2021 14:00

perf: more tight vim wait loop

f2d8681

chore: reorder methods

8f812e3

feat: use jobwait with nvim

2810f26

chore: minor adjustment

023f9b8

chore: more consistent use of system calls

97f9887

clason mentioned this pull request Oct 8, 2021

TUI/Windows: text draws over line numbers neovim/neovim#15734

Closed

lervag added a commit that referenced this pull request Oct 8, 2021

merge: modernise process handler

ef3b014

Use jobstart (neovim) or job_start (Vim) instead of the more "old fashioned" system() and ":!". refer: #2195

lervag closed this Oct 8, 2021

lervag deleted the feat/process-to-jobs branch October 8, 2021 11:22

habamax mentioned this pull request Oct 9, 2021

Use "pre-cache" for omni-completion in Windows. #2200

Closed

lervag mentioned this pull request May 23, 2023

Weird cmd prompt pop up when compiling #2706

Closed

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create modern jobs api for simple processes #2195

Create modern jobs api for simple processes #2195

lervag commented Oct 6, 2021

lervag commented Oct 7, 2021

lervag commented Oct 7, 2021

habamax commented Oct 7, 2021

habamax commented Oct 7, 2021 •

edited

Loading

lervag commented Oct 7, 2021

habamax commented Oct 7, 2021

habamax commented Oct 7, 2021 •

edited

Loading

lervag commented Oct 7, 2021

habamax commented Oct 7, 2021

lervag commented Oct 7, 2021

habamax commented Oct 7, 2021

lervag commented Oct 7, 2021

clason commented Oct 7, 2021

lervag commented Oct 7, 2021

lervag commented Oct 8, 2021

lervag commented Oct 8, 2021

lervag commented Oct 8, 2021

lervag commented Oct 8, 2021

bongbang commented Oct 9, 2021

habamax commented Oct 9, 2021

lervag commented Oct 9, 2021

lervag commented Oct 9, 2021

habamax commented Oct 9, 2021

lervag commented Oct 9, 2021

Create modern jobs api for simple processes #2195

Create modern jobs api for simple processes #2195

Conversation

lervag commented Oct 6, 2021

lervag commented Oct 7, 2021

lervag commented Oct 7, 2021

habamax commented Oct 7, 2021

habamax commented Oct 7, 2021 • edited Loading

lervag commented Oct 7, 2021

habamax commented Oct 7, 2021

habamax commented Oct 7, 2021 • edited Loading

lervag commented Oct 7, 2021

habamax commented Oct 7, 2021

lervag commented Oct 7, 2021

habamax commented Oct 7, 2021

lervag commented Oct 7, 2021

clason commented Oct 7, 2021

lervag commented Oct 7, 2021

lervag commented Oct 8, 2021

lervag commented Oct 8, 2021

lervag commented Oct 8, 2021

lervag commented Oct 8, 2021

bongbang commented Oct 9, 2021

habamax commented Oct 9, 2021

lervag commented Oct 9, 2021

lervag commented Oct 9, 2021

habamax commented Oct 9, 2021

lervag commented Oct 9, 2021

habamax commented Oct 7, 2021 •

edited

Loading

habamax commented Oct 7, 2021 •

edited

Loading