Hyper: Update to futures 0.2

[seanmonstar]

It's not released yet, but this is just tracking that it needs to happen and is part of 0.12.

[cramertj]

I'm super excited for this and would be interested in helping out! futures-0.2 is nearly code-complete, so we could probably get started soon. Do you have a branch you'd like PRs submitted to?

[seanmonstar]

There's a 0.12.x branch now on the repo where I've been trying to add new breaking changes to.

[srijs]

Now that tokio 0.1 has landed in the 0.12.x branch, we should be able to attempt this.

Is anyone working on this yet? Otherwise I'd have some time tomorrow to take a stab!

[srijs]

Good news: So far I've got everything but the tests to compile. I'm using the unstable-futures feature that recently landed in tokio, together with two PRs I've opened to use futures 0.2.0-alpha in dependency crates:

• want: https://github.com/seanmonstar/want/pull/1
• futures-timer: https://github.com/alexcrichton/futures-timer/pull/5

I'll try to find some time tonight to fix up the tests, and then this should be ready for review (fingers crossed).

[srijs]

Tests compile now, however running into issues with iovec. Will investigate and (hopefully) fix tomorrow!

[seanmonstar]

@srijs just as a heads up, I've merged 0.12.x into master, will be deleting the 0.12.x branch so there's no confusion of the difference between master and 0.12.x.

[aturon]

@srijs awesome work!

[srijs]

Okay, I think I figured out the issue with iovec, and could use some help finding the right way to go.

Previously (iirc) WriteBufAuto was used to detect whether the underlying AsyncWrite object supported/used vectored i/o, and was adjusting the buffering strategy accordingly. This worked because the method on AsyncWrite was write_buf, which actually accepted a Buf object. So the detection is more or less straight-forward: If the underlying object calls Buf::bytes, it likely does not use vectored io, but it's likely that it does when it called Buf::bytes_vec instead.

With the new interface in futures, the equivalent to write_buf is now poll_vectored_write which accepts a mutable slice of IoVec. The issue with that is that the control is inverted: Instead of the write object making the choice of whether to use vectored i/o, the caller decides whether to use it, with the write object using the default impl of poll_vectored_write instead if it doesn't support vectored io (the default impl will fallback to using poll_write on the first iovec that was passed to it). This makes detection a lot harder in my understanding.

From what I can see, we have multiple options now:

1. Drop the detection logic, always attempt to use vectored I/O (the AsyncWrite impl will fall back if not supported)
2. Drop the detection logic, never use vectored I/O (sounds like a bad idea?)
3. Petition for the AsyncWrite trait in futures 0.2 to use Buf instead of requiring a slice of IoVec (or possibly add a second method poll_write_buf?)
4. Another magic way how we could detect this?

Conceivably we could also combine the approaches, e.g. go with Option 1 first and pursue Option 3 longer term.

[srijs]

I've got a sketch for a possible (imo slightly hacky) detection algorithm based on poll_vectored_write.

For context, my logic to write a Buf into an underlying AsyncWrite object looks something like this:

if !buf.has_remaining() {
    return Ok(Async::Ready(0));
}
let n = {
    static PLACEHOLDER: &[u8] = &[0];
    let mut bufs = [From::from(PLACEHOLDER); 64];
    let i = Buf::bytes_vec(&buf, &mut bufs[..]);
    try_ready!(io.poll_vectored_write(cx, &bufs[..i]))
};
buf.advance(n);
return Ok(Async::Ready(n));

We could exploit our knowledge of what the default impl for poll_vectored_write does in the following way:

If i > 1 (more than one io vec available) and n == bufs[0].len(), then we know with some certainty that the default impl was used, since only the first of many buffers was written. Switch our strategy to Strategy::Flatten. A false positive could be produced in a situation where the number of bytes that the underlying I/O can accept coincides with the length of the first buffer.

If i > 1 (more than one io vec available) and n > bufs[0].len(), then we know that a non-default impl was used, since more than just the first buffer was written, meaning there is a good chance that vectored I/O is supported. Switch our strategy to Strategy::Queue.

If neither of these conditions apply, we can't be certain either way, and should keep using the current strategy (which is Strategy::Auto by default).

The big risk here is that we depend on the behaviour of default impl for poll_vectored_write in futures, which I assume might change at any point.

[srijs]

@seanmonstar thanks for the heads-up, I'm rebasing now...

@aturon thanks, let's make futures 0.2 a success! :)

[srijs]

The PR can now be found here: https://github.com/hyperium/hyper/pull/1470