Re: [eigen] Eigen2, G++ and sizeof()

On Jan 27, 2008 12:48 PM, Benoît Jacob <jacob@xxxxxxxxxxxxxxx> wrote:

Hi,

Thanks for your mail. I tried with g++ 4.2.3 and got the same result.

You really found a bug in Eigen: I made the wrong assumption that empty
members in a class didn't increase the sizeof().

Apparently this is a classical issue and there are several known workarounds:
http://www.google.com/search?q=c%2B%2B+sizeof+empty+class

I do not have time right now to look deeper into this issue but I will do it
as soon as I get back to coding (which is not before in one month). Or, if
you are interested in doing it yourself, I am always happy to receive
patches. A good solution should provide optimal sizeof() in all cases,
without obfuscating the source code too much.

Cheers,

Benoit

On Saturday 26 January 2008 21:17:21 Andri Möll wrote:
> Morning,
>
> I was just reviewing Eigen2 and discovered that for some reason GCC's g++
> lists Eigen's matrices a bit larger than they were meant to be.
> That is, 16 bytes for Vector3f unpacked, and 14 bytes when packed with
> #pragma pack(1).
>
> #include <iostream>
> #include "eigen2/Eigen/Core"
> int main(void)
> {
> Eigen::Vector3f v;
> std::cerr << "sizeof(v) = " << sizeof(v) << std::endl;
> std::cerr << "v.size() = " << v.size() << std::endl;
> }
>
> $ g++ -Wall test.cpp && ./a.out
> sizeof(eigen) = 16
> eigen.size() = 3
>
> My disassembling skills under Unix-like OSes are a bit lacking, but a
> simple 'info gcc' and removal of some code helped identify the cause, which
> seems to be the templated value holding class you're using. GCC's manual
> says that empty classes end up with a "virtual" char, so that explains why
> temporarily removing dynamic matrix support and substituting it with enums
> makes the data usage Eigen1-like.
>
> I'm using Gentoo's stable GCC (4.1.2) and rev 766861 of Eigen2.
>
> I haven't tried the 4.2.x branch of GCC, does that have the same behavior?
>
>
> Andri

Index: Eigen/src/Core/Matrix.h =================================================================== --- Eigen/src/Core/Matrix.h (revision 774209) +++ Eigen/src/Core/Matrix.h (working copy) @@ -26,25 +26,33 @@ #ifndef EIGEN_MATRIX_H #define EIGEN_MATRIX_H -template<typename T, int Size> class Array +template<typename T, int Size, int _Rows, int _Cols> class Array { T m_data[Size]; public: Array() {} - explicit Array(int) {} - void resize(int) {} + explicit Array(int,int,int) {} + static int rows(void) {return _Rows;} + static int cols(void) {return _Cols;} + void resize(int,int,int) {} const T *data() const { return m_data; } T *data() { return m_data; } }; -template<typename T> class Array<T, Dynamic> +template<typename T> class Array<T, Dynamic, Dynamic, Dynamic> { T *m_data; + int m_rows; + int m_cols; public: - explicit Array(int size) : m_data(new T[size]) {} + explicit Array(int size, int nbRows, int nbCols) : m_data(new T[size]), m_rows(nbRows), m_cols(nbCols) {} ~Array() { delete[] m_data; } - void resize(int size) + int rows(void) const {return m_rows;} + int cols(void) const {return m_cols;} + void resize(int size, int nbRows, int nbCols) { + m_rows = nbRows; + m_cols = nbCols; delete[] m_data; m_data = new T[size]; } @@ -52,6 +60,44 @@ T *data() { return m_data; } }; +template<typename T, int _Rows> class Array<T, Dynamic, _Rows, Dynamic> +{ + T *m_data; + int m_cols; + public: + explicit Array(int size, int, int nbCols) : m_data(new T[size]), m_cols(nbCols) {} + ~Array() { delete[] m_data; } + static int rows(void) {return _Rows;} + int cols(void) const {return m_cols;} + void resize(int size, int, int nbCols) + { + m_cols = nbCols; + delete[] m_data; + m_data = new T[size]; + } + const T *data() const { return m_data; } + T *data() { return m_data; } +}; + +template<typename T, int _Cols> class Array<T, Dynamic, Dynamic, _Cols> +{ + T *m_data; + int m_rows; + public: + explicit Array(int size, int nbRows, int) : m_data(new T[size]), m_rows(nbRows) {} + ~Array() { delete[] m_data; } + int rows(void) const {return m_rows;} + static int cols(void) {return _Cols;} + void resize(int size, int nbRows, int) + { + m_rows = nbRows; + delete[] m_data; + m_data = new T[size]; + } + const T *data() const { return m_data; } + T *data() { return m_data; } +}; + /** \class Matrix * * \brief The matrix class, also used for vectors and row-vectors @@ -124,28 +170,26 @@ : _MaxRows * _MaxCols }; - IntAtRunTimeIfDynamic<RowsAtCompileTime> m_rows; - IntAtRunTimeIfDynamic<ColsAtCompileTime> m_cols; - Array<Scalar, MaxSizeAtCompileTime> m_array; + Array<Scalar, MaxSizeAtCompileTime, RowsAtCompileTime, ColsAtCompileTime> m_array; Ref _ref() const { return Ref(*this); } - int _rows() const { return m_rows.value(); } - int _cols() const { return m_cols.value(); } + int _rows() const { return m_array.rows(); } + int _cols() const { return m_array.cols(); } const Scalar& _coeff(int row, int col) const { if(StorageOrder == ColumnMajor) - return m_array.data()[row + col * m_rows.value()]; + return m_array.data()[row + col * m_array.rows()]; else // RowMajor - return m_array.data()[col + row * m_cols.value()]; + return m_array.data()[col + row * m_array.cols()]; } Scalar& _coeffRef(int row, int col) { if(StorageOrder == ColumnMajor) - return m_array.data()[row + col * m_rows.value()]; + return m_array.data()[row + col * m_array.rows()]; else // RowMajor - return m_array.data()[col + row * m_cols.value()]; + return m_array.data()[col + row * m_array.cols()]; } public: @@ -165,9 +209,7 @@ && cols > 0 && (MaxColsAtCompileTime == Dynamic || MaxColsAtCompileTime >= cols) && (ColsAtCompileTime == Dynamic || ColsAtCompileTime == cols)); - m_rows.setValue(rows); - m_cols.setValue(cols); - m_array.resize(rows * cols); + m_array.resize(rows*cols,rows,cols); } /** Copies the value of the expression \a other into *this. @@ -230,9 +272,7 @@ * it is redundant to pass the dimension here, so it makes more sense to use the default * constructor Matrix() instead. */ - explicit Matrix(int dim) : m_rows(RowsAtCompileTime == 1 ? 1 : dim), - m_cols(ColsAtCompileTime == 1 ? 1 : dim), - m_array(dim) + explicit Matrix(int dim) : m_array(dim,RowsAtCompileTime == 1 ? 1 : dim,ColsAtCompileTime == 1 ? 1 : dim) { assert(dim > 0); assert((RowsAtCompileTime == 1 @@ -251,7 +291,7 @@ * it is redundant to pass these parameters, so one should use the default constructor * Matrix() instead. */ - Matrix(int x, int y) : m_rows(x), m_cols(y), m_array(x*y) + Matrix(int x, int y) : m_array(x*y,x,y) { if((RowsAtCompileTime == 1 && ColsAtCompileTime == 2) || (RowsAtCompileTime == 2 && ColsAtCompileTime == 1)) @@ -307,17 +347,13 @@ /** Constructor copying the value of the expression \a other */ template<typename OtherDerived> Matrix(const MatrixBase<Scalar, OtherDerived>& other) - : m_rows(other.rows()), - m_cols(other.cols()), - m_array(other.rows() * other.cols()) + : m_array(other.rows() * other.cols(), other.rows(), other.cols()) { *this = other; } /** Copy constructor */ Matrix(const Matrix& other) - : m_rows(other.rows()), - m_cols(other.cols()), - m_array(other.rows() * other.cols()) + : m_array(other.rows() * other.cols(), other.rows(), other.cols()) { *this = other; } Index: Eigen/src/Core/Map.h =================================================================== --- Eigen/src/Core/Map.h (revision 774209) +++ Eigen/src/Core/Map.h (working copy) @@ -188,7 +188,7 @@ template<typename _Scalar, int _Rows, int _Cols, int _StorageOrder, int _MaxRows, int _MaxCols> Matrix<_Scalar, _Rows, _Cols, _StorageOrder, _MaxRows, _MaxCols> ::Matrix(const Scalar *data, int rows, int cols) - : m_rows(rows), m_cols(cols), m_array(rows*cols) + : m_array(rows*cols, rows, cols) { *this = map(data, rows, cols); } @@ -206,9 +206,7 @@ template<typename _Scalar, int _Rows, int _Cols, int _StorageOrder, int _MaxRows, int _MaxCols> Matrix<_Scalar, _Rows, _Cols, _StorageOrder, _MaxRows, _MaxCols> ::Matrix(const Scalar *data, int size) - : m_rows(RowsAtCompileTime == 1 ? 1 : size), - m_cols(ColsAtCompileTime == 1 ? 1 : size), - m_array(size) + : m_array(size, RowsAtCompileTime == 1 ? 1 : size, ColsAtCompileTime == 1 ? 1 : size) { *this = map(data, size); }