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

Forward thinking discussion on static async support #386

Closed
stevefan1999-personal opened this issue Dec 19, 2022 · 2 comments
Closed

Forward thinking discussion on static async support #386

stevefan1999-personal opened this issue Dec 19, 2022 · 2 comments

Comments

@stevefan1999-personal
Copy link

stevefan1999-personal commented Dec 19, 2022

Since Rust stablized GAT support back in early November, the follow up is of course static async fn.

However, the current design model of tarpc (or let's say the plugin/codegen) is based around storing RPC state within a Future itself:

tarpc/plugins/src/lib.rs

Lines 611 to 628 in 7e872ce

fn enum_response_future(&self) -> TokenStream2 {
let &Self {
vis,
service_ident,
response_fut_ident,
camel_case_idents,
future_types,
..
} = self;
quote! {
/// A future resolving to a server response.
#[allow(missing_docs)]
#vis enum #response_fut_ident<S: #service_ident> {
#( #camel_case_idents(<S as #service_ident>::#future_types) ),*
}
}
}

And then selecting it in the generated serve function:

tarpc/plugins/src/lib.rs

Lines 535 to 566 in 7e872ce

quote! {
impl<S> tarpc::server::Serve<#request_ident> for #server_ident<S>
where S: #service_ident
{
type Resp = #response_ident;
type Fut = #response_fut_ident<S>;
fn method(&self, req: &#request_ident) -> Option<&'static str> {
Some(match req {
#(
#request_ident::#camel_case_idents{..} => {
#request_names
}
)*
})
}
fn serve(self, ctx: tarpc::context::Context, req: #request_ident) -> Self::Fut {
match req {
#(
#request_ident::#camel_case_idents{ #( #arg_pats ),* } => {
#response_fut_ident::#camel_case_idents(
#service_ident::#method_idents(
self.service, ctx, #( #arg_pats ),*
)
)
}
)*
}
}
}
}

Although this increases flexibility so users can opt to use their own implementation of Future as shown in the readme example:

impl World for HelloServer {
// Each defined rpc generates two items in the trait, a fn that serves the RPC, and
// an associated type representing the future output by the fn.
type HelloFut = Ready<String>;
fn hello(self, _: context::Context, name: String) -> Self::HelloFut {
future::ready(format!("Hello, {name}!"))
}
}

This actually implied Sized requirement since we have stored the response future as an enum unit tuple, which by itself requires Sized.

This means cannot just change the future type to impl Future (as you do with GAT futures), since this exactly is ?Sized (unsized) which is contradictory to the current design of tarpc.

We have to think forward for this. One possibility is to use microsoft/stackfuture. This gives us a Sized dyn future while we don't have to use Pin<Box<dyn Future<Output = T> + Send>> and thus implied the use of heap. I've created a helper for this, and it worked out alright:

    #[tarpc::service]
    trait Hello {
        async fn hello() -> String;
    }


    impl Hello for () {
        #[asynchelp::tarpc::stackfuture(size = 1024)]
        async fn hello(self, _: context::Context) -> String {
            "hello".to_string()
        }
    }

The problem is this requires user interaction to specify the stack size, and if the heap size is too large (like if the system set a size to not go beyond a certain stack ulimit), they have to fallback using heap as well.

Otherwise, the best workaround for now is to either keep using boxed heap dyn future, or wait for Rust team to implement static async fn in dyn which I believe will take a year to finalize because of its unprecedented difficulty.

I took the stackfuture inspiration directly from Dyn async traits, part 9: call-site selection of the Baby Steps blog series, focused deeply on the topics of async fn ecosystem. Not sure if this would also throw us some more hint in the future but just saying.

@tikue
Copy link
Collaborator

tikue commented Dec 19, 2022

Hey, thanks for starting this discussion! I have an experimental branch that uses async fns in traits, please take a look and share any feedback: master...tikue:tarpc:master

@stevefan1999-personal
Copy link
Author

After a year I think this can be closed. Thanks for all the effort @tikue!

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

No branches or pull requests

2 participants