Re: [eigen] Decrease in performances

[Maxime REIS <maxime.reis@xxxxxxxxx>]

Hi,

I ran my own bench on my project using dynamic size matrices, and there is a huge perf loss between 3.0.3 and the head (after applying the patch). I'll try and isolate a working snippet that shows this regression, and post it ASAP.

As I previously stated though, perf are still good on static size matrices (I can see no difference between before the perf regression and after applying the patch).

Regards,

Maxime

On 26 June 2012 23:07, Gael Guennebaud <gael.guennebaud@xxxxxxxxx> wrote:

Patch improved and applied:

https://bitbucket.org/eigen/eigen/changeset/46124a1d2c6c/
changeset:   46124a1d2c6c
user:        ggael
date:        2012-06-26 22:16:07
summary:     fix performance regression due to
check_rows_cols_for_overflow and add appropriate assertions in the
PlainObjectBase::resize() functions.
The fix consists in disabling this useless test for statically
allocated objects.


Now about optimizing the check itself, before implementing subtle
mechanisms I'd like to see an example of any performance hit. Here is
my test program:

#include <iostream>
#include <Eigen/Dense>
#include <bench/BenchTimer.h>

using namespace Eigen;
const int N = 4;
//typedef Matrix<float,N,N> Mat;
typedef Matrix<float,Dynamic,Dynamic> Mat;

EIGEN_DONT_INLINE void foo(Mat& a, Mat& b, int n)
{
 a.resize(n,n);
}


int main()
{
 BenchTimer t;
 Mat a(N,N), b(N,N);
 a.setZero();
 b.setZero();
 BENCH(t,10,10000,foo(a,b,N));
 std::cout << t.best() << "\n";
}


With a fixed size type I've been able to see a clear difference with
and without the overflow test. However, with a dynamic size, I cannot
see any difference...

Maxime: would it be possible to have a snippet of the part of your
code that exhibited the perf regression? I known it uses fixed size
matrices but I'd like to test it with dynamic sizes too.

gael

On Tue, Jun 26, 2012 at 10:23 AM, Maxime REIS <maxime.reis@xxxxxxxxx> wrote:
> Hi,
>
> Well that was quick.
> The patch works for me. Perfs are as they were in 3.0.3, and as far as I can
> tell, nothing seems to be broken.
> Thanks a lot !
>
> Regards,
> Maxime Reis
>
> On 26 June 2012 09:50, Gael Guennebaud <gael.guennebaud@xxxxxxxxx> wrote:
>>
>> Hi,
>>
>> thanks for catching the precise changeset. I think this is a serious
>> issue which has to be addressed.
>>
>> The attached patch is a first step that does two things:
>> 1 - it removes the integer overflow detection for objects which are
>> statically allocated (MaxColsAtCompileTime!=Dynamic &&
>> MaxRowsAtCompileTime!=Dynamic)
>> 2 - it adds an assertion in resize() to ensure that the sizes are
>> positive and equal to the fixed sizes (if any) or lower than the
>> maximal fixed sizes if any.
>>
>> Indeed, currently doing:
>>
>> Matrix4f A;
>> A.resize(-1,-4);
>> or:
>> A.resize(4,8);
>>
>> triggers an exception in the first case, and nothing in the second
>> case while they are both programming errors.
>>
>> Maxime: could you try this patch to see if it works for you?
>>
>> The second step is to optimize the overflow detection itself, let's
>> check what the other proposed....
>>
>> cheers,
>> gael
>>
>>
>> On Mon, Jun 25, 2012 at 4:36 PM, Maxime REIS <maxime.reis@xxxxxxxxx>
>> wrote:
>> > Hi,
>> >
>> > Everything is fine up to the fix of 'Bug 363' :
>> > - 4281:25ba289d5292 runs fast
>> > - 4282:a07628375b86 doesn't compile
>> > - 4283:f76f77b7d086 is sloooooooow
>> > The bug fix apparently slowed down the whole thing, and the perfs were
>> > improved again in c29d777b278c (some 40 or so commits after the
>> > decrease),
>> > but not to the point they were before the bug fix.
>> > I guess it's the price to pay to have a robust library. From that point
>> > on,
>> > all subsequent commits don't seem to affect overall performances (at
>> > least
>> > from my observations)
>> >
>> > I don't know if it deserves further investigation...
>> >
>> > Best regards,
>> > Maxime REIS
>> >
>> > On 25 June 2012 15:26, Gael Guennebaud <gael.guennebaud@xxxxxxxxx>
>> > wrote:
>> >>
>> >> Hi,
>> >>
>> >> in-between the 3.0.3 and 3.0.4 release I see only the 5 following
>> >> change-sets that could affect the performances:
>> >>
>> >> 248f1d7c5f86
>> >> dfea1f0cbf7a
>> >> e5f87fa9e5c2
>> >> f95322d996f2
>> >> 9c5248ab7ac1
>> >>
>> >> using bissection strategy you should be able to find exact change-set
>> >> that introduced a regression with at most 3 tries. Not that if you are
>> >> using SelfAdjointEigenSolver then the slowdown might simply be due to
>> >> to lack of precision in previous releases (see changeset
>> >> f95322d996f2).
>> >>
>> >>
>> >> thanks for your help,
>> >>
>> >> gael
>> >>
>> >> On Mon, Jun 25, 2012 at 12:10 PM, Maxime REIS <maxime.reis@xxxxxxxxx>
>> >> wrote:
>> >> > Hi everyone,
>> >> >
>> >> > A few releases ago, I noticed a slight decrease in overall
>> >> > performances
>> >> > in
>> >> > my soft, using eigen. Since execution time is my main concern and the
>> >> > reason
>> >> > why I chose eigen in the first place, I ran a few tests to make sure
>> >> > :
>> >> > I recompiled my project using all 3.x releases of eigen (and some
>> >> > 2.0.x
>> >> > too), and compared the execution time of some algorithms I
>> >> > implemented.
>> >> > Versions 3.0.0 to 3.0.3 gave fairly stable results, but versions
>> >> > 3.0.4,
>> >> > 3.0.5 and 3.1 made my execution time up to 50% longer on some
>> >> > critical
>> >> > algorithms (performances are similar in the last three versions
>> >> > though).
>> >> > All
>> >> > my code uses mostly basic matrix operations (product, transpose,
>> >> > submatrix
>> >> > extraction) on fixed-size matrices.
>> >> >
>> >> > I can try and be more precise on what takes more time if needed, but
>> >> > I
>> >> > think
>> >> > the eigen benchmark suite would be better suited for that purpose
>> >> > (the
>> >> > last
>> >> > published bench is over a year old...)
>> >> > Has anyone else had similar problems with performances ?
>> >> >
>> >> > Regards,
>> >> > Maxime REIS.
>> >>
>> >>
>> >
>
>