Reducing copies of nestByValue()
@ggael
Submitted by Gael GuennebaudAssigned to Nobody
Link to original bugzilla bug (#1663)
Description
After resurrecting nestByValue(), I decided to see whether the numerous copies could be removed by move ctors. Mostly for curiosity as I'm not sure nestByValue() is a really useful feature as it was broken since 3.3.0 and nobody complained!
OK, so let's consider this toy example:
auto get_xpr_with_temps(const MatrixXd& a)
{
return a.rowwise().reverse().eval().nestByValue()
+ (a+a).eval().nestByValue();
}
Currently this creates 6 temporaries, and 4 copies. Ouch!
Those can be reduced to 2 temporaries only (the ideal) if we do the following changes:
-
Make all functions return non const expressions, including eval(). So far, they return const object to prevent stupid code like:
expr.eval().normalized(); -
Add ctors from rvalue refs to all expressions, e.g.:
CwiseBinaryOp(Lhs&& aLhs, Rhs&& aRhs, const BinaryOp& func = BinaryOp())
: m_lhs(std::move(aLhs)), m_rhs(std::move(aRhs)), m_functor(func) {}
(plus the 2 hybrid versions for binary expressions).
- Add overloads members/operators for rvalue ref, e.g., for EIGEN_MAKE_CWISE_BINARY_OP(), we have to add:
template<typename OtherDerived> \
EIGEN_DEVICE_FUNC EIGEN_STRONG_INLINE EIGEN_CWISE_BINARY_RETURN_TYPE(Derived,OtherDerived,OPNAME) \
(METHOD)(EIGEN_CURRENT_STORAGE_BASE_CLASS<OtherDerived> &&other) && \
{ \
return EIGEN_CWISE_BINARY_RETURN_TYPE(Derived,OtherDerived,OPNAME)(std::move(derived()), std::move(other.derived())); \
} \
/* + the 2 hybrid versions */
The regular const operator/member must be qualified by "const &" instead of "const" only.
That's all.
Whereas points 1) and 2) are easily manageable, point 3) represents a lot of boilerplate code. E.g., think about all the block variants! Moreover, this only addresses heap-allocated temporaries, not fixed size ones.