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

Simple blocking (non-async) RPC example over unix domain sockets #232

Open
jmaargh opened this issue Sep 12, 2021 · 3 comments
Open

Simple blocking (non-async) RPC example over unix domain sockets #232

jmaargh opened this issue Sep 12, 2021 · 3 comments

Comments

@jmaargh
Copy link

jmaargh commented Sep 12, 2021

I was just thinking about whether it would be possible to use Cap'n Proto RPC in a non-async application (in particular, I was interested in using unix domain sockets as transport). I've thrown together a simple example which I'd love to get your thoughts on: gist.

Observations:

  • I tried to keep the dependencies to a minimum (in library size, rather than number of libraries)
  • I'm a big fan of how all you need to create a two-party VatNetwork is AsyncRead and AsyncWrite objects, made the socket stuff surprisingly easy.
  • My biggest issue was dealing with polling the RpcSystem on the client side. I get the generated Client struct contains some handle back to the RpcSystem so it can pass requests to it/receive responses from it. However, it would have been amazing to be able to get a future from RpcSystem which is "run until you've nothing to do", so I don't have to select with the promise.
  • capnp_rpc is surprisingly heavy in binary-size (4.5% 28.0% 209.5KiB capnp_rpc from cargo-bloat for the server, for example), not problematically huge, just a little surprising.
  • Client-side is also surprisingly heavy, but this time in lib.rs itself: 5.0% 30.8% 233.9KiB cprpc_blocking
  • Generally, more verbose documentation would be amazing for playing around with the library like this (e.g., what does RpcSystem actually do when polled? what does it mean to "bootstrap" it?). Generally, I find the API and naming very confusing (though I get that some of this is inherited from the upstream project). This is especially fraught because so much of the machinery is generated from the schema file.

Specific questions:

  1. Am I doing anything wrong here? Obviously, this is an exceptionally simple example, so I know that this seeming to work doesn't necessarily validate the approach.
  2. I used async_io::block_on as the executor. I'm no expert, but this seemed to be among the simplest executors available for simple blocking usage (and I was already importing async_io for the Async trait anyways).
  3. Obviously, this was written quickly and a lot of best-practices are missing, but how careful do I need to be about tearing down RpcSystems nicely?

Finally (and this may be better for a different issue), I don't understand why there isn't an option to generate a more idiomatic version of the Server trait? Is it just that it would be too much work? I'm thinking something that would look like

impl Counter for Server {
    fn count(&mut self) -> capnp::capability::Promise<u64, capnp::Error> {
        // ... do something ...
        Promise::ok(0)
    }
}

rather than messing around with Params and Results objects. My guess is that the answer is "you'd just be hiding the boilerplate and potentially doing unnecessary work", but surely it's much more common to want access to all params and results?

@dwrensha
Copy link
Member

Thanks for the feedback! I am well aware that the capnp-rpc has a lot of rough edges, and it does help to hear about your specific experiences.

  1. Am I doing anything wrong here? Obviously, this is an exceptionally simple example, so I know that this seeming to work doesn't necessarily validate the approach.
  2. I used async_io::block_on as the executor. I'm no expert, but this seemed to be among the simplest executors available for simple blocking usage (and I was already importing async_io for the Async trait anyways).

I don't see anything obviously wrong with your example code. I agree that it's annoying that you need to select() on the RpcSystem. Usually I end up spawning the RpcSystem future on some separate task so that I don't need to worry about that. Does async_io let you do that? Maybe it would be possible to use futures::executor::LocalPool together with async_io?

  1. Obviously, this was written quickly and a lot of best-practices are missing, but how careful do I need to be about tearing down RpcSystems nicely?

On a client, if all of the calls have completed, there's no harm in dropping the RpcSystem.
On a server, dropping an RpcSystem might interrupt some in-progress calls. Those clients would get a Disconnected error.

Finally (and this may be better for a different issue), I don't understand why there isn't an option to generate a more idiomatic version of the Server trait? Is it just that it would be too much work?

What would your idiomatic version look like in the case where the method's return type a struct?

struct CountContext {
   name @0 : String;
  ...
}
interface Counter {
    count @0 () -> (count :UInt64, context: CountContext);
}

To make this feel like idiomatic Rust, I think we would need to figure out an automatic mapping between capnp types and Rust-native types. That would be very cool to have, but it would take a lot of work.

@jmaargh
Copy link
Author

jmaargh commented Sep 19, 2021

I don't see anything obviously wrong with your example code.

Great, thanks so much for looking over it.

I agree that it's annoying that you need to select() on the RpcSystem. Usually I end up spawning the RpcSystem future on some separate task so that I don't need to worry about that. Does async_io let you do that? Maybe it would be possible to use futures::executor::LocalPool together with async_io?

It would definitely be possible to spawn a task, but I was trying to stay as far away from async as possible, ideally so that when any of the library functions return nothing else is left running, hence looking for the simplest possible blocking executor. Perhaps this isn't a sensible goal, but it was a goal I had in mind.

On a client, if all of the calls have completed, there's no harm in dropping the RpcSystem.
On a server, dropping an RpcSystem might interrupt some in-progress calls. Those clients would get a Disconnected error.

Thanks, good to know. It would be great if this were added to doc comments

What would your idiomatic version look like in the case where the method's return type a struct?

struct CountContext {
   name @0 : String;
  ...
}
interface Counter {
    count @0 () -> (count :UInt64, context: CountContext);
}

To make this feel like idiomatic Rust, I think we would need to figure out an automatic mapping between capnp types and Rust-native types. That would be very cool to have, but it would take a lot of work.

Yeah, I guess this is where the work would balloon somewhat. My first thought would be to generate rust struct definitions for each capn proto struct. Since all built-in types seem to have direct Rust equivalents, this should always be possible, the members are always going to be rust primitives, String, Vec, or another generated struct.

@dwrensha
Copy link
Member

My first thought would be to generate rust struct definitions for each capn proto struct. Since all built-in types seem to have direct Rust equivalents, this should always be possible, the members are always going to be rust primitives, String, Vec, or another generated struct.

See #157 for some work in this direction.

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