feat: a more lightweight approach #149
Conversation
rklaehn
changed the title
…nding on rustc version E.g. EXPECTED: ┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈ error: RpcRequests can only be applied to enums --> tests/compile_fail/non_enum.rs:4:1 | 4 | struct Foo; | ^^^^^^^^^^^ ┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈ ACTUAL OUTPUT: ┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈ error: RpcRequests can only be applied to enums --> tests/compile_fail/non_enum.rs:4:1 | 4 | struct Foo; | ^^^^^^ ┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈
| #[cfg(feature = "rpc")] | ||
| #[cfg_attr(quicrpc_docsrs, doc(cfg(feature = "rpc")))] | ||
| #[error("error opening stream: {0}")] | ||
| Other(#[from] anyhow::Error), |
There was a problem hiding this comment.
maybe using an io::Error would match better with the rest of the system?
There was a problem hiding this comment.
The problem is that iroh currently is using anyhow::Error for connecting.
Connect returns an anyhow::Result, here: https://docs.rs/iroh/latest/iroh/endpoint/struct.Endpoint.html#method.connect
And Endpoint::open_bi also returns an anyhow::Result:
https://docs.rs/iroh/latest/iroh/endpoint/struct.OpenBi.html
I can of course use io::Error::other(theanyhowrerror). Not sure what our plan is there for iroh 1.0. What we don't want here is to put in a concrete iroh error, because then we have the semicircular dependency again.
|
NOOOO minimal crates is broken again!!!! What is it this time? |
It's such a distraction, just not worth it IMO. It gives an error in a crate that isn't even in my cargo tree, possibly related to some unstable feature that came and went.
we want the raw (iroh-)quinn streams anyway so we can tune them
|
Hm, this thing only works with iroh-quinn send and recv streams, and only with iroh or iroh-quinn connections. And I don't intend to change that. So maybe we should publish this under |
|
maybe |
.github/workflows/ci.yml
Outdated
| run: | | ||
| rm -f Cargo.lock | ||
| cargo +nightly check -Z minimal-versions --workspace --all-features --lib --bins | ||
| # minimal-crates: |
There was a problem hiding this comment.
I know this sucks, but lets bring this back before merging please
There was a problem hiding this comment.
I just plain don't know how to fix it. I am getting an error in ahash, which is not even a dependency
cargo +nightly tree --all-features | grep ahash
<crickets>
There was a problem hiding this comment.
Pushed an update that changes CI to this:
cargo +nightly check -Z minimal-versions -p quic-rpc -p quic-rpc-derive --all-features --lib --bins
Error is gone, but I am not super confident that it is doing anything at all anymore...
They are not yet boxed, just boxable and dynable...
|
Published as https://github.com/n0-computer/irpc |
An attempt to make quic-rpc more lightweight, in support of the blobs 1.0 stabilisation.
API
The old approach tried to provide a high level api for rpc calls. But in our experience with iroh-blobs, gossip, docs etc. in every single case the api was wrapped with some other api anyway. So it seems a lot of the complexity with patterns, to provide said high level api, was not as useful as initially thought.
Efficiency
In memory case
In the old quic-rpc, you would send a pair of mpsc channels, a sender and a receiver, from the client to the server using a mpmc channel. This means that every rpc request, even when the response channel is just an oneshot sender, would have the overhead of 2 mpsc channels. You could reduce this overhead by some sort of complex caching scheme, but the problem remains that you do more that absolutely necessary in many cases.
In the new approach, you manually create an in-memory channel of the appropriate type (oneshot, spsc or none). You can either use the fns in this crate, or even just start with a tokio channel and use the Into conversions into the crate channels.
The result is that the overhead compared to the completely manual case is basically zero, just an enum discriminator. No additional allocations, nothing.
Rpc case
In the old rpc case, each response had to be converted to a large response enum. This does not exist anymore. You still have to serialize the response using postcard and send it, so there is no huge efficiency gain. But at least you get rid of the annoying procedure of defining a response enum.
Boilerplate
Yes, there is more. Every time you do a request, you need to do different things depending on if you are local or remote. For local, you create channels, send one side with the request, and return the other side as usual.
For remote, you write the request to the remote side, get back a pair of send/recv streams, and then return them as a channel of the appropriate kind (or not, in case of channel type none).
This is not that great, but on the plus side it allows you to adjust some details in a straightforward way. E.g. you can set the capacity of local channels and adjust the settings of the send/recv stream for the remote case.