Tokio: Use futures AsyncRead, AsyncWrite, ready! macro everywhere

[95th]

Now that futures crate has AsyncRead and AsyncWrite traits and ready! Macro, I think we should use those and re-export them in tokio.

Is there a reason we have not done this so far?

[carllerche]

ready! was previously not available in futures-core. It is now, so we can switch to that.

As for AsyncRead / AsyncWrite, that was mentioned here.

I will keep this issue open for a bit in case there is follow up discussion.

[95th]

Thanks.. I'll submit a PR for ready!. About AsyncRead/AsyncWrite, I had the same concern as mentioned in that conversation. But it is just a concern, I have not run into any such compatibility issues yet.

[Kvikal]

I have run into an issue regarding this. I wanted to use BufReader with tokio_tcp::split::TcpStreamReadHalf, but Tokio itself does not provide one (unless I missed it).
Futures has futures::io::BufReader, but it is built using different AsyncRead trait then the one used by tokio_tcp::split::TcpStreamReadHalf.

It would certainly be nice, if tokio/futures shared the same AsyncRead/AsyncWrite traits.

[LucioFranco]

@Kvikal there is this https://github.com/tokio-rs/tokio/blob/master/tokio-io/src/async_buf_read.rs

[Nemo157]

If you do need compatibility between libraries using tokio's traits and libraries using futures' traits for testing stuff now, I threw together a shim: https://github.com/Nemo157/futures-tokio-compat. As the readme mentions this is purely there to allow experimental testing (as I wanted to try using new hyper integrated with a library that was already based around futures' IO/task spawning traits), I have no plans on publishing it.

[LucioFranco]

@Nemo157 this seems really nice! We may want to bring this into actual tokio-io under a futures-compat feature?

[Nemo157]

My hope is that there is only one set of traits in the end, I don't see any reason to have multiple of them (at least, when they're this similar, if they were more differentiated then they might be useful in different usecases). The only result of that I see is fragmentation in the ecosystem requiring compat shims like this everywhere.

Also note that the compat layer there is not zero-cost, and as far as I know cannot be; for spawning it requires extra boxing in one direction, and the IO traits don't proxy the buffer initialization so they will fall back to zeroing on both sides (it is possible to proxy it one direction at least, but I didn't feel like guaranteeing that unsafe code).

But, either way I'll stick an Apache/MIT license on that repo, so if you do wish to pull it into tokio feel free to do so.

[LucioFranco]

@Nemo157 sorry, I didn't mean that we should have it in there as the goal around keeping it there forever. Mostly around the thought, while we play around with the traits users can easily continue to work with other libraries. I do hope, in the end, there is only one set of traits 😄.

[o01eg]

What if at least move provided AsyncRead::poll_read_buf and AsyncWrite::poll_write_buf to *Ext traits?

[carllerche]

@o01eg these functions are intended to be implemented as needed. This prevents moving them to ext traits.

[carllerche]

Discussion is primarily in https://github.com/tokio-rs/tokio/pull/1744 now.