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

Make the Reactor public… or split it out? #14

Closed
Ekleog opened this issue Apr 22, 2020 · 3 comments
Closed

Make the Reactor public… or split it out? #14

Ekleog opened this issue Apr 22, 2020 · 3 comments

Comments

@Ekleog
Copy link

Ekleog commented Apr 22, 2020

Hello,

First, this looks like an awesome project! Being able to do that much with that little code, and having the ability to seamlessly spawn Send, !Send and blocking tasks looks great.

That being said, I came to smol with, as my main approach, “should a library choose to depend on smol rather than on tokio/async-std” (the best choice IMO being to depend on none, but that may or may not be reasonable depending on the API, too many generic arguments kill everything and taking a trait for every interaction with the outside world can be cumbersome to use… plus no one has written traits that could be used by all libraries and implemented by all executors yet, though I plan on doing that someday in the next twenty years if no one does it before).

And… it looks like (right now) Reactor isn't exposed. Which basically means that a library that chooses to depend on smol won't be usable from other executors without the application spawning a full smol, complete with its three executors. While it probably wouldn't cause any noticeable performance penalty, it still feels unclean.

Hence, I believe it might be helpful to expose the Reactor to the outside world, so that libraries could choose to depend on smol's Reactor and especially the Async struct -- like tokio once did.

Now, there are two ways to go for this: either just make it pub, or make it a completely separate crate. The advantage I would see in making it a completely separate crate would be that then, it becomes easier to port smol to new OSes: it'd just consist in forking the crate, and the user could then simply use the forked reactor, while still using upstream smol. Not sure there's a difference that's not purely theoretical, though the second one might appear a tad bit cleaner.

That'd basically mean splitting smol in two: the Reactor and Async struct, and the executor series and Task struct. Each would then (hopefully) be usable independently. (While still keeping, gated under a feature that's in the default set, the run function using smol's current Reactor for people who just want the easy thing)

What do you think about this idea? Am I missing a reason why that'd not be possible and/or a bad idea?

(also, if you're aiming for feature-parity, I think task_locals are something that can't currently be implemented in a reasonable fashion without support from the executor, so it may make sense to add support for them, but I'm not sure what level of feature-parity you're trying to achieve right now, so won't open an issue straight away -- I can do if you want)

Anyway, thank you for smol, which looks like a really impressive piece of software!

@ghost
Copy link

ghost commented Apr 22, 2020

Thank you for the kind words and insightful feedback! <3

I came to smol with, as my main approach, “should a library choose to depend on smol rather than on tokio/async-std"

True, how should we make async libraries interoperate is still an open question. Some libraries choose to depend on async-std, some on tokio, some on both depending on feature flags, and some on no runtime.

I wonder - perhaps there is another way, to make a runtime built into the library? Think of big async libraries like hyper, tide, tonic, surf, or sqlx. Is it a bad idea to simply have a hidden lightweight runtime built into the library?

In fact, surf depends on isahc, which uses curl in the background, and curl has its own epoll! So in a way, those libraries have a built-in runtime already, and they work really well.

There's another crate like this - PingCAP's grpcio. It's a wrapper around gRPC Core, which a C framework that also has built-in networking, and even a simple executor. And it also works well.

Database libraries could have an epoll instance handling a connection pool to the database, and perhaps a single thread driving epoll. Yes, it would be even better if that thread wasn't necessary, but is that after all such a terrible solution?

Don't get me wrong - I don't expect every single async library to have a built-in runtime, but perhaps it'd be totally okay for the several biggest libraries to have built-in runtimes.

Even some libraries in Go roll their own epoll, e.g. https://github.com/fsnotify/fsnotify

also, if you're aiming for feature-parity, I think task_locals are something that can't currently be implemented in a reasonable fashion without support from the executor, so it may make sense to add support for them

I'm curious - what do you need task-locals for? My guess is for logging, but thought I'd ask anyway.

In theory, task-locals can be implemented without support from the executor as a separate library. Imagine you wrap spawned futures in something like:

let task = Task::spawn(SupportTaskLocals(future));

Then, SupportTaskLocals puts task-locals inside thread-locals every time the future is polled.

@Ekleog Ekleog mentioned this issue Apr 22, 2020
@Ekleog
Copy link
Author

Ekleog commented Apr 22, 2020

I've split out the discussion about task locals into #15 ; so as to simplify the discussions :)

As for splitting the Reactor out, the more I think about it, the more I must say I don't have a great many concrete arguments in favor. So I'm going to first answer on one of your sentences:

Database libraries could have an epoll instance handling a connection pool to the database, and perhaps a single thread driving epoll. Yes, it would be even better if that thread wasn't necessary, but is that after all such a terrible solution?

The issue with something like that is that then, a no_std system that doesn't have threads (but has its custom implementation of a network stack) could not use this database library. Now… that's not something that would be solved by making the Reactor public anyway, unless that embedded system also supported epoll or similar, which sounds… quite unlikely. So… maybe it's not as bad as I was originally thinking: anyway, libraries that depend on just a reactor will have the same issues, and having a smol thread running in order to be able to use it from another executor is probably no big deal.

Right now, the reason I am currently thinking of splitting the Reactor out is mostly to be able to use the executor alongside another reactor, on a platform unsupported by the current reactor. For instance, a platform where thread::spawn is legitimate, but the *poll mechanism is completely different, for a target that has a small enough userbase that it'd be quite unlikely to be upstreamed into smol (thinking right now of a potential seL4 userspace).

In other words, I've completely changed the approach, and am now thinking about applications that would want to use the (awesome-looking) smol executor on top of a system unsupported by the smol reactor.

Now, I'm giving this new approach even a bit more thought, and I now remember IoEvent being required for the executors. Meaning that a fork of the whole smol would be required anyway.

So… the reactor being made independent of the executor brings little advantage (just one fewer thread running), and the executor being made independent of the reactor is (currently at least) not a technical possibility.

So… maybe we should close this? At least I feel like my idea burned itself down with your help, thank you for helping me see it was pointless anyway! :)

@ghost
Copy link

ghost commented Apr 25, 2020

Thanks for sharing your thoughts! :) I appreciate you writing all this up, it definitely gives me a fresh perspective on things.

I guess the next step here right now is "do nothing", i.e. more feedback/thinking is needed. Closing this issue, but if anything new comes up, feel free to reopen and continue.

@ghost ghost closed this as completed Apr 25, 2020
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

1 participant