neovim-gtk icon indicating copy to clipboard operation
neovim-gtk copied to clipboard

Use glib's async runtime

Open Lyude opened this issue 2 years ago • 15 comments

The big question is do we really care? Honestly using tokio feels a bit silly (and was mainly a choice I made since at that time, I had just started programming in rust), but also we'd have to go through the work of hooking up a glib Source and a number of other things… maybe it's good enough just to limit the tokio worker to one thread and keep things as-is?

Welcome to hear if anyone has strong opinions on this

Lyude avatar Jan 01 '23 23:01 Lyude

I will say that the few times in the past when I used a different async runtime I eventually migrated back to tokio because it's simply better and has wider support where some libraries depend on it specifically. If it's not holding us back then I think we might as well keep doing what we're doing until we find some good reason to switch.

jacobmischka avatar Jan 03 '23 16:01 jacobmischka

You're not really using much of the tokio API (nvim-rs seems completely runtime-agnostic fortunately), so switching should be quite simple. Instead of the async Mutex you'd use the one from the futures crate, etc.

The main advantage of doing this would be less resource usage, fewer threads, faster compile times and smaller binary size. And also you'd only have to deal with a single thread in the whole application application, making things like the UiMutex unnecessary.

IMHO it would be a good simplification of the codebase.

Also nvim-rs already depends on async-std and uses that internally, so additionally using tokio here seems like a bad idea. If you don't want to move to the glib async runtime, at least move from tokio to async-std. Having both in the same process is a waste of resources :)

we'd have to go through the work of hooking up a glib Source and a number of other things

What do you mean with that? You don't have to deal with any of the GSource things, you can just spawn async blocks on the default glib::MainContext just like you currently do with the tokio Runtime. Am I missing something? :)

sdroege avatar Jan 03 '23 17:01 sdroege

I forgot to add: I'd be happy to do this work if you want go ahead with that.

sdroege avatar Jan 03 '23 18:01 sdroege

You're not really using much of the tokio API (nvim-rs seems completely runtime-agnostic fortunately), so switching should be quite simple. Instead of the async Mutex you'd use the one from the futures crate, etc.

The main advantage of doing this would be less resource usage, fewer threads, faster compile times and smaller binary size. And also you'd only have to deal with a single thread in the whole application application, making things like the UiMutex unnecessary.

IMHO it would be a good simplification of the codebase.

Also nvim-rs already depends on async-std and uses that internally, so additionally using tokio here seems like a bad idea. If you don't want to move to the glib async runtime, at least move from tokio to async-std. Having both in the same process is a waste of resources :)

I'm not sure what you mean by this? nvim-rs uses futures-rs, but that's a bit different from async-std isn't it?

we'd have to go through the work of hooking up a glib Source and a number of other things

What do you mean with that? You don't have to deal with any of the GSource things, you can just spawn async blocks on the default glib::MainContext just like you currently do with the tokio Runtime. Am I missing something? :)

I was basically just talking about hooking up the nvim_rs::Neovim read/writer, I haven't looked too closely at whether or not glib provides anything like this in it's rust packages.

Lyude avatar Jan 04 '23 06:01 Lyude

Hm, TBH looking at the code in src/nvim/mod.rs yeah honestly it doesn't look like it'd be that much work at all to move things over. I think I'm fine with us moving over to glib's runtime as long as it's actually as idiomatic to use as tokio or async-std are with rust. FWIW too regarding async-std, I did actually originally write the async stuff for neovim-gtk with async-std and I'm fine using that runtime as well, I can't remember why I decided to go with tokio in the end.

Lyude avatar Jan 04 '23 07:01 Lyude

I'm not sure what you mean by this? nvim-rs uses futures-rs, but that's a bit different from async-std isn't it?

Ah, I misread. It optionally depends on async-std, just like it optionally depends on tokio. Sorry for the confusion :) So makes not much of a difference whether tokio or async-std.

I was basically just talking about hooking up the nvim_rs::Neovim read/writer, I haven't looked too closely at whether or not glib provides anything like this in it's rust packages.

From looking through it, I think that all exists.

I think I'm fine with us moving over to glib's runtime as long as it's actually as idiomatic to use as tokio or async-std are with rust.

It's not as polished as async-std/tokio, but for the things you're doing here it seems sufficient. And like I said, it will simplify the other parts of your code (there will only be one thread in your whole application code, for example) and reduce compile times / binary size, so I think it's worth it.

sdroege avatar Jan 04 '23 08:01 sdroege

(forgot to remove that label :)

Lyude avatar Jan 06 '23 22:01 Lyude

(forgot to remove that label :)

I.e. let's go ahead with this?

sdroege avatar Jan 07 '23 08:01 sdroege

(forgot to remove that label :)

I.e. let's go ahead with this?

Yeah I think so :), ~~btw, thoughts on #80 with this?~~ nevermind, I see you already answered that!

Lyude avatar Jan 07 '23 19:01 Lyude

Also as long as tokio is used, it would make sense to limit its runtime to a single worker thread or so. It's not like we need one thread per core for that little work that goes through the runtime :)

sdroege avatar Jan 14 '23 18:01 sdroege

Oh BTW, @sdroege did you want me to assign this to you?

Lyude avatar Jan 24 '23 02:01 Lyude

Yes please. I'm waiting for the TCP PR to be merged before starting with this though

sdroege avatar Jan 24 '23 08:01 sdroege

BTW - no rush on getting this done ♥

Lyude avatar Jan 26 '23 20:01 Lyude

Hey! Just curious if anything ever ended up happening with this? FWIW: I've still been around, I've just been a bit pre-occupied with some 3D printing stuff I started working on in my spare time :), just in case it looked like I abandoned this project

Lyude avatar Jun 19 '23 18:06 Lyude

It's still on my list but I'm too busy right now. If someone else wants to take this, please go ahead. Otherwise I'll get to it at some point :)

sdroege avatar Jun 26 '23 11:06 sdroege