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

Codegen with Arc<dyn ..> instead of Arc<Box<..>> #234

Closed
Xynnn007 opened this issue Aug 19, 2024 · 0 comments · Fixed by #235
Closed

Codegen with Arc<dyn ..> instead of Arc<Box<..>> #234

Xynnn007 opened this issue Aug 19, 2024 · 0 comments · Fixed by #235

Comments

@Xynnn007
Copy link
Contributor

Hi, I am using ttrpc for some time. One thing confused me is that with ttrpc's Codegen, we would generate a .._ttrpc.rs where a XXXMethod would have a service member with type Arc<Box<dyn ...>> rather than Arc<dyn ...>.

I wonder if we could convert Arc<Box<dyn ...>> to Arc<dyn ...>?

The reason comes from some ttrpc server implementation.

We are using a proto with more-than-one Service, e.g.

service A {
    rpc funcA(ARequest) returns (AResponse) {};
}

service B {
    rpc funcB(BResquest) returns (BResponse) {};
}

We want that a same object would serve the two APIs, thus we have

// Server impls A and B
let server = Server::new();
let server = Arc::new(Box::new(server as _));

let a = create_a_service(server.clone());

// compiler errors because we cannot convert Arc<Box<T>> to both Arc<Box<dyn A>> and Arc<Box<dyn B>>
let b = create_a_service(server);

So what we can do is only to create two Servers to create different services, which means double the memory consumption.

If the ttrpc CodeGen returns type Arc<dyn T>, we can facilitate by

let server = Arc::new(Server::new());

let a = create_a_service(server.clone() as _);
let b = create_b_service(server as _);

let mut server = TtrpcServer::new()
        .bind(&socket)
        .unwrap()
        .register_service(a)
        .register_service(b);

where the as _ would help the compiler convert Arc<Server> to Arc<dyn A> and Arc<dyn B>.

Xynnn007 added a commit to Xynnn007/ttrpc-rust that referenced this issue Aug 19, 2024
This commit changes the generated ttrpc server from Arc<Box<T>>
to Arc<T>. This helps the type conversion and also avoids extra runtime
cost caused by double pointer.

Fixes containerd#234

Signed-off-by: Xynnn007 <xynnn@linux.alibaba.com>
Xynnn007 added a commit to Xynnn007/ttrpc-rust that referenced this issue Aug 19, 2024
This commit changes the generated ttrpc server from Arc<Box<T>>
to Arc<T>. This helps the type conversion and also avoids extra runtime
cost caused by double pointer.

Fixes containerd#234

Signed-off-by: Xynnn007 <xynnn@linux.alibaba.com>
Xynnn007 added a commit to Xynnn007/ttrpc-rust that referenced this issue Aug 19, 2024
This commit changes the generated ttrpc server from Arc<Box<T>>
to Arc<T>. This helps the type conversion and also avoids extra runtime
cost caused by double pointer.

Fixes containerd#234

Signed-off-by: Xynnn007 <xynnn@linux.alibaba.com>
Xynnn007 added a commit to Xynnn007/ttrpc-rust that referenced this issue Aug 19, 2024
This commit changes the generated ttrpc server from Arc<Box<T>>
to Arc<T>. This helps the type conversion and also avoids extra runtime
cost caused by double pointer.

Fixes containerd#234

Signed-off-by: Xynnn007 <xynnn@linux.alibaba.com>
Xynnn007 added a commit to Xynnn007/ttrpc-rust that referenced this issue Aug 20, 2024
This commit changes the generated ttrpc server from Arc<Box<T>>
to Arc<T>. This helps the type conversion and also avoids extra runtime
cost caused by double pointer.

Fixes containerd#234

Signed-off-by: Xynnn007 <xynnn@linux.alibaba.com>
jokemanfire pushed a commit to jokemanfire/ttrpc-rust that referenced this issue Sep 27, 2024
This commit changes the generated ttrpc server from Arc<Box<T>>
to Arc<T>. This helps the type conversion and also avoids extra runtime
cost caused by double pointer.

Fixes containerd#234

Signed-off-by: Xynnn007 <xynnn@linux.alibaba.com>
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 a pull request may close this issue.

1 participant