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

Improve usability of spawn #29

Closed
mneumann opened this issue Jan 11, 2021 · 10 comments
Closed

Improve usability of spawn #29

mneumann opened this issue Jan 11, 2021 · 10 comments
Labels
enhancement New feature or request

Comments

@mneumann
Copy link

I'd love to see something like spawn_default which would use a sensible default Spawner depending on the configured cargo feature. For async-std, this would default to spawn(&mut AsyncStd).

Having a default spawner makes extensions easier. See my xtra-addons crate, which implements a Registry. The registry needs to be able to spawn a new actor, so that you can just type MyActor::from_registry (xactor has the same feature ;-). But spawning a new actor depends on the runtime, so that I have to provide an implementation myself (that's why I only support async-std atm).

If you like to have this feature, I am willing to implement.

@Restioson
Copy link
Owner

I'd love to see something like spawn_default which would use a sensible default Spawner depending on the configured cargo feature

Xtra had this, but this leads to cargo features being essentially broken. If two were enabled, then what would happen? This is why I changed it to take a spawner. Mutually exclusive features are generally not good practice with cargo because of how features enabled by different dependencies are unioned together. If, for instance, a library used xtra internally with with-smol-1_1 and you used it with with-tokio-1_1, then this would cause compile errors. Furthermore, AFAIK it would not technically be a breaking change for them to enable a new feature in xtra, so this is just potential for bad times all around. spawn(&mut Tokio::Global) is indeed less ergonomic than just .spawn() but I don't know any way to do it better without breaking the features in the way that I described.

But spawning a new actor depends on the runtime, so that I have to provide an implementation myself

You should be able to just take &mut Spawner like xtra does currently and use that, I believe. Are there any issues with this approach?

@Restioson Restioson added the enhancement New feature or request label Jan 11, 2021
@Restioson
Copy link
Owner

I thought of one possible way to do this. There could be a SpawnGlobalTokioExt and such for each, defined like so:

trait SpawnGlobalTokioExt {
    fn spawn_global(self) -> Address<Self> {
        self.create().spawn(&mut Tokio::Global)
    }
}

Then, you would use xtra::spawner::SpawnGlobalTokioExt;

@mneumann
Copy link
Author

@Restioson I'd have to pass-through the &mut Spawner. So this:

let a: Address<MyActor> = MyActor::from_registry().await;

would become:

let a: Address<MyActor> = MyActor::from_registry(&mut AsyncStd).await;

It's not terribly bad, but on the other hand would tie the code towards one specific async runtime.

@mneumann
Copy link
Author

@Restioson Your suggestion would not really help much, as my library would still need to use xtra::spawner::SpawnGlobalTokioExt for tokio, or use ... for another runtime, wouldn't it?

I think the main issue is that multiple runtimes can co-exist via the feature flags. IMHO, this does not make much sense.

I just tried to compile sqlx with multiple async runtimes and this is what happens:

error: only one of ['runtime-actix-native-tls', 'runtime-async-std-native-tls', 
'runtime-tokio-native-tls', 'runtime-actix-rustls', 'runtime-async-std-rustls', 
'runtime-tokio-rustls'] can be enabled
# Cargo.toml
sqlx = { version = "0.4.2", features = ["runtime-tokio-rustls", 
"runtime-async-std-native-tls"] }

@Restioson
Copy link
Owner

It would need to use it yes. That would only be one line though, and save space for all the .spawn()s across the file.

I don't really want to go the sqlx compile error route here, as it still can break with cargo and dependencies as I described

@mneumann
Copy link
Author

That's perfectly fine for me. I can live with that. Would there be a spawn_global available for the other runtimes as well? That'd be nice. Or you could add a spawn_global function to the Spawner trait and then to the ActorManager.

@Restioson
Copy link
Owner

Would there be a spawn_global available for the other runtimes as well?

Yea, that's the idea. Each runtime would get a {Runtime}SpawnGlobalExt trait. Then, if you could use it at the top of the file to do this

use xtra::spawner::RuntimeNameSpawnGlobalExt;

x.spawn_global();
// ...
y.spawn_global();
// ...
z.spawn_global();

and save few characters over each doing x.spawn(&mut RuntimeName::Global)

@mneumann
Copy link
Author

That looks great! I'd then simply add #[cfg(....)] on each of the use statements in my library and it would work.

@Restioson
Copy link
Owner

Hi, could you test out the spawn_global_ext branch?

Also, the recommended way of writing a library with xtra would be to take the spawner as a parameter where possible, and then pass it to spawn. Is this not possible in your use case?

@Restioson
Copy link
Owner

Restioson commented Jun 25, 2021

Please let me know if there is any interest in the functionality contained in the spawn_global_ext branch. If so, I will merge it into mainline

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants