search

eraserhd / parinfer-rust

A Rust port of parinfer.
ISC License
546 stars 42 forks source link

plugin: optimize OS detection logic for Neovim. #88

Closed averms closed 4 years ago

averms commented 4 years ago

OS detection in Vim requires using the system command "uname", which is very slow. This is not required in Neovim. Adding a specific case for Neovim improves startup times by up to 15 ms.

Before the change: https://0x0.st/iIQU.txt
After the change: https://0x0.st/iIQl.txt
Notice the large difference in total time and that nearly all of the change was in the single system('uname') command

eraserhd commented 4 years ago

I'm not liking the resulting code here, since it increases the surface area needed to test changes.

How about skipping the OS checks altogether, and just testing if each of "libparinfer_rust.so", "libparinfer_rust.dylib", and "parinfer_rust.dll" can be loaded, in sequence, with libcall, and choosing the first one found?

averms commented 4 years ago

Yes, it currently is an absolute mess of conditionals. How would libcall() perform?

eraserhd commented 4 years ago

Actually, I guess it would perform the same in theory. :(

Ok, how about moving the conditionals into a function and calling that function just before the existing libcall(), therefore deferring the cost until the first time plugin is used?

averms commented 4 years ago

Ok, how about moving the conditionals into a function and calling that function just before the existing libcall(), therefore deferring the cost until the first time plugin is used?

That seems like a fine idea.

averms commented 4 years ago

Do you mean moving the current has checks or your proposed switch to running libcall for every type of extension. I think the latter would be better, since we're deferring the cost anyway.

eraserhd commented 4 years ago

I like the latter better also, but would accept the first also.

averms commented 4 years ago

I like the latter better also, but would accept the first also.

I ended up going with the first one because the help text for libcall() seemed to caution against using it without knowing it is going to actually work. Also it would require me to learn how to use exceptions with Vimscript.

In addition, I wasn't sure how to make it so that we do one trial libcall() of all possible libraries and then use the correct one from then on. It would require some sort of variable setting in the catch blocks and something in the finally block. Something that would likely be more spaghetti than the status quo.

averms commented 4 years ago

We could also look at this for a somewhat simpler solution: https://vimways.org/2018/make-your-setup-truly-cross-platform/

eraserhd commented 4 years ago

I think the changes look good. Thanks! I queued the article for later reading.