xolox / vim-easytags

Automated tag file generation and syntax highlighting of tags in Vim
http://peterodding.com/code/vim/easytags/
1.01k stars 109 forks source link

Asynchronous tag updates #49

Closed xolox closed 10 years ago

xolox commented 11 years ago

I would love to see asynchronous tag updating implemented and working so reliably that it can be made the default. In pull request #43 I discussed with @inkarkat how to implement this in a portable manner (without too much dependencies and machinery). He pointed out that I could always fork Vim to the background:

@inkarkat said: One idea I had was to strip off the tags updating part of easytags into a separate Vimscript, and then just launch a non-interactive separate Vim process in the background (without loading .vimrc and other plugins). Though not as efficient as Python or binary, it might be a good compromise, and everybody probably has a terminal Vim available :-)

@xolox said: For some reason this seems really dirty to me :-) however it would definitely be portable and it would completely solve the code duplication issue. You definitely got me wondering, so I tried it out and it seems surprisingly fast:

peter@lucid-vm> time vim -u NONE -U NONE --noplugin --cmd 'call xolox#misc#msg#info("testing, 1, 2, 3") | scriptnames | quit'

… Vim startup screen snipped …

testing, 1, 2, 3

1: ~/Dropbox/Dotfiles/vim/autoload/xolox/misc/msg.vim

vim -u NONE -U NONE --noplugin --cmd   0.00s user 0.02s system 58% cpu 0.034 total

Hopefully I'll have some time to create a prototype to play around with in the next weekend.

So a bit later than estimated, but here's a start. This code definitely still has some issues, so instead of simply releasing it as the default I'm keeping it in a feature branch until I trust it a bit more :-).

xolox commented 11 years ago

The first three commits simply rip out the Python code and any functionality that conflicted with (or significantly complicated) the asynchronous update process. What's nice about this is the diff --stat output:

 README.md                   |   41 +-----
 autoload/xolox/easytags.vim |  386 +++++++++++++++++++++----------------------
 doc/easytags.txt            |  105 ++----------
 misc/easytags/highlight.py  |   55 ------
 plugin/easytags.vim         |   21 +--
 5 files changed, 220 insertions(+), 388 deletions(-)

So by adding asynchronous tag updates I've actually reduced the amount of code significantly :-). That's not to say that the feature is finished and ready to release, but I digress.

xolox commented 11 years ago

Issues I noticed with this code so far:

  1. Once or twice now while I was working on vim-easytags, an empty Vim window popped up without any indication as to why that happened. Of course this Vim window was meant to be used for an asynchronous tag update, but somehow it didn't happen.
  2. In 3408ba9 I removed the special casing of the "first run" without thinking about it too much, but I guess it was there for a reason: When Exuberant Ctags reports tags to standard output, it doesn't include a header, so tags files will now be created without any headers...?
  3. In aa65f9f I ripped out the Python code to make syntax highlighting faster, but I did not implement an alternative yet. I'm considering moving most of the highlighting process to the asynchronous background processes as well, but have to give it a bit more thought...
  4. A couple of times now I've had Vim crash on me since I started using the asynchronous branch during unrelated work. I used the verbosefile option to capture a message log when it happened, but the log was buffered so it stopped before the crash ಠ_ಠ. I guess I'll have to dig out my notes on compiling Vim with debugging symbols and running it under gdb. Already looking forward to the experience ಠ_ಠ.

Edit: So far I've been putting of recompiling Vim with debugging symbols, but I do have a suspicion what the problem might be: Maybe remote_expr() is not always save to call? The only problem then would be: How can I tell when it's safe and when it's not? (before crashing Vim :-)

xolox commented 11 years ago

@inkarkat: I just posted an addition to my list of issues above:

  1. After fixing xolox/vim-easytags#51 I was able to try the asynchronous tag updates on Windows, but it turns out to be quite annoying: When Vim has no interface to show a message printed with :echomsg, it will pop up a dialog showing the message. For me it does this every single time an asynchronous tag update runs. I'll have to silence those messages somehow (preferably without losing the ability to easily debug the plug-in).

I assume you are testing the asynchronous branch on Windows. Have you noticed this issue? Maybe you found an easy way to fix it? I guess I can always use something like the verbosefile option, but I'd rather not :-)

inkarkat commented 11 years ago

Yes, GVIM does that. The way I've worked around that is by (first hard-coding, now configuring)

let g:xolox#misc#os#vim_progname = 'vim'

This always uses terminal Vim, which does not show this behavior. Why do you need to always echo messages from stage 2 (I could understand during debugging); that whole process isn't visible to the user, anyway?

xolox commented 11 years ago

You're right that it doesn't make sense to echo messages in an invisible window, however easytags relays the messages to the user's Vim instance. I was kind of abusing the existing :echomsg infrastructure for that :-). However I like your approach of always running the console Vim; that definitely won't have any graphical interaction, which is exactly what we need. I changed xolox#misc#os#find_vim() to accept the string vim or gvim so callers can indicate their preference. I also changed the async branch to pass the vim argument to xolox#misc#os#find_vim(). Thanks for the tip :-)

inkarkat commented 10 years ago

I've noticed some recent fixes going into master that haven't yet been merged into the async-cleanup branch yet. I hope you're not abandoning that branch; I've been using it without problems for the past 3 months!

My vote is clearly for including this in the mainline now. Because maintaining both synchronous and async modes is a lot of (testing) effort, I would recommend to completely switch over to async (in a new alpha release), and only go down the route of configurable modes if we encounter many async problems that cannot be fixed easily. As I said, I found the original plugin too slow, but I'm happily running the async branch (mostly with a 5 MB tags database of > 2000 Vimscript files).

jcf commented 10 years ago

Have you considered adding support for vimproc? Something like the system_bg function would handle shelling out to ctags, without the need to support directly in easytags.

You could detect if it's installed, and when available use the vimproc function to run ctags, and when it's not run the call synchronously…

If I were to create a PR with this feature, would you consider merging it?

sjdrodge commented 10 years ago

+1 for @jcf's suggestion. vim-easytags is currently unusable for me because it sometimes blocks for several seconds after I open a new file. Vimproc support is by far the easiest way to implement async, and even if you don't like it as a permanent solution, it would be great in the meantime.

inkarkat commented 10 years ago

@sjdrodge Try out this async branch; it works very well for me (no more annoying blocking indeed)!

And another bump @xolox to please have this integrated with the mainline!

aktau commented 10 years ago

This does not seem to work on osx (mavericks) homebrew vim on the commandline. Most likely because vim remote doesn't work on it. When I run vim --servername ... --remote-expr ... it just gives me that --servername is not recognized... I've done some reading and it appears X11 is required for vim remote. A bit sad as it looked really nice.

mikedfunk commented 10 years ago

async branch doesn't work for me either on osx mavericks. I get a bunch of errors about not being able to find syntax files.

inkarkat commented 10 years ago

I'm happy to see you updating the master branch again. Do you plan to merge those updates to this async branch, too, or have you abandoned it?! I still urge you to integrate this soon; I'm still happily using it (and can't imagine going back to the old blocking behavior)!!!

xolox commented 10 years ago

@jcf, @sjdrodge

The problem is not with asynchronous command execution so using vimproc is not a solution. That part was solved when I opened this feature branch.

@inkarkat

I haven't exactly abandoned this feature branch, but I'm also not very enthusiastic about merging it into master as is, because the last status update from me was that it had a tendency to sporadically crash my GVim on Linux, which is totally unacceptable and a blocker for merging it into master.

I just tried merging master into the feature branch and this has already become too complex for my taste. I have to resolve merge conflicts in my day job as well and having to do it when I get paid to do it is one thing, but this is another :-). What I'm considering now: Merging async-cleanup (maybe selectively) into master and hiding it behind feature flags. That would solve the most urgent problems:

That is not to say that this will be an easy undertaking, but I do still believe that this (async mode) is the future for vim-easytags (assuming it has a future, but given that it's by far my most popular plug-in together with vim-notes I expect it does) so it should be worth the effort (I would personally love it if Vim never blocks / hangs again).

@ everyone

I hope to post an update here soon. Feel free to bug me if I don't ;-)

inkarkat commented 10 years ago

I fully agree that a merge to master as an optional feature is the way to go; in fact, I would have done so earlier, before the branches diverged so much. I hope you'll find the time to go through this; I know it won't be trivial. I'm definitely willing to test the result.

As there's already quite a lot of code, I'd recommend to create separate autoload/xolox/easytags/async.vim and .../sync.vim, and only leave the general functions in the original autoload/xolox/easytags.vim.

xolox commented 10 years ago

Hi all,

I'm almost done reimplementing this on top of master. I took a very different route this time, pushing most of the complexity over to vim-misc where I can use it in my other Vim plug-ins. I just committed xolox/vim-misc@81fdec8317a53695fb9d27eaad344da55eb159e6 (reformatted commit message below). My next commit will integrate this new functionality into easytags (mostly done already).


Generalized asynchronous Vim script evaluation (for vim-easytags)

The idea of forking Vim into the background to perform long running tasks that would otherwise block the user's editing session originates from the 'async-cleanup' feature branch of vim-easytags, where I picked it up from a tip by Ingo Karkat (@inkarkat). Because I can see this being useful in several of my Vim plug-ins I decided to generalize the concept and implement it in vim-misc.

One thing that may not be immediately obvious from the commit is why I implemented two ways for the results of asynchronous processes to feed back into the main Vim process. The reason is twofold, both having to do with Vim's client/server implementation:

  1. Reading through the pull request of the mentioned feature branch it looks like Vim's client/server support on Mac OS X is either missing or problematic.
  2. Running the 'async-cleanup' branch (which always used client/server communication) I saw several crashes on Linux. I have no proof but suspected the client/server communication to be the cause of this.

So now there's an alternative if the client/server mechanism starts to act up (which it may very well do because it's a complex part of Vim).

xolox commented 10 years ago

Hi all,

I created a new feature branch (async-take-two) and a new pull request (#84). Let's continue the discussion there. I'll close this now because the async-cleanup feature branch has suffered bit rot and I'm not going to revive it.

Thanks for all of the feedback!

xolox commented 10 years ago

In case anyone who participated in this discussion didn't notice yet: I created a new feature branch in #84. That feature branch is now merged so you can try out the feature by pulling the latest changes on master and enabling an option.

Reading through this discussion once more I notice the bug reports about Mac OS X and Vim's client/server support. The new feature branch can use Vim's client/server support but it can also work without it, so it might Just Work on Mac OS X. If it doesn't work out of the box on Mac OS X feel free to report a new issue on GitHub, I don't think it will be a lot of effort to make it work out of the box on Mac OS X (assuming it doesn't already; I can't easily test).

aktau commented 10 years ago

@xolox just a heads up, for async updates it might be possible to defer to jobs or async_send on neovim to simplify the whole ordeal quite a bit:

Some issues that could be relevant:

I'm not entirely sure if the events or the jobs feature is most appropriate, @tarruda could probably expound on that :).

xolox commented 10 years ago

@aktau: Thanks for the heads up. I was actually completely unaware of neovim until yesterday evening, then again I haven't been following vim-dev for quite a while. I assume these mechanisms are backwards incompatible with "stock" Vim, right? But I read something about it being 99% compatible, so I suppose I could probe for supported mechanisms using exists()? I would need to actually start using neovim then though... I hope it's easy to compile on Linux/GTK? :-)

One thing to note about the neovim proposal is that I would never break compatibility with "stock" Vim, because as I said I'd never heard of neovim until yesterday evening so I'm not going to hedge my bets on neovim just yet (with all due respect because I do think it's awesome what you guys are doing).

aktau commented 10 years ago

One thing to note about the neovim proposal is that I would never break compatibility with "stock" Vim, because as I said I'd never heard of neovim until yesterday evening so I'm not going to hedge my bets on neovim just yet (with all due respect because I do think it's awesome what you guys are doing).

Don't worry, we don't expect anyone to drop vanilla Vim compatibility unless they're developing from scratch and really want to use neovim-only features. We do our utmost to maintain backwards compatibility.

But, there are (and will be) has and/or exists for features that neovim supports, so developers can make use of a simpler and hopefully faster/bugfree codepath when the feature is detected.

Perhaps, if it proves popular, Bram might consider integrating a specific feature in vanilla Vim (though I don't see it happening any time soon, especially the async features which would be quite difficult to implement with Vim's architecture).

tarruda commented 10 years ago

@xolox You can test if a script is running under neovim using the has('neovim') expression. The best way to run asychronous tag updates would be to use the job control feature, I have published a demo script