Skip to content
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.

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

bindeval / funcrefs / if_py, if_lua #1898

Open
abstiles opened this issue Jan 27, 2015 · 42 comments
Open

bindeval / funcrefs / if_py, if_lua #1898

abstiles opened this issue Jan 27, 2015 · 42 comments
Labels
api libnvim, Nvim RPC API compatibility compatibility with Vim or older Neovim performance issues reporting performance problems provider rpc
Milestone

Comments

@abstiles
Copy link

The 'vim' module exposed to python calls appears to be missing a few classes/functions/methods compared to Vim (at least) 7.4. This results in errors when attempting to use the powerline plugin for Vim.

Error detected while processing function provider#python#Call:
line    1:
AttributeError("'Nvim' object has no attribute 'bindeval'",)

Executing :python import vim; print dir(vim) gets you the following output:

In ordinary Vim:

['Buffer', 'Dictionary', 'Function', 'List', 'Options', 'Range', 'TabPage',
'VAR_DEF_SCOPE', 'VAR_FIXED', 'VAR_LOCKED', 'VAR_SCOPE', 'VIM_SPECIAL_PATH',
'Window', '_Loader', '__doc__', '__name__', '__package__', '_chdir', '_fchdir',
'_find_module', '_get_paths', '_getcwd', '_load_module', 'bindeval', 'buffers',
'chdir', 'command', 'current', 'error', 'eval', 'fchdir', 'find_module',
'foreach_rtp', 'options', 'os', 'path_hook', 'strwidth', 'tabpages', 'vars',
'vvars', 'windows']

In NeoVim:

['__class__', '__delattr__', '__dict__', '__doc__', '__format__',
'__getattribute__', '__hash__', '__init__', '__module__', '__new__',
'__reduce__', '__reduce_ex__', '__repr__', '__setattr__', '__sizeof__',
'__str__', '__subclasshook__', '__weakref__', '_session', 'buffers',
'channel_id', 'chdir', 'command', 'current', 'err_write', 'error', 'eval',
'fchdir', 'feedkeys', 'foreach_rtp', 'from_session', 'input',
'list_runtime_paths', 'metadata', 'options', 'out_write', 'quit',
'replace_termcodes', 'session', 'strwidth', 'subscribe', 'tabpages',
'ui_attach', 'ui_detach', 'ui_try_resize', 'unsubscribe', 'vars', 'vvars',
'windows', 'with_hook']

I'm not entirely sure if the need for any/all of these is obviated by some of the NeoVim changes, but at least the powerline plugin explicitly relies on "bindeval," which NeoVim does not expose.

@fwalch
Copy link
Member

fwalch commented Jan 27, 2015

From :help nvim-python-intro:

For now only the old Vim 7.3 API is supported.

So this has just not been implemented yet; I don't know when this will happen, though.

@abstiles
Copy link
Author

Ah, so the problem is that some plugins (like powerline) check for the version, and if the version reports >703, they assume they have access to the 7.4 API. Since the 7.4 API is not supported, does it really make sense for NeoVim to report its version as 704?

@fwalch
Copy link
Member

fwalch commented Jan 27, 2015

Hm, that's a bit tricky. Neovim has been forked from Vim 7.4.160, so it does provide a 7.4 API.. in Vimscript, that is. It's only the Python API that only supports the old commands.

@abstiles
Copy link
Author

I see. So NeoVim is just in an awkward situation when it comes to Python support. Is there an existing issue tracking the work to support the 7.4 Python API? I'd like to follow it to know when I might be able to try the powerline plugin again.

@justinmk
Copy link
Member

There's no issue for that because it's low priority. Maybe @ZyX-I would consider adding a condition to check has("nvim"), and possibly add support for the nvim python client in the future.

@elmart
Copy link
Contributor

elmart commented Jan 27, 2015

AFAIR, bindeval is the only "important" thing that is missing. Most common Python plugins out there run without problems, unless they use bindeval (which most of them don't).
Now that plugin arch has stabilized, maybe @tarruda could comment if support for bindeval is feasible. Would be a pity having to advertise neovim as 7.3 API only just because of that.

@justinmk
Copy link
Member

Both bindeval() and the Neovim python client are new. Isn't it reasonable to expect plugin authors to choose one or the other? After all, choosing bindeval() is a choice not to support vim 7.3. (or to add separate logic that does support it). If plugin authors are willing to support vim 7.3. and 7.4, they can support neovim too, or instead.

@elmart
Copy link
Contributor

elmart commented Jan 27, 2015

I think it's in our own interest to be as much a drop-in replacement for vim as possible. Specially for what regards plugins. Many plugin authors won't write neovim-specific code for some time yet. And one user wanting to try neovim but discovering one of his preferred plugins is not supported is one less user.

So, if there's a strong reason not to support bindeval, then it's ok. But if not, then I would support it, even if non-optimal. In other words, we should try to make writing neovim-specific code something optional, as far as we can.

Of course, speaking is easy. I don't really know what the obstacle for implementing bindeval is, certainly. So I don't know how much costly it would be.

@aktau
Copy link
Contributor

aktau commented Jan 27, 2015

I agree with @elmart, and equally so I don't remember exactly why we don't support bindeval, but something tells me it was definitely not trivial.

@tarruda
Copy link
Member

tarruda commented Jan 28, 2015

So, if there's a strong reason not to support bindeval, then it's ok. But if not, then I would support it, even if non-optimal. In other words, we should try to make writing neovim-specific code something optional, as far as we can.

Here are some reasons for not implementing bindeval:

  • Main reason: bindeval was introduced in 7.4 and does not seem to have widespread usage, so I don't think its worth the effort
  • It would be very complex to correctly maintain reference count across process for garbage collection. The most sensible approach would be to use weak references in the python host(which does not fully emulate the current bindeval behavior)
  • It would make the API more complex(we would need reference types for vimscript lists and dicts and methods to operate on those) just for the sake of maintaing 100% compatibility with python/lua APIs
  • IMO there's little reason for the existence of bindeval, I've done some benchmarks, and its possible to transfer more data than any plugin should ever need without noticeable delay(and that would be even faster for vim-python since its everything in the same process). If for some other reason one absolutely needs to manipulate vimscript objects directly, its possible to do so with vimscript functions that are called from python.

With that said, it may be possible to emulate bindeval without changes to the API by including some support vimscript that is called from python. For example, consider this vimscript code:

let s:objects = {}
let s:id = 1

function! BindEval(str)
  let id = s:id
  let s:id += 1
  let s:objects[id] = eval(str)
  return id
endfunction

function! Unref(id)
  call remove(s:objects, a:id)
endfunction

function! DictGet(id, key)
  let object = s:objects[a:id]
  if type(object) != type({})
    throw 'Not a dict'
  endif
  return object[a:key] 
endfunction

" Plus some other functions to manipulate dicts/lists

Clearly the above approach still would have to solve problems of resource leak(when should a remote reference be freed?), but at least no changes to the C API are required. As I said, I haven't done this yet because I don't think its worth the effort(I never used any plugin that depends on bindeval), but anyone is welcome to do so.

@ZyX-I
Copy link
Contributor

ZyX-I commented Jan 28, 2015

IMO there's little reason for the existence of bindeval, I've done some benchmarks, and its possible to transfer more data than any plugin should ever need without noticeable delay(and that would be even faster for vim-python since its everything in the same process). If for some other reason one absolutely needs to manipulate vimscript objects directly, its possible to do so with vimscript functions that are called from python.

Input:

  1. A few KiB of binary data.
  2. Using usual NL-used-for-Nul encoding within list of strings (i.e. what readfile(, 'b') returns).
  3. Pure Python and VimL implementation.

The second and third condition leads to the very expensive operation of dumping the data into a string coded in Python compared to json.dumps coded in C. This did a noticeable increase of performance in aurum, though I do not remember exact numbers (something like a one third of time cut). Though maybe it is possible to write json.dumps replacement the other way in Python so it is more performant.

vim.bindeval gets rids of both the serialising and deserialising steps with conversion as simple as replacing Nuls with NLs (done by str.replace, not by bindings themselves).

I am pretty sure that if you try Powerline in NeoVim unchanged with code for vim-7.0 (yes, this is supported) there will be greater problems then absense of bindeval: due to IPC and lots of blocking requests over it that are being made for a single &stl evaluation this is not going to work well I guess.

@elmart
Copy link
Contributor

elmart commented Jan 28, 2015

@tarruda Ok. I still don't fully understand the ins and outs of this, but I'll take your word for it.

@ZyX-I I'm not very sure about the meaning of your previous message. In any case: You're the main contributor to powerline. It has become a pretty popular plugin. So, having it not working with neovim is for sure taking many users away from us. You're also a very important contributor here. Then, have you got plans to add neovim support to poweline?

@tarruda
Copy link
Member

tarruda commented Jan 28, 2015

@tarruda Ok. I still don't fully understand the ins and outs of this, but I'll take your word for it.

Here's some background on vim.eval and vim.bindeval to help understand:

  • vim.eval: Evaluate vim expression, recursively convert to a python object and pass to python code. There's no link between the converted and the original objects.
  • vim.bindeval: Evaluate vim expression, wrap into a python native class and return it, also increasing the reference count to the original vim object. The native class implements methods that operate directly on the original vim object(so when you do d = vim.bindeval('{}'); d['key'] = 'value' it is actually changing a vimscript dict, not a python dict) . When the python wrapper object is about to be garbage collected, the reference count to the vim object is decreased

I am pretty sure that if you try Powerline in NeoVim unchanged with code for vim-7.0 (yes, this is supported) there will be greater problems then absense of bindeval: due to IPC and lots of blocking requests over it that are being made for a single &stl evaluation this is not going to work well I guess.

Is this only a guess or did you actually try it? YouCompleteMe constantly transfers the entire buffer between vim and python(not to mention to its daemon) and it still performs well even with big files such as eval.c

@elmart
Copy link
Contributor

elmart commented Jan 28, 2015

Here's some background on vim.eval and vim.bindeval to help understand:

Yes, yes, I know about those. The part I don't fully understand is why new neovim plugin architecture plays bad with that. I can imagine, more or less, but haven't gone into the details.

@ZyX-I
Copy link
Contributor

ZyX-I commented Jan 28, 2015

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

On January 28, 2015 2:13:45 PM EAT, Thiago de Arruda notifications@github.com wrote:

@tarruda Ok. I still don't fully understand the ins and outs of this,
but I'll take your word for it.

Here's some background on vim.eval and vim.bindeval to help
understand:

  • vim.eval: Evaluate vim expression, recursively convert to a python
    object and pass to python code. There's no link between the converted
    and the original objects.
  • vim.bindeval: Evaluate vim expression, wrap into a python native
    class and return it, also increasing the reference count to the
    original vim object. The native class implements methods that operate
    directly on the original vim object(so when you do d = vim.bindeval('{}'); d['key'] = 'value' it is actually changing a
    vimscript dict, not a python dict) . When the python wrapper object is
    about to be garbage collected, the reference count to the vim object is
    decreased

I am pretty sure that if you try Powerline in NeoVim unchanged with
code for vim-7.0 (yes, this is supported) there will be greater
problems then absense of bindeval: due to IPC and lots of blocking
requests over it that are being made for a single &stl evaluation this
is not going to work well I guess.

Is this only a guess or did you actually try it? YouCompleteMe
constantly transfers the entire buffer between vim and python(not to
mention to its daemon) and it still performs well even with big files
such as eval.c

I have not. YCM is irrelevant because expected problem is not data transfer rate and not even the speed of (de)serializing it like it was with aurum. Expected problem is an amount of context switches made by the processor. Each request (like vim.eval('&readonly')) makes process write to a socket and then wait for result and waiting ought to trigger switching to the next task. Also neovim is going to wait for :python to complete with the same result. And all this couples with switches made by the kernel for other reasons. Given that you can count for at least twenty requests per &statusline evaluation and that &statusline is evaluated at least once each time user presses a key (with a few exceptions) I do not need tests to have strong opinion this is not going to work well.

And since opinion is rather strong I never actually tried to test. Do not think this will be too problematic: correct me if I am wrong, but API from around vim-7.0.112 (this is the first version with Python support which I was able to compile; it is now used in integration tests) is fully supported. I have a machine though where I almost never experience any performance problems with powerline, even though I sometimes get reports about them. If my opinion proves right even here the only reasonable variant will be a complete rewrite of Vim bindings.


Reply to this email directly or view it on GitHub:
#1898 (comment)

-----BEGIN PGP SIGNATURE-----
Version: APG v1.1.1

iQJNBAEBCgA3BQJUyNg8MBwfMDI7PjIgHTg6PjswOSAQOzU6QTA9NEA+MjhHIDxr
cC1wYXZAeWFuZGV4LnJ1PgAKCRBu+P2/AXZZIulKD/9fxyn9wxxRmkqw46QiVM9u
RFluIjJMOsB5t8SLRBAx2HZwIbt5Y8nhIyCjOjl2gx+rsLBKHDFBujzjntrITuSG
YsF3TBQPO77N1+rztCv/o+ZBgK4y1Y2gyaPdOnkUigasEtd5Kw45O5kZPHHneFAa
Ia87NuCiJMXr6VaouAs5irE8txUVue3njCdENGjjnQYQ3932TzPWIxhSPQZb91WO
UrrYx1m/HdFX12RKvp2NOT6XVxqyyc+yl+TGH750MVAy2zprRHLcXr3335zx47X3
ppLqm05qva8k7WXiRfVPOjPo1IJQSztkD2aon4FR8fYUf4IeHWf0m37hfVbDcD2N
V1ZpvyAy0XhG/8ZyJPQD4BsYI3ps1KOIXBVl5NlY4rhuLd7E7bYJZivXYj1QrVTB
3eS/7VeZN8QKlLwsLEnBrccJMOOEKoLpxEvEJYk82BSxoKt97m6DnV2K3bqkcxS4
BzeCDThPg7ODrnfo7wHKcaYxw1p1KBJKNqyOcijiI7zOQHz2pp935X4frXNeysnh
NZhlYiyyBq88ljKOn867hk3uKDBlVGRZiSOCykrNjo0GwUjykfRb6qJzakL/5rWj
HeogrJCFpmPdFBJpRq/1F7A4Qpz1GSEop0fRdbZYThOb+UxAwQkoGcaPiEtIEY2W
iV7z376fS25ogNvoaw4Yjg==
=Vvg1
-----END PGP SIGNATURE-----

@ZyX-I ZyX-I added api libnvim, Nvim RPC API compatibility compatibility with Vim or older Neovim labels Jan 28, 2015
@tarruda
Copy link
Member

tarruda commented Jan 28, 2015

Yes, yes, I know about those. The part I don't fully understand is why new neovim plugin architecture plays bad with that. I can imagine, more or less, but haven't gone into the details.

Its more a matter of not being worth the effort than limitations of the plugin architecture.

The problematic part is managing the lifetime of vim objects, which could be done by using weak references(when the vimscript refcount reaches 0, release the memory and throw an exception when the python code tries to access it later) but that is not 100% compatible with the current interface, and since compatibility would be the main goal this kinda becomes pointless.

The much more complex alternative would be to maintain references across the plugin host, but there are more cases to deal with(plugin host holding references is killed, for example).

Both approaches would require making the C API more complex, again for the sake of compatibility with very few plugins.

@tarruda
Copy link
Member

tarruda commented Jan 28, 2015

And since opinion is rather strong I never actually tried to test. Do not think this will be too problematic: correct me if I am wrong, but API from around vim-7.0.112 (this is the first version with Python support which I was able to compile; it is now used in integration tests) is fully supported. I have a machine though where I almost never experience any performance problems with powerline, even though I sometimes get reports about them. If my opinion proves right even here the only reasonable variant will be a complete rewrite of Vim bindings.

I'm not sure if I understood you correctly, but are you suggesting a rewrite of neovim remote interface as the only reasonable choice for using powerline? If so, thats a very radical thought and I couldn't disagree more. Here are a couple of more reasonable approaches that comes to mind:

  • Adapt neovim-python compatibility layer to use a hack like I suggested in this comment
  • Adapt powerline code to not depend on bindeval(which can also be done by creating wrapper vim functions for accessing/modifying objects) and as an added bonus, make it compatible with vim 7.3

Even if bindeval turns out to be an absolute requirement for powerline(according to your untested assumptions), I bet most other plugins can easily switch away from it without noticeable impact, here's one that did it.

Another choice powerline users have is to switch to vim-airline which seems to have a lot of powerline features without the python dependency.(Not sure if its a drop-in replacement for powerline, but it seems to be more popular)

Let me conclude by reminding you that while the new remote plugin infrastructure won't have the same efficiency for marshaling data from/to nvim, I say it will be fast enough for 90% of the plugins out there. It also has the following advantages which I may already have mentioned:

  • Sandboxing: Plugins have a lesser chance of crashing the editor
  • Better threading support. Even if vim had thread-safe mechanisms for communicating with python threads, any signal delivered to vim could crash everything(ref)
  • Support for plugins in programming languages that do not support embedding:

IMO the extra context switches and CPU cycles are heavily outweighted by these advantages.

@ZyX-I
Copy link
Contributor

ZyX-I commented Jan 28, 2015

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

On January 28, 2015 5:45:06 PM EAT, Thiago de Arruda notifications@github.com wrote:

And since opinion is rather strong I never actually tried to test. Do
not think this will be too problematic: correct me if I am wrong, but
API from around vim-7.0.112 (this is the first version with Python
support which I was able to compile; it is now used in integration
tests) is fully supported. I have a machine though where I almost never
experience any performance problems with powerline, even though I
sometimes get reports about them. If my opinion proves right even here
the only reasonable variant will be a complete rewrite of Vim bindings.

I'm not sure if I understood you correctly, but are you suggesting a
rewrite of neovim remote interface as the only reasonable choice for
using powerline? If so, thats a very radical thought and I couldn't

NeoVim has an entity called "Vim bindings"? I have never heard of it. More details is in powerline issue tracker, powerline/powerline#1287.

disagree more. Here are a couple of more reasonable approaches that
comes to mind:

  • Adapt neovim-python compatibility layer to use a hack like I
    suggested in this
    comment
  • Adapt powerline code to not depend on bindeval(which can also be done
    by creating wrapper vim functions for accessing/modifying objects) and
    as an added bonus, make it compatible with vim 7.3

Even if bindeval turns out to be an absolute requirement for
powerline(according to your untested assumptions), I bet most other
plugins can easily switch away from it without noticeable impact,
here's one that
did it.

Another choice powerline users have is to switch to
vim-airline which seems to have
a lot of powerline features without the python dependency.(Not sure if
its a drop-in replacement for powerline, but it seems to be more
popular)

Let me conclude by reminding you that while the new remote plugin
infrastructure won't have the same efficiency for marshaling data
from/to nvim, I say it will be fast enough for 90% of the plugins out
there. It also has the following advantages which I may already have
mentioned:

  • Sandboxing: Plugins have a lesser chance of crashing the editor
  • Better threading support. Even if vim had thread-safe mechanisms for
    communicating with python threads, any signal delivered to vim could
    crash everything(ref)
  • Support for plugins in programming languages that do not support
    embedding:

IMO the extra context switches and CPU cycles are heavily outweighted
by these advantages.


Reply to this email directly or view it on GitHub:
#1898 (comment)

-----BEGIN PGP SIGNATURE-----
Version: APG v1.1.1

iQJNBAEBCgA3BQJUyRBMMBwfMDI7PjIgHTg6PjswOSAQOzU6QTA9NEA+MjhHIDxr
cC1wYXZAeWFuZGV4LnJ1PgAKCRBu+P2/AXZZIrLmD/oDEVMsviDFazm5aZ8kdcE8
1A2TGDrysRVtKdyXARZIps13iAMMl2P+Cx0oW03oI3EZjVMbX3ksSPWmhWRPg2tE
MS7BT3rm9kBX716KVh+kjY4074eUX4sQ6/I+x6+G0HNhY0SeJQRKSeabKx10ENAu
tYffu7FCSsY9NZxJG6Qlef3bxSs6bS6ZYn6XJHGuXDHi50POWVbL21E+Bk69fDwS
C2MTw7ORWSeftdrdNaCKos6sJRHblTptvA4tBkiFssP+KkE+YC1T+ET2cLfQszGv
yD433YAljb4G1lRLnCpaX1RWXBJre79m/H8rXgn3LBYOlBIoobkiyqfW6/BX4qM8
aHFA5IMqSahY8Eo7oYwXf3pQI8tj1ZPs//Mgnv9pWeClW/BQQJe0eHZ12zSTkp8i
9ZEVEoDJg7gPKX5+zG911asypOmGK1eieWftWuILimgmfazmTrYDP99Xba+up/aO
p6rAa2bOeRYRC1xxKNEMRVaQYs/F5gPhsryTwga8sFQkCvanIR2EVK2WLpuc/pMI
AZI0cdAv8+fo3ms2/HdRwnDcp6vhggdboQ3J0fruEo8KVMTukbXKbAV+fTWXk/Lg
ZOL6Kl5/T/NWEFJX+d32uMz35Bigk4rY1LWsIcPPFlFZvM9djyknwW40ICEO0Cn+
P+30pZJ67h1f9hTdwuHINQ==
=1wf9
-----END PGP SIGNATURE-----

@Shougo
Copy link
Contributor

Shougo commented Jan 28, 2015

I need weak reference for if_lua compatibilities.
It is not neovim's if_python problem.

@aktau
Copy link
Contributor

aktau commented Jan 28, 2015

Let me try to recap if I can follow the conversation. To support this, something like this would be needed in the C api:

/// references by external plugins (no sanity checks)
int vim_ref(oid_t id) { return objmap[id].refcount++; }
int vim_unref(oid_t id) { return objmap[id].refcount--; }

/// returns kInvalidVimObject if there was a problem with the expression
oid_t vim_bindeval(const char *expr) {
  // eval expression
  oid_t id = ...;

  int refcnt = vim_ref(id);
  assert(refcnt == 1);

  return id;
}

Where objmap[] would be (for efficiency's sake) a map storing only the vim variables that have been shared with external plugins.

(this is likely horribly misguided, but I need a mental model and I'm not that well versed in our external API)

A concern with this is that now plugins can "crash" neovim by referencing a lot of memory, dying (not dereferencing them) and then trying again until the OOM killer comes for neovim. Perhaps a per-plugin map is better, which could be cleaned whenever a plugin exists.

Anyway, from @ZyX-I's explanation about the context switches that a socket read/write would provoke, I assume that the problem wouldn't be fixed even if bindeval where to implemented for Neovim. Every time the variable is read/written, it would need to send/recv on the socket anyway. In fact, I think the performance could be worse than just copying the entire object everytime (depending on object size, of course). In which case there would be only one send or recv per object per action.

Please correct me on everything.

@ZyX-I
Copy link
Contributor

ZyX-I commented Jan 29, 2015

@aktau I understand it this way:

  1. Object vim_bindeval(String expr) with typedef struct { ObjectId id; ObjectType type; } ObjectReference; that is one of the possible choices for union in Object (note: vim_bindeval may just as well return scalar values). This thing will increase the internal object reference counter by one and save object to ObjectReferenceDetailed *objlist; (typedef struct { ObjectReference ref; void *object; } ObjectReferenceDetailed;).
  2. Broadcasted message “please list all objects you currently posess”. If no client claims it posesses some object it is unreferenced and removed from objlist.
  3. API function void vim_disown(ObjectReference objref);.
  4. API function void vim_hasobj(ObjectReference objref); (this is being called in response to 2.).

The idea is that objects are normally obtained using vim_bindeval API function and deleted using vim_disown when needed. To prevent memory leaks when clients owning objects suddenly disconnected p. 2. is there. No vim_ref/vim_unref API functions provided: API knows only of one reference per vim_bindeval invocation just as it does in Vim now.

@justinmk
Copy link
Member

I tried to follow @ZyX-I explanation, but I don't see how implementing this in neovim would afford any performance benefit.

vim.bindeval: Evaluate vim expression, wrap into a python native class and return it, also increasing the reference count to the original vim object. The native class implements methods that operate directly on the original vim object(so when you do d = vim.bindeval('{}'); d['key'] = 'value' it is actually changing a vimscript dict, not a python dict)

So this would provide a compatible behavior, but underneath it's still making the same msgpack calls that would otherwise be done without bindeval.

It could be quite confusing to plugin authors, if bindeval() is provided, yet it has no performance benefit.

@ZyX-I
Copy link
Contributor

ZyX-I commented Jan 29, 2015

On January 29, 2015 7:43:46 AM EAT, "Justin M. Keyes" notifications@github.com wrote:

I tried to follow @ZyX-I explanation, but I don't see how implementing
this in neovim would afford any performance benefit.

vim.bindeval: Evaluate vim expression, wrap into a python native
class and return it, also increasing the reference count to the
original vim object. The native class implements methods that operate
directly on the original vim object(so when you do d =
vim.bindeval('{}'); d['key'] = 'value' it is actually changing a
vimscript dict, not a python dict)

So this would provide a compatible behavior, but underneath it's still
making the same msgpack calls that would otherwise be done without
bindeval.

It could be quite confusing to provide bindeval() if it has no
performance benefit.


Reply to this email directly or view it on GitHub:
#1898 (comment)

vim.bindeval will only get rid of (de)serialization of objects required by pyeval, but at a cost of a few more msgpack calls. Benefit may be there only for long-living objects (additional calls are only needed at startup) and should be smaller then for Vim.

Actually just vim.call_func(funcname, args, self=None) and vim.locals (which is something like the output of vim.bindeval('l:'), but without mechanics for keeping ref in Python, *always modifying only the current scope and returning non-binded Python objects on .getitem() or .values()/.items()) should be enough if you talk about performance: this is getting rid of unnecessary (de)serialisation (msgpack (de)serialisation stays, of course, but it will only be one, not like serialise to json then resulting string to msgpack) and is thus not more performant, but also more convenient.

I was actually saying how to be compatible with bindeval, not how to increase performance.

@tarruda
Copy link
Member

tarruda commented Jan 29, 2015

I need weak reference for if_lua compatibilities.
It is not neovim's if_python problem.

@Shougo I understand you need if_lua compatibility for your plugins, unfortunately I cant focus on this right now because there are higher priorities(UI refactor, terminal emulation for bang commands, windows support).

If you want to improve lua-client to implement if_lua compatibility I can provide some guidance, but it may be better to write from the scratch using luvit due to the better standard library and module system.

@tarruda
Copy link
Member

tarruda commented Jan 29, 2015

Anyway, from @ZyX-I's explanation about the context switches that a socket read/write would provoke, I assume that the problem wouldn't be fixed even if bindeval where to implemented for Neovim. Every time the variable is read/written, it would need to send/recv on the socket anyway. In fact, I think the performance could be worse than just copying the entire object everytime (depending on object size, of course). In which case there would be only one send or recv per object per action.

Please correct me on everything.

Thats the big issue with implementing bindeval, it would add too much API complexity only for the sake of compatibility layer that would be sub-optimal for plugins running over msgpack-rpc. The only added advantage I can see is having direct access to deep/private objects. For example:

nested_list = vim.bindeval("g:dict1['dict2']['list']")
nested_list.append(1)

It is simply not worth adding a bunch of C functions and types to the API just for this.

However, it just ocurred to me that it may be possible to fully implement bindeval(with proper garbage collection) in pure vimscript, all we need to do is expose a ChannelDestroyed autocommand that can be used to delete all references made by a certain channel. It would certainly improve the cost of this compatibility layer.

@tarruda
Copy link
Member

tarruda commented Feb 2, 2015

Thank you. Can you provide the gidence?

The lua client is very similar to the python client in how the code is organized, it has four main layers:

   Event loop(data connection to Neovim)
                | ^
                | |
                | |
                v |
      msgpack parser/serializer               
                | ^
                | |
                | |
                v |
         msgpack-rpc async session API
                | ^
                | |
                | |
                v |
          msgpack-rpc sync session API(uses coroutines in lua and greenlets in python)

The big difference is that the lua client event loop can only connect to a child nvim process(used for functional tests). There are 3 transport types that the lua client doesn't support yet:

  • TCP
  • Unix socket
  • stdin/stdout <- This one is required for msgpack-rpc plugins.

These transport types must be added in the event loop layer(the rest doesn't need to be changed)

So you have two choices:

  • Modify the lua client to support stdin/stdout connection. This must be done in the loop.c file because libuv API is used directly.
  • Create a new client from the scratch using a better platform such as luvi. The advantages are:
    • It has better standard library
    • It already has libuv bindings so no C code is required.

I recommend you to write from the scratch with luvi using the same code organization, its not a huge amount of work(the lua client has 600 loc, but 300 is for the libuv C bridge). If you do, read the lua/python client source code in top->bottom order:

If you have any questions, feel free to ask

@ZyX-I
Copy link
Contributor

ZyX-I commented Feb 7, 2015

It appears that I cannot use Python client at all: it simply hangs whenever I do :python import vim or connect via from neovim import attach; nvim = attach('socket', path='…').

@ZyX-I
Copy link
Contributor

ZyX-I commented Mar 7, 2016

I actually need every method Vim defines for vim.List, vim.Dictionary and vim.Function in if_py_both, this is more than I have already listed. Guess this is better to be put into src/nvim/api/ref.c. And construct a structure like PyTypeObject in place of ReferenceType enum just in case (i.e. in extended-funcref my Vim branch I AFAIR was converting Python callables into Vim function-like objects, and AFAIR I did plan something like this for lua functions in luaviml). Plus dummy channel for internal uses (i.e. lua compatibility interface, it needs to keep reference table somewhere).

jamessan added a commit to jamessan/neovim that referenced this issue Oct 4, 2016
In order to provide better compatibility with the classic bindings, the
API needs to provide the ability to query the number (really index) of
the window/tabpage.

This is needed for neovim/pynvim#87, as discussed in
neovim#1898.

Signed-off-by: James McCoy <jamessan@jamessan.com>
jamessan added a commit to jamessan/neovim that referenced this issue Oct 4, 2016
In order to provide better compatibility with the classic bindings, the
API needs to provide the ability to query the number (really index) of
the window/tabpage.

This is needed for neovim/pynvim#87, as discussed in
neovim#1898.

Signed-off-by: James McCoy <jamessan@jamessan.com>
jamessan added a commit to jamessan/neovim that referenced this issue Oct 4, 2016
In order to provide better compatibility with the classic bindings, the
API needs to provide the ability to query the number (really index) of
the window/tabpage.

This is needed for neovim/pynvim#87, as discussed in
neovim#1898.

Signed-off-by: James McCoy <jamessan@jamessan.com>
jamessan added a commit to jamessan/neovim that referenced this issue Oct 4, 2016
In order to provide better compatibility with the classic bindings, the
API needs to provide the ability to query the number (really index) of
the window/tabpage.

This is needed for neovim/pynvim#87, as discussed in
neovim#1898.

Signed-off-by: James McCoy <jamessan@jamessan.com>
@justinmk justinmk changed the title Python 'vim' module is missing some attributes if_py (python): missing bindeval() Oct 19, 2016
@justinmk justinmk added the rpc label Jun 6, 2017
@justinmk justinmk changed the title if_py (python): missing bindeval() bindeval / funcrefs / if_py, if_lua Jun 6, 2017
@justinmk justinmk added performance issues reporting performance problems provider labels Aug 20, 2018
@neovim neovim deleted a comment from Shougo Apr 14, 2024
@neovim neovim deleted a comment from Shougo Apr 14, 2024
@neovim neovim deleted a comment from hotgloupi Apr 14, 2024
@neovim neovim deleted a comment from ZyX-I Apr 14, 2024
@neovim neovim deleted a comment from sassanh Apr 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api libnvim, Nvim RPC API compatibility compatibility with Vim or older Neovim performance issues reporting performance problems provider rpc
Projects
None yet
Development

No branches or pull requests