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

plugin: optimize OS detection logic for Neovim. #88

Merged
merged 1 commit into from Oct 26, 2020

Conversation

averms
Copy link
Contributor

@averms averms commented Sep 13, 2020

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
Copy link
Owner

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
Copy link
Contributor Author

averms commented Sep 14, 2020

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

@eraserhd
Copy link
Owner

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
Copy link
Contributor Author

averms commented Sep 19, 2020

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
Copy link
Contributor Author

averms commented Sep 19, 2020

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
Copy link
Owner

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

@averms
Copy link
Contributor Author

averms commented Oct 23, 2020

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
Copy link
Contributor Author

averms commented Oct 23, 2020

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

@eraserhd
Copy link
Owner

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

@eraserhd eraserhd merged commit 998484b into eraserhd:master Oct 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants