Loops, Constructors and Performance

beecksche

Hi,
code is worth a thousand words ;-)
I have this loop in my class:

ON_3dPoint on_p;
ON_3dVector on_du1;
ON_3dVector on_dv1;

QVector3D p;
QVector3D duu;
QVector3D dvv;

while (...) {
    ...
    m_surface.Ev1Der(u, v, on_p, on_du1, on_dv1);

    p.setX(on_p.x);
    p.setY(on_p.y);
    p.setZ(on_p.z);

    duu.setX(on_du1.x);
    duu.setY(on_du1.y);
    duu.setZ(on_du1.z);

    dvv.setX(on_dv1.x);
    dvv.setY(on_dv1.y);
    dvv.setZ(on_dv1.z);
    ...
}

Is it better to use the setter function of QVector3D or to put the objects p, duu and dvv into the while loop:

ON_3dPoint on_p;
ON_3dVector on_du1;
ON_3dVector on_dv1;

while (...) {
    ...
    m_surface.Ev1Der(u, v, on_p, on_du1, on_dv1);

    QVector3D p = QVector3D(on_p.x, on_p.y, on_p.z);
    QVector3D duu = QVector3D(on_du1.x, on_du1.y, on_du1.z);
    QVector3D dvv = QVector3D(on_dv1.x, on_dv1.y, on_dv1.z);
    ...
}

To read the code I would prefer the second way. But I think that I read something about, that you should create a object outside of a loop.

Do you know some advantages or disadvantages of this? I'm working with the MSVC 2017 compiler (if this is necessary)
Thanks a lot!

sierdzio

@beecksche said in Loops, Constructors and Performance:

To read the code I would prefer the second way. But I think that I read something about, that you should create a object outside of a loop.

The second way is better (throw in a const there, though, if you don't modify the vectors) in my opinion, not only due to readability. You avoid calling multiple methods on an object (potentially less overhead), you avoid making hard-to-debug errors (object holding outdated information from previous loop run), you don't needlessly keep vectors in memory after the loop finishes.

aha_1980

@beecksche

The golden rule is:

Write the code so it is easy to understand and maintain.
If you hit performance problems, profile your code to find bottlenecks
Refactor your code to eliminate the bottlenecks

As you didn't say you already hit 2, you should stick at 1 :)

beecksche

@sierdzio, @aha_1980
Thanks a lot.

For me it's a general problem: it's a juggling act between readability, compactness and speed.

AFAIK the compiler optimizes the code quite well to gain some performance. And so far I hadn't problems with the performance, therefore I think I'll follow your (@aha_1980) golden rule. Sounds logical for me.

Thanks guys!

sierdzio

@beecksche said in Loops, Constructors and Performance:

For me it's a general problem: it's a juggling act between readability, compactness and speed.

It's a problem we're all facing :-) But as @aha_1980 said, there is no need to worry about performance too early. Our intuitive take on what is faster is often wrong, too - his hint at profiling the code is a very good advice.

Kent-Dorfman

while (...) {
    ...
    m_surface.Ev1Der(u, v, on_p, on_du1, on_dv1);

    QVector3D p = QVector3D(on_p.x, on_p.y, on_p.z);
    QVector3D duu = QVector3D(on_du1.x, on_du1.y, on_du1.z);
    QVector3D dvv = QVector3D(on_dv1.x, on_dv1.y, on_dv1.z);
    ...
}

This is the more C++ way of doing it because it limits the scope of the objects to where they are used. You can drive yourself nuts worrying about premature optimization issues of what's more efficient so that's not as big an initial concern.

One change I do recommend going forward is that instead of using the

Ojbect o = Object(a,b,c) form you should use the C++ way of initializing object as:

Object o(a,b,c) This syntax has the advantage that it is explicitly running the constructor only. The previous syntaxt may do both: construct the object into a temporary and then assign it to the destination object: 2 operations!

VRonin

@Kent-Dorfman said in Loops, Constructors and Performance:

Ojbect o = Object(a,b,c) form you should use the C++ way of initializing object as:
Object o(a,b,c)

Minor note auto o = Object(a,b,c) is also acceptable

Konstantin Tokarev

@beecksche said in Loops, Constructors and Performance:

But I think that I read something about, that you should create a object outside of a loop.

In this case it doesn't matter, because you overwrite all vector elements on each iteration, and constructor of QVector3D doesn't make any dynamic allocations (QVector3D is actually float[3] with fancy API around)

Konstantin Tokarev

@VRonin said in Loops, Constructors and Performance:

Minor note auto o = Object(a,b,c) is also acceptable

Why do this when just QVector3D p(a, b, c) can work?

J.Hilk

@Konstantin-Tokarev said in Loops, Constructors and Performance:

@VRonin said in Loops, Constructors and Performance:

Minor note auto o = Object(a,b,c) is also acceptable

Why do this when just QVector3D p(a, b, c) can work?

Why use auto at all... It looks more modern and fancy.

I'm with Bjarne Stroustrup and wish auto would have never been added to the standard.

VRonin

@Konstantin-Tokarev said in Loops, Constructors and Performance:

Why do this when just QVector3D p(a, b, c) can work?

Linking ≠ Endorsement
https://www.youtube.com/watch?v=xnqTKD8uD64&t=40m00s

Kent-Dorfman

@J.Hilk said in Loops, Constructors and Performance:

@Konstantin-Tokarev said in Loops, Constructors and Performance:

@VRonin said in Loops, Constructors and Performance:

Minor note auto o = Object(a,b,c) is also acceptable

Why do this when just QVector3D p(a, b, c) can work?

Why use auto at all... It looks more modern and fancy.

I'm with Bjarne Stroustrup and wish auto would have never been added to the standard.

Actually I think Bjarne was misrepresented on that one. When I read his review of c++11, he seemed to at least be "politicially positive" about the new features...and really, auto is comes into its own when using long winded iterator declarations.

for(auto i: container) {} is so much more elegant than manually declaring the iterator type

but many of these arguements are just esoteric/philosophical rants to exercise and justify our OCD. Programmers with OCD?...yeah, right!

kshegunov

@VRonin
How is

auto o = Object(a, b, c);

different from:

Object o = Object(a, b, c);

for the generated code?

@Kent-Dorfman
In my mind auto was invented for the template junkies out there. The only real reason to actually "need" it in non-(heavily-)templated code is when doing ranged loops with arrays of anonymous structures. Something like this:

static struct  {
   int member1;
   // ....
} anonymousGlobal[] =  {
  { 0, /* ... */ }
};

for (auto x : anonymousGlobal)
    // ...

And even then I'd put some question on the clarity of such code.

Kent-Dorfman

@Kent-Dorfman

In my mind auto was invented for the template junkies out there. The only real reason to actually "need" it in non-(heavily-)templated code is when doing ranged loops with arrays of anonymous structures. Something like this:
static struct  {
   int member1;
   // ....
} anonymousGlobal[] =  {
  { 0, /* ... */ }
};
for (auto x : anonymousGlobal)
    // ...
And even then I'd put some question on the clarity of such code.

fair enough...too often features become peoples religion and they go out of their way to overuse or justify their use. I personally think auto is a good idea, for specifically the reasons specified...it is a good shorthand for complex type declarations and I hate being long-winded, although you usually couldn't tell that from my written correspondence. I've been in python land for a while now and am less than anxious to see all the overuse of lambdas that have undoubtedly sprung up now that they are in the c++ spec. lambdas are my soap-box rant.

VRonin

@kshegunov said in Loops, Constructors and Performance:

How is
auto o = Object(a, b, c);
different from:
Object o = Object(a, b, c);

Actually all 3 forms are the same, the compiler (MSVC2015 and GCC 4.9 at least) takes care of doing the right thing:

#include <QDebug>
class Object{
public:
    Object(int){qDebug("Constructor");}
    Object(const Object&){qDebug("Copy Constructor");}
    Object(Object&&){qDebug("Move Constructor");}
};

int main(int argc, char ** argv)
{
   Object a(6);
   qDebug("############");
   Object b = Object(6);
   qDebug("############");
   auto c = Object(6);
   qDebug("############");
   return 0;
}

Outputs

Constructor
############
Constructor
############
Constructor
############

So we are worrying about a non-problem

Christian Ehrlicher

@Kent-Dorfman said in Loops, Constructors and Performance:

Object o(a,b,c) This syntax has the advantage that it is explicitly running the constructor only. The previous syntaxt may do both: construct the object into a temporary and then assign it to the destination object: 2 operations!

I would accept this answer 10 years ago but not nowadays and with c++14 it's wrong at all:
https://en.wikipedia.org/wiki/Copy_elision and https://en.cppreference.com/w/cpp/language/copy_elision

kshegunov

@Christian-Ehrlicher said in Loops, Constructors and Performance:

I would accept this answer 10 years ago but not nowadays and with c++14 it's wrong at all

Maybe in a perfect world. The compiler I use on the cluster has c++11 and partial c++14 support. Old, but it's out of my hands. Considerations to such contexts apply.

Christian Ehrlicher

gcc has '-fno-elide-constructors' since 4.0 so what ancient compiler do you use? ;)

kshegunov

This disables eliding - the optimization you're after. So the expression:

Object a = Object(x, y, z);

is going to always invoke the copy constructor with that flag. On the other hand -felide-constructors may or may not generate a constructor call depending on the specific version and optimizations the compiler does.