Improve implementation of constructors

[elfring]

I have noticed that a couple of assignments are used in the constructor bodies.

Examples:

• EventQueue
• MessageQueue
• MethodInfo

The recommended way for performing efficient construction is to use the initialisation list.

• C++ FAQ
• Article by Alex Allain
• Description by Danny Kalev

[reduz]

Oh, I hate initialization lists, find them unreadable :|

On Thu, Feb 13, 2014 at 11:55 AM, Markus Elfring
notifications@github.comwrote:

I have noticed that a couple of assignments are used in the constructor
bodies.

Examples:

• EventQueuehttps://github.com/okamstudio/godot/blob/0b806ee0fc9097fa7bda7ac0109191c9c5e0a1ac/core/event_queue.cpp#L126
• MessageQueuehttps://github.com/okamstudio/godot/blob/0b806ee0fc9097fa7bda7ac0109191c9c5e0a1ac/core/message_queue.cpp#L396
• MethodInfohttps://github.com/okamstudio/godot/blob/0b806ee0fc9097fa7bda7ac0109191c9c5e0a1ac/core/object.cpp#L60

The recommended way for performing efficient construction is to use the
initialisation list.

• C++ FAQ http://dietmar-kuehl.de/mirror/c++-faq/ctors.html#faq-10.6
• Article by Alex Allainhttp://cprogramming.com/tutorial/initialization-lists-c++.html
• Description by Danny Kalevhttp://www.informit.com/guides/content.aspx?g=cplusplus&seqNum=15&rl=1

Reply to this email directly or view it on GitHubhttps://github.com//issues/58
.

[gamedevsam]

This is totally syntactic sugar. Not really sure this deserves to be an issue.

[ghost]

This is not syntactic sugar. Initialization by assignment uses an operator, which involves one extra method lookup per assignment. Initialization lists therefore perform better. Language features are not syntactic sugar when used as intended.

Initialization lists can be readable, depending on how you format them. C++ ignores whitespace, so I'll usually have one to three initializations per line with aligned indentation. Here's an example in a copy constructor.

    /**> Copy Constructor                                                                       */
    method_interface ( const method_interface& original ) :
        parent       ( original.parent      ),
        _method      ( original._method     ),
        signature    ( original.signature   ),
        hash_code    ( original.hash_code   )

    { }

edit: The real drawback to initialization lists is that they initialize by constructor, so there has to be a constructor compatible with the expression used to initialize each class member. It's not much of a drawback if the objects initialized are well-written.

[bwzuk]

I think you'd find any real world performance increase after compiler
optimisation to be negligible.

On 17 February 2014 09:46, Gnostici notifications@github.com wrote:

This is not syntactic sugar. Initialization by assignment uses an
operator, which involves one extra method lookup per assignment.
Initialization lists therefore perform better. Language features are not
syntactic sugar when used as intended.

Initialization lists can be readable, depending on how you format them.
C++ ignores whitespace, so I'll usually have one to three initializations
per line with aligned indentation. Here's an example in a copy constructor.

/**> Copy Constructor                                                                       */
method_interface ( const method_interface& original ) :
    parent       ( original.parent      ),
    _method      ( original._method     ),
    signature    ( original.signature   ),
    hash_code    ( original.hash_code   )

{ }

Reply to this email directly or view it on GitHubhttps://github.com//issues/58#issuecomment-35241902
.

[ghost]

That seems to leave a lot to faith in the compiler that one might wish to ensure oneself, especially for code intended to compile in different environments, with different compilers, to include those still implementing newer standards.

The performance gain is not negligible in all circumstances, as numerous copies can be avoided; often at least one for whatever is done within an assignment operator method, and another returning from that method. Implicit copies and conversions can introduce more -- C++ is very bad about that. That might be circumvented by move semantics (RValue References), but we can't count on that occurring in every library we might use.

In fact, this issue is exactly why move semantics exist and RValue References are useful.

One thing that often troubles me about the C++ community is the way that dialects of the language seem to be preferred culturally with adamantly closed minds about when different approaches are preferable. Doesn't make more sense to assume that each approach has its appropriate time and place? This is not meant to marginalize the ideas on this page, but it's worth pointing out that for any adamant statement about the language one can often find more convincing and detailed arguments to the contrary -- at least within caveats.

[reduz]

I think readability is a better argument than performance, and to me
initialization lists are unreadable

On Mon, Feb 17, 2014 at 12:33 PM, Gnostici notifications@github.com wrote:

That seems to leave a lot to faith in the compiler that one might wish to
ensure oneself, especially for code intended to compile in different
environments, with different compilers, to include those still implementing
newer standards.

The performance gain is not negligible in all circumstances, as numerous
copies can be avoided; often at least one for whatever is done within an
assignment operator method, and another returning from that method. That
might be circumvented by move semantics (RValue References), but we can't
count on that occurring in every library we might use.

One thing that often troubles me about the C++ community is the way that
dialects of the language seem to be preferred culturally with adamantly
closed minds about when different approaches are preferable. Doesn't make
more sense to assume that each approach has its appropriate time and place?

Reply to this email directly or view it on GitHubhttps://github.com//issues/58#issuecomment-35293225
.

[bwzuk]

I certainly don't disagree that initialisation lists are the recommended
way of setting member values. Personally I was just suggesting that in
terms of the godot engine, if you profiled it before and after, I would
suspect you would see no difference in its performance on any of the target
platforms it is compiled for, and therefore performance is not a good
argument to rely on unless you have some profile which does indicate this
is a top bottleneck.

So I'm not being closed minded, only questioning within the list of cool
development people could carry out on this code, extra functionality, more
platforms etc, whether this should be a high priority, even without the
readability discussion which I'm not going to wade into on either side :)

On 17 February 2014 15:47, reduz notifications@github.com wrote:

I think readability is a better argument than performance, and to me
initialization lists are unreadable

On Mon, Feb 17, 2014 at 12:33 PM, Gnostici notifications@github.com
wrote:

That seems to leave a lot to faith in the compiler that one might wish to
ensure oneself, especially for code intended to compile in different
environments, with different compilers, to include those still
implementing
newer standards.

The performance gain is not negligible in all circumstances, as numerous
copies can be avoided; often at least one for whatever is done within an
assignment operator method, and another returning from that method. That
might be circumvented by move semantics (RValue References), but we can't
count on that occurring in every library we might use.

One thing that often troubles me about the C++ community is the way that
dialects of the language seem to be preferred culturally with adamantly
closed minds about when different approaches are preferable. Doesn't make
more sense to assume that each approach has its appropriate time and
place?

Reply to this email directly or view it on GitHub<
https://github.com/okamstudio/godot/issues/58#issuecomment-35293225>

.

Reply to this email directly or view it on GitHubhttps://github.com//issues/58#issuecomment-35295016
.

[ghost]

Both of those arguments make perfect sense! It will be some time before I can really experiment with Godot's source, as I'm running into one Scons wall after another due to my aversion to Visual Studio (I know it's good for many... this is a personal quirk).

Once I manage to catch up to the herd compiling and catch up on my queue of goals, I'll have to try some testing just to see this aspect for myself. It may be an awesome learning experience!

[elfring]

How do you think about to avoid the default initialisation of string objects in copy constructors for example?

[ghost]

That's almost a trick question. flags is already allocated at this point, so an assign-copy should not be happening. Move semantics also don't apply because the source data is a const that may be used elsewhere. Also, the assignment operator does the copying within the implementation of String.

Granted, I haven't looked at the implementation of String yet, but I can't imagine why flags would be copy assigned there considering that first its data would have to be deallocated, set to void, reallocated, and then the copy could happen anyway. That would involve several unnecessary steps for a setter method; especially if you don't use new ( ) and delete ( ).

If within String you have a char array, then that could explain it but then I'd still need to look at the implementation before I could guess further. Since you're not using the standard library, your best performance gain in that case would be to use a fast resize and copy algorithm -- give yourself extra space on the first allocation to reduce cases of reconstruction and loop over more than one char at a time wherever possible.

Or, if we really wanted to get to the brass tacks performance wise, that's where you take your memory management down to C.