Core: Avoid copies when conservatively resizing vectors through two-argument overloads libeigen/eigen!3402 Co-authored-by: Rasmus Munk Larsen <4643818-rmlarsen1@users.noreply.gitlab.com>
diff --git a/Eigen/src/Core/PlainObjectBase.h b/Eigen/src/Core/PlainObjectBase.h index 523eb4d..1896bea 100644 --- a/Eigen/src/Core/PlainObjectBase.h +++ b/Eigen/src/Core/PlainObjectBase.h
@@ -1005,17 +1005,25 @@ } }; -// Here, the specialization for vectors inherits from the general matrix case -// to allow calling .conservativeResize(rows,cols) on vectors. +// Vectors preserve their linear layout when their length changes. template <typename Derived, typename OtherDerived> struct conservative_resize_like_impl<Derived, OtherDerived, true> : conservative_resize_like_impl<Derived, OtherDerived, false> { using Base = conservative_resize_like_impl<Derived, OtherDerived, false>; using Base::IsRelocatable; using Base::MustInitializeAppended; - using Base::run; + + static void run(DenseBase<Derived>& _this, Index rows, Index cols) { + EIGEN_STATIC_ASSERT_DYNAMIC_SIZE(Derived) + const Index size = Derived::RowsAtCompileTime == 1 ? cols : rows; + eigen_assert((Derived::RowsAtCompileTime == 1 ? rows == 1 : cols == 1) && + "Invalid sizes when resizing a vector."); + run(_this, size); + } static void run(DenseBase<Derived>& _this, Index size) { + eigen_assert(size >= 0 && "Invalid sizes when resizing a vector."); + if (_this.size() == size) return; const Index new_rows = Derived::RowsAtCompileTime == 1 ? 1 : size; const Index new_cols = Derived::RowsAtCompileTime == 1 ? size : 1; EIGEN_IF_CONSTEXPR (IsRelocatable) {
diff --git a/test/conservative_resize.cpp b/test/conservative_resize.cpp index bd63ba6..28c5f8a 100644 --- a/test/conservative_resize.cpp +++ b/test/conservative_resize.cpp
@@ -8,6 +8,7 @@ // with this file, You can obtain one at http://mozilla.org/MPL/2.0/. // SPDX-License-Identifier: MPL-2.0 +#define EIGEN_RUNTIME_NO_MALLOC #include "main.h" #include <Eigen/Core> @@ -264,7 +265,124 @@ VERIFY(m.data() == data); } +struct InitializedResizeScalar { + int value; + InitializedResizeScalar() : value(0) {} +}; + +template <typename Vector> +void same_shape_vector_resize() { + using Scalar = typename Vector::Scalar; + STATIC_CHECK(std::is_trivially_copyable<Scalar>::value); + STATIC_CHECK(NumTraits<Scalar>::RequireInitialization); + Vector v(8); + for (Index i = 0; i < v.size(); ++i) v(i).value = int(i + 1); + const auto* data = v.data(); + const bool malloc_allowed = internal::is_malloc_allowed(); + internal::set_is_malloc_allowed(false); + // Every overload must leave unchanged storage alone, including scalars that require initialization. + v.conservativeResize(v.size()); + v.conservativeResize(v.rows(), v.cols()); + v.conservativeResize(NoChange, v.cols()); + v.conservativeResize(v.rows(), NoChange); + internal::set_is_malloc_allowed(malloc_allowed); + VERIFY(v.data() == data); + for (Index i = 0; i < v.size(); ++i) VERIFY_IS_EQUAL(v(i).value, i + 1); +} + +template <typename Vector> +void two_dimensional_vector_resize() { + STATIC_CHECK(!std::is_trivially_copyable<typename Vector::Scalar>::value); + Vector v; + const auto* data = v.data(); + ResizeScalar::reset(); + for (Index i = 0; i < 16; ++i) { + if (i % 3 == 0) { + v.conservativeResize(i + 1); + } else if (i % 3 == 1) { + v.conservativeResize(Vector::RowsAtCompileTime == 1 ? 1 : i + 1, Vector::RowsAtCompileTime == 1 ? i + 1 : 1); + } else { + if (Vector::RowsAtCompileTime == 1) + v.conservativeResize(NoChange, i + 1); + else + v.conservativeResize(i + 1, NoChange); + } + v(i).value = int(i + 1); + } + VERIFY_IS_EQUAL(ResizeScalar::copies, 0); + VERIFY_IS_EQUAL(ResizeScalar::moves, 0); + VERIFY(v.data() == data); + for (Index i = 0; i < v.size(); ++i) VERIFY_IS_EQUAL(v(i).value, i + 1); + v.conservativeResize(Vector::RowsAtCompileTime == 1 ? 1 : 8, Vector::RowsAtCompileTime == 1 ? 8 : 1); + VERIFY_IS_EQUAL(ResizeScalar::copies, 0); + VERIFY_IS_EQUAL(ResizeScalar::moves, 0); + Vector other(12); + for (Index i = 0; i < other.size(); ++i) other(i).value = int(100 + i); + v.conservativeResizeLike(other); + VERIFY_IS_EQUAL(ResizeScalar::copies, 4); + VERIFY_IS_EQUAL(ResizeScalar::moves, 0); + for (Index i = 0; i < v.size(); ++i) VERIFY_IS_EQUAL(v(i).value, i < 8 ? i + 1 : 100 + i); + other.resize(4); + ResizeScalar::reset(); + v.conservativeResizeLike(other); + VERIFY_IS_EQUAL(ResizeScalar::copies, 0); + VERIFY_IS_EQUAL(ResizeScalar::moves, 0); + for (Index i = 0; i < v.size(); ++i) VERIFY_IS_EQUAL(v(i).value, i + 1); + VERIFY_RAISES_ASSERT(v.conservativeResize(2, 2)); + VERIFY_RAISES_ASSERT( + v.conservativeResize(Vector::RowsAtCompileTime == 1 ? 1 : -1, Vector::RowsAtCompileTime == 1 ? -1 : 1)); + VERIFY_RAISES_ASSERT( + v.conservativeResize(Vector::RowsAtCompileTime == 1 ? 1 : 17, Vector::RowsAtCompileTime == 1 ? 17 : 1)); + v.conservativeResize(Vector::RowsAtCompileTime == 1 ? 1 : 0, Vector::RowsAtCompileTime == 1 ? 0 : 1); + VERIFY_IS_EQUAL(ResizeScalar::copies, 0); + VERIFY_IS_EQUAL(ResizeScalar::moves, 0); + VERIFY(v.data() == data); +} + +template <typename Vector> +void heap_vector_resize() { + for (bool no_change : {false, true}) { + Vector v = Vector::LinSpaced(3, 1, 3); + for (Index size : {5, 2, 0, 4}) { + const Index common_size = (std::min)(size, v.size()); + if (!no_change) { + v.conservativeResize(Vector::RowsAtCompileTime == 1 ? 1 : size, Vector::RowsAtCompileTime == 1 ? size : 1); + } else if (Vector::RowsAtCompileTime == 1) { + v.conservativeResize(NoChange, size); + } else { + v.conservativeResize(size, NoChange); + } + VERIFY_IS_EQUAL(v.rows(), Vector::RowsAtCompileTime == 1 ? 1 : size); + VERIFY_IS_EQUAL(v.cols(), Vector::RowsAtCompileTime == 1 ? size : 1); + for (Index i = 0; i < common_size; ++i) VERIFY_IS_EQUAL(v(i), i + 1); + for (Index i = common_size; i < size; ++i) v(i) = typename Vector::Scalar(i + 1); + } + // A zero fixed dimension used to free the buffer without changing the vector's length. + if (Vector::RowsAtCompileTime == 1) { + VERIFY_RAISES_ASSERT(v.conservativeResize(0, v.cols())); + VERIFY_RAISES_ASSERT(v.conservativeResize(0, NoChange)); + } else { + VERIFY_RAISES_ASSERT(v.conservativeResize(v.rows(), 0)); + VERIFY_RAISES_ASSERT(v.conservativeResize(NoChange, 0)); + } + VERIFY_IS_EQUAL(v.size(), 4); + for (Index i = 0; i < v.size(); ++i) VERIFY_IS_EQUAL(v(i), i + 1); + } +} + EIGEN_DECLARE_TEST(conservative_resize) { + CALL_SUBTEST_7((same_shape_vector_resize<Matrix<InitializedResizeScalar, Dynamic, 1>>())); + CALL_SUBTEST_7((same_shape_vector_resize<Matrix<InitializedResizeScalar, 1, Dynamic>>())); + CALL_SUBTEST_7((same_shape_vector_resize<Array<InitializedResizeScalar, Dynamic, 1>>())); + CALL_SUBTEST_7((same_shape_vector_resize<Array<InitializedResizeScalar, 1, Dynamic>>())); + CALL_SUBTEST_7((two_dimensional_vector_resize<Matrix<ResizeScalar, Dynamic, 1, ColMajor, 16, 1>>())); + CALL_SUBTEST_7((two_dimensional_vector_resize<Matrix<ResizeScalar, 1, Dynamic, RowMajor, 1, 16>>())); + CALL_SUBTEST_7((two_dimensional_vector_resize<Array<ResizeScalar, Dynamic, 1, ColMajor, 16, 1>>())); + CALL_SUBTEST_7((two_dimensional_vector_resize<Array<ResizeScalar, 1, Dynamic, RowMajor, 1, 16>>())); + CALL_SUBTEST_7(heap_vector_resize<VectorXd>()); + CALL_SUBTEST_7(heap_vector_resize<RowVectorXd>()); + CALL_SUBTEST_7(heap_vector_resize<ArrayXd>()); + CALL_SUBTEST_7((heap_vector_resize<Array<double, 1, Dynamic>>())); for (int i = 0; i < g_repeat; ++i) { CALL_SUBTEST_1((run_matrix_tests<int, Eigen::RowMajor>())); CALL_SUBTEST_1((run_matrix_tests<int, Eigen::ColMajor>()));