Future::boxed and Stream::boxed should prevent double boxing

[stepancheg]

Fixes #511

travis-ci also runs tests on rust 1.10. In rust 1.10 TypeId::of requires Reflect trait. I have not found an easy way to work around this limitation, so I decided to make boxed prevent double boxing only starting from rust 1.18 (which is latest stable at the moment). Some older version between 1.10 and 1.18 could be chosen for switch, but because travis-ci does not test these versions, I think it is reasonable to enable this behavior only from 1.18.

[sinkuu]

Reflect bound was removed in 1.13 (according to this comment). Linux distros usually provides old versions of Rust, so including them may profit some.

[3Hren]

This seems to be a fully copy/paste version of boxed() for Future except the result type itself. I'd rather decompose them both to a some type-parametrized function.

[stepancheg]

Here I also tried to write generic version and couldn't.

[3Hren]

Can't we just return future here without unsafe magic?
Understood.

[stepancheg]

No, because compiler doesn't know that future can be cast to BoxFuture<...>. Moreover, even transmute doesn't work, because compiler cannot prove that type sizes are equal.

[stepancheg]

OK, updated the PR with slightly more generic version.

[alexcrichton]

Are there measureable gains from this PR? I agree that theoretically this is a plausible optimization but I'd much rather prefer the solution in https://github.com/alexcrichton/futures-rs/pull/513/files

[alexcrichton]

I also fear that most users aren't actually using .boxed() because they don't want Send trait objects. For example all throughout sccache this method is barely used, if at all. In that sense I'm not sure this is worth the weight of the implementation.

[stepancheg]

Are there measureable gains from this PR?

I've updated PR with synthetic test. Before this PR:

test boxed             ... bench:       2,087 ns/iter (+/- 420)
test boxed_boxed       ... bench:       3,106 ns/iter (+/- 577)
test boxed_boxed_boxed ... bench:       5,536 ns/iter (+/- 1,032)
test plain             ... bench:         791 ns/iter (+/- 190)

So, accidental additional unnecessary boxing adds significant overhead for simple streams.

It is hard to say what's performance impact of accidental boxing in real applications. However, the problem is that from user point of view it is easier to simply use safe boxed instead of thinking, whether each boxed call is expensive or could be avoided.

[stepancheg]

I also fear that most users aren't actually using .boxed() because they don't want Send trait objects. For example all throughout sccache this method is barely used, if at all. In that sense I'm not sure this is worth the weight of the implementation.

I also simply call Box::new(..), but if boxed will provide double boxing prevention I will.

About Send, probably, another function should be added like boxed_unsend until Rust type system allows writing generic functions parameterized by optional Send, simething like:

fn boxed(self) -> Box<Future<Item=Self::Item, Error=Self::Eror> + SendIfSend<Self>>
        where Self: Sized + 'static { ... }

[stepancheg]

Updated the PR with specialization instead of transmute.

[alexcrichton]

Yes I think it's obvious that this is a performance gain in microbenchmarks, but my worry is that this doesn't actually get exercised that much higher up the stack. I'm ok with the specialization approach but I would not want to land it until specialization is stable. I'm ok leaving this PR open, however.

[carllerche]

I would rather see boxed deprecated and eventually removed (#228). It's probably been the (or close to) number one source of confusion of people in the gitter channel.

[khuey]

FWIW I usually skip boxed because I don't want 'static.

[alexcrichton]

Currently I agree with @carllerche that we're highly likely to remove/deprecate boxed as in #228, so I'm going to close this. Thanks though for this PR!