search

containerd / ttrpc-rust

Rust implementation of ttrpc (GRPC for low-memory environments)
Apache License 2.0
195 stars 45 forks source link

Allow concurrent usage of the asynchronous Client (part 2/2) #118

Closed Kazurin-775 closed 2 years ago

Kazurin-775 commented 2 years ago

Currently, the request() method of ttrpc::r#async::Client takes an &mut self. This prevents the Client from being used concurrently, since a mutable reference in Rust indicates (and enforces) exclusive use of an object.

In this patch, we change the signature of request() to take &self instead of &mut self. This allows multiple coroutines to issue concurrent RPC requests via the same Client object (e.g. by using an Arc), without breaking the API compatibility (although most of existing code will generate a warning indicating that a let mut is unnecessary).

We see (in the example code) that this problem is currently worked around by cloning the Client. However, we still hope to propose this patch, since by avoiding cloning the Client, we could improve the performance in scenarios where the Client is already wrapped in an Arc.

Meanwhile, the synchronized flavor of this library already takes &self in the request() method: https://github.com/containerd/ttrpc-rust/blob/dfae1ad06ec29d93ed76dfcf3e931168cd1d426a/src/sync/client.rs#L216

Therefore, by applying this patch, we maintain the consistency between the synchronized and asynchronized APIs.

(UPDATE: changes in the compiler part has been moved to a sub-PR #123.)

Tim-Zhang commented 2 years ago

@Kazurin-775 The pr https://github.com/containerd/ttrpc-rust/pull/123 has been merged and the new version of ttrpc-compiler and ttrpc-codegen has been published

you can rebase this pr now.

Kazurin-775 commented 2 years ago

@Tim-Zhang Done rebasing. (Sorry for being late)

Tim-Zhang commented 2 years ago

@lifupan PTAL