Optimize out allocations as much as possible

[SUPERCILEX]

Current paint points:

• All tweenables in sequences and tracks are allocated even though most people will primarily use the built-ins.
• There's double boxing causing extreme indirection: tweenables are stored in a vec which contains boxes of boxes (since IntoBoxDynTweenable unconditionally boxes and there's no way to avoid this without specialization which is a long ways away).
• Creating seqs/tracks of heterogeneous types is very ugly and confusing.

Solutions:

• Use an enum to fake dynamic typing for built-ins.
• Provide an impl for boxed types for 3P tweenables.
• Export a BoxedTweenable type to alleviate typing pain.

This makes built-ins zero cost while 3P types only suffer one layer of indirection.

Closes #28

[djeedai]

I don't understand where the double boxing is supposed to be. IntoBoxDynTweenable exists primarily for the Sequence::new() constructor, which takes an iterator over some (non-boxed) trait objects and box them to be able to store them into the internal Vec. Are you trying to create a Sequence from an existing Vec<Box<dyn Tweenable>>? If so, I think there's indeed redundant work to unbox the tweenables in the iterator then re-box them in the constructor, but I don't think there's double-boxing.

I'm also not clear on what justifies the extra complexity of that new enum. If anything, it seems it adds an extra lookup/branching for each call to the Tweenanble interface, which are plenty.

[SUPERCILEX]

The double boxing happens in something like this where boxing is needed to create homogeneous types:

    let seq2 = Sequence::new([
        Box::new(tween_move) as Box<dyn Tweenable<Transform> + Send + Sync + 'static>,
        Box::new(tracks) as Box<dyn Tweenable<Transform> + Send + Sync + 'static>,
    ]);

That's an array of boxes which get boxed again because of impl<T> Tweenable<T> for Box<dyn Tweenable<T> + Send + Sync + 'static> and impl<T, U: Tweenable<T> + Send + Sync + 'static> IntoBoxDynTweenable<T> for U.

[djeedai]

That's an array of boxes which get boxed again because of impl<T> Tweenable<T> for Box<dyn Tweenable<T> + Send + Sync + 'static> and impl<T, U: Tweenable<T> + Send + Sync + 'static> IntoBoxDynTweenable<T> for U.

Ok, so it seems that adding a constructor for an already-boxed collection solves the issue, no?

fn from_vec(v: Vec<Box<dyn Tweenable<T> + Send + Sync + 'static>>) -> Self;

Or alternatively maybe some From<[Box<dyn Tweenable<T> + Send + Sync + 'static>]> if possible. I didn't try it though.

The Sequence::new() constructor is really to work with unboxed heterogeneous collections, when chaining iterators which can return various types of tweenables.

See #28 for a draft of what that looks like with the from_vec() approach.

Note that double-boxing seems to be a tough issue with many crates and the language itself having issues with it. Just searching for double boxing returns 4 related GitHub items just for futures (rust-lang/futures-rs#228. rust-lang/futures-rs#511, rust-lang/futures-rs#512, rust-lang/futures-rs#513). They're all closed in favor of using the upcoming trait specialization, which was discussed on those issues as being the correct way to fix this (but is not in stable yet).

[SUPERCILEX]

Yeah, I was hoping to avoid adding new methods since I don't think people will know why they are different and ideally we'll use specialization once it comes out. The other bummer with adding a from_vec method is that it prevents us from doing something like using a smallvec inside the sequence + Tracks (which would make sense if we keep a vec of boxes since those are only 16 bytes).

If you're on board with avoiding the double boxing but not the other stuff, I can continue fiddling around with the into iterator method to get just the double boxing avoidance.

[djeedai]

If you're on board with avoiding the double boxing but not the other stuff, I can continue fiddling around with the into iterator method to get just the double boxing avoidance.

Yes, the double boxing should be avoided if possible, while I'm not too warm at the idea of the enum for manual dynamic dispatch, so if possible I'd rather separate those. If you have a better way to prevent the double-boxing I'm all for it; I couldn't find one, and if the folks from rustlang/futures-rs can't either I'm skeptical there's much better than this (but who knows...).

[SUPERCILEX]

This shouldn't double box because into on self just returns self:

// From implies Into
#[stable(feature = "rust1", since = "1.0.0")]
#[rustc_const_unstable(feature = "const_convert", issue = "88674")]
impl<T, U> const Into<U> for T
where
    U: ~const From<T>,
{
    /// Calls `U::from(self)`.
    ///
    /// That is, this conversion is whatever the implementation of
    /// <code>[From]&lt;T&gt; for U</code> chooses to do.
    fn into(self) -> U {
        U::from(self)
    }
}

// From (and thus Into) is reflexive
#[stable(feature = "rust1", since = "1.0.0")]
#[rustc_const_unstable(feature = "const_convert", issue = "88674")]
impl<T> const From<T> for T {
    /// Returns the argument unchanged.
    fn from(t: T) -> T {
        t
    }
}

[djeedai]

Minor comment on .into() instead of direct Box::new() use, and missing CHANGELOG. After that looks good.

[SUPERCILEX]

Done!