async message reader with explicit continuations

[danburkert]

This is a sketch of a possible solution to #38. The strategy taken here is to return a enum type of Complete or Continue whenever reading or writing a message could block. Complete signals that the operation completed successfully, while Continue signals that the operation must be tried again, and carries along with it any necessary continuation state.

The initial commit only includes a read_message implementation with continuations. The next step will be to implement a read_message_continue which takes the continuation and Read object and continues reading the message. I believe write_message can be implemented in much the same way, and perhaps with less impact to the code. Finally, I think packed serialization could be extended in much the same way.

The motivation to use continuations instead of a higher level abstraction like Futures or Promises is that Rust doesn't have a standard implementation of these abstractions, so adapting the read/write machinery to one specific implementation would not benefit the others. My goal is that this continuation API is a stepping stone to implementing async message reading and writing in any higher level async abstraction.

I'd love to get feedback on this strategy. I will continue filling it out in the meantime.

[danburkert]

I have finished implementing serialization/deserialization with continuations, and found that it fits in very naturally with the existing logic. The latest commit (50e909e) replaces the existing write_message, read_message implementation with the continuation-based implementations, and simply ignore the continuations. The performance seems to be comparable, but perhaps slightly more variance. I'm very happy with the results here since it allows only a single implementation, but retains the benefit of allowing async usage. I'm still looking into how continuations can be applied to packing/unpacking.

[dwrensha]

Although I haven't had the time to read through the details of this yet, I'm definitely warming up to the approach you're taking.

I'm specifically trying to think through how it will integrate with the port of KJ promises that I've started working on.

Building on your ideas here, I think it'll be possible to do stuff like this:

enum StepResult<T, S> {
   TheresMore(S),
   Done(T)
}

/// Intermediate state for a reading a T.
trait AsyncReadState<T> {

   /// Reads as much as possible without blocking. If done, returns the final T value. Otherwise
   /// returns the new intermediate state T.
   fn read_step<R: Read>(self, r: &mut R) -> ::std::io::Result<StepResult<T,Self>>;
}

/// Gives back `r` once the T has been completely read.
fn read_async<R, S, T>(r: R, state: S) -> gj::Promise<(R, T)>
  where R: Read + 'static,
        S: AsyncReadState<T> + 'static,
        T: 'static {
    ....
}

The read_async() function would just be implemented as part of GJ. In the case of Cap'n Proto serialization, we would implement AsyncReadState<OwnedSpaceMessageReader> for your ReadContinuation.

[dwrensha]

I noticed that mio is implemented in terms of TryRead and TryWrite, which return a Result<Option<T>>, with Ok(None) corresponding to the Err(WouldBlock) case. I guess that we would need to write adapters that remap the Ok(None) back to Err(WouldBlock) if we want mio to work with the functionality you've added here.
Note that bytes::RingBuf does quite not do what we would want, as it returns Ok(0) when there is no data available.

[danburkert]

@dwrensha the API exposed by the PR is pretty much exactly what you described, just with some slightly different naming. I haven't had time to make any progress on the non-blocking serialized reader/writer, but I think the PR is providing a lot of value as it is right now.

I hadn't seen gj before, what will it do different than mio?

The mio TryRead/TryWrite traits are a mistake, and I hope they will be removed soon; the builtin Read/Write traits are totally sufficient, given the WouldBlock error kind.

[danburkert]

Ah so looking at it more, it looks like gj is providing a higher level abstraction over mio. I guess it would be more appropriate to compare it to eventual and eventual_io then.

[danburkert]

rebased

[dwrensha]

I'm still trying to digest this. I'm wary of extra complexity this brings to the code, although it does look like you've done a good job adding tests to keep things maintainable. Have you thought at all about the serialize_packed case? I imagine that doing something like this would add even more complexity there.

How much more inconvenient would it be to do do what you're trying to accomplish in a separate crate?

[danburkert]

I haven't done much work on the serialize_packed case, except to see that it's non-trivial. Doing this in a separate crate would be inconvenient, because OwnedSpaceMessageReader does not have a public constructor.

[dwrensha]

Doing this in a separate crate would be inconvenient, because OwnedSpaceMessageReader does not have a public constructor.

What it we figured out a way to make it convenient for third-party crates to implement capnp::message::MessageReader? It seems like that could help here and would be generally useful.

[danburkert]

What it we figured out a way to make it convenient for third-party crates to implement capnp::message::MessageReader?

Sorry for not getting back to you sooner, I've been thinking this over the past few days. My first thought was that effort spent abstracting at this layer would be wasted, since so few people would implement a reader. After considering it some more, though, I think it may be more common than I first thought. For instance, #25 is basically getting towards this.

I never really considered making a custom implementation of MessageReader, because the interface is really hairy and there aren't any comments explaining it (or any of the interfaces it relies on). By comparison, following the existing pattern of creating an OwnedSpaceMessageReader is somewhat simpler. I think some cleanup and documentation of the MessageReader interface could go a long way.

[dwrensha]

I've been thinking about how we might move forward with the user-defined-message-readers plan. I think what we really want is for users to be able to specify the behavior for the ReaderArena::try_get_segment(), BuilderArena::get_segment() and BuilderArena::allocate_owned_memory() methods. This suggests that perhaps MessageReader and MessageBuilder should just be plain structs, where users get to pass in a trait-object arena. Lots of details still to work out, though...

[dwrensha]

I've pushed some changes (b1c54c9, included in the 0.3.0 release) that should enable you to customize the behavior of message readers without needing access to the internals of capnproto-rust.

message::Reader is now a plain struct that takes a ReaderSegments. I think you can accomplish your goals by writing your own implementation of ReaderSegments.

[dwrensha]

@dcb FYI, I'm considering pulling these changes in after all, probably with some minor renamings and reorganization. I've rebased the pull request here.

[dwrensha]

Eek. I of course meant to tag @danburkert above.

[danburkert]

Interesting, is this to support gj and futures-rs? By the way, this work culminated in capnp-nonblock, which does the continuation based serialize/deserialize, and wraps it up to make it a little easier to use with mio. You might want to look there, I think I tweaked some of the serialization/deserialization methods. Of course feel free to grab any of that code (I think the license matches).

[dwrensha]

Yeah, I'm trying to imagine a world where capnp-rpc-rust is based on futures-rs instead of gj, and it looks like this pull request could help.

[dwrensha]

In #66 I've rebased this and made some changes to make it more closely match the style of futures-rs.