Performance vs Readability: Local copies in functions

Question

Consider the following code:

Vector2f Box::getCenter() const
{
    const float x = width / 2;
    const float y = height / 2;

    return Vector2f(x, y);
}

Is there a performance increase to instead write it like this:

Vector2f Box::getCenter() const
{
    return Vector2f(width / 2, height / 2);
}

I prefer the first one since it is nice and readable, but I am starting to wonder if I am losing some performance if I do this too much since it creates an extra unnecessary copy.

I know that some of you think the second function is just as readable but this is just an example, I am asking more generally and what is good coding practice in this case.

It's a near certainty that a modern C++ compiler will generate identical code in both cases. You can answer your own question all by yourself: all compilers give an option to produce machine language code instead of a binary executable. Use that option in both cases, and see if there's any difference. — Sam Varshavchik, Jul 04 '19 at 13:13
Make a release build (with optimizations enabled) of both versions, and compare the generated code. My guess is that both variants would be equal. — Some programmer dude, Jul 04 '19 at 13:13
In this case this is not an issue. In `C` and `C++` there is ["As if" rule](https://stackoverflow.com/a/15718279/1387438). Compiler can freely transform code to be as fast as possible as long as outcome remains unchanged. So bottom line both examples will produce exactly same machine code. — Marek R, Jul 04 '19 at 13:13
Also note that "readability" is very subjective. For an experienced programmer, both variants are probably equally readable (it is for me anyway). — Some programmer dude, Jul 04 '19 at 13:15
I find the variables in the first a confusing diversion. They look like they're significant, but it's very difficult to guess if or how without examining the surrounding code in detail. — molbdnilo, Jul 04 '19 at 13:29
In fact, the second looks more readable to me. I can immediately tell that we are halving the width and height. For the first one, I have to store x and y to my brain registers, and then load them to interpret the return, which incurs overhead ... — L. F., Jul 04 '19 at 13:30
@L.F. for the given example, yes, but for a more complex example, it is a great help. Especially with good names. `halfWidth` and `halfHeight` might be better names here, unless there is a more specific name relating to intent. — Baldrickk, Jul 04 '19 at 14:37

score 10 · Accepted Answer · edited Jul 04 '19 at 17:32

10

As a rule of thumb: Write code to be readable.

C++ as a language is designed with efficiency as one of the primary goals. In an ideal world, readable code is efficient code and c++ comes rather close to that. It is rare that you are forced to write unreadable code for performance. And even then, you should not do that right from the start. Write code for readability first always. Only when you measure you can know what is more and what is less performing.

This is a nice tool to inspect the compiler output: https://godbolt.org/ I would expect any decent compiler to emit very similar assembly when you turn on optimizations.

This is another tool that lets you easily benchmark code: http://quick-bench.com/. Don't start to optimize things before you know it makes a difference.

Compilers are rather powerful in optimizing things and most of the time they know better than you. Of course you should not do "stupid" things, e.g. clearly you wouldn't return Vector2f(width * sin(pi/2) * exp( ln( 0.5)) , height / 2); even though a super clever compiler could realize that it is the same as your original.

Last but not least, do not forget that you primarily write code for humans to read and understand.

PS: Readability is very subjective. One could argue that the second is more readable and I'd go even one step further by writing

Vector2f Box::getCenter() const
{
    return {width * 0.5 , height * 0.5};
}

This has the added benefit that you do not necessarily need to change the body if you ever decide to change the return type (or if you ever change the value type to int).

edited Jul 04 '19 at 17:32

Costantino Grana

3,132
1
15
35

answered Jul 04 '19 at 13:25

463035818_is_not_an_ai

109,796
11
89
185

Excellent answer. Great to know that my compiler will optimize away stuff like this. I will continue to write it the first way without any shame :D – Oscar Jul 04 '19 at 13:31
I'm not going to downvote, but I disagree with this for the reasons I set out in my answer. – Bathsheba Jul 04 '19 at 13:41
@Oscar hum, if it came across as "write what you like and dont care" then it isnt a great answer ;). The intended message was rather: write for readability and study what your compiler does for performance – 463035818_is_not_an_ai Jul 04 '19 at 13:41
@Bathsheba I read your answer and I think there actually is no disagreement. I tried to keep the answer general (mainly pointing to tools). I added a PS that is more specific to OPs code – 463035818_is_not_an_ai Jul 04 '19 at 13:47
@formerlyknownas_463035818 nah, it didn't come across like that. Ofcourse I'll think about performance, but it's just good to know that the general wisdom is the prioritize readability in a case like this since the optimized could would be the same. – Oscar Jul 04 '19 at 13:47
@formerlyknownas_463035818 I prefer the first one because if I have a longer function name instead of just a short variable name like `width` and `height`, and then the return statement would be so long I would have to scroll horizontally to see it all. – Oscar Jul 04 '19 at 13:52

score 6 · Answer 2 · answered Jul 04 '19 at 13:27

Here is a code written in C++:

Vector2f GetCenterLong(float width, float height)
{
    const float x = width / 2;
    const float y = height / 2;

    return Vector2f(x, y);
}

Vector2f GetCenterShort(float width, float height)
{
    return Vector2f(width / 2, height / 2);
}

Here is the assembly code produced with x64 msvc v19.21 and -Ox optimization. You can notice that there is no difference between the two functions.

__$ReturnUdt$ = 8
width$ = 16
height$ = 24
Vector2f GetCenterLong(float,float) PROC                ; GetCenterLong
        mulss   xmm1, DWORD PTR __real@3f000000
        mov     rax, rcx
        mulss   xmm2, DWORD PTR __real@3f000000
        movss   DWORD PTR [rcx], xmm1
        movss   DWORD PTR [rcx+4], xmm2
        ret     0
Vector2f GetCenterLong(float,float) ENDP                ; GetCenterLong

__$ReturnUdt$ = 8
width$ = 16
height$ = 24
Vector2f GetCenterShort(float,float) PROC             ; GetCenterShort
        mulss   xmm1, DWORD PTR __real@3f000000
        mov     rax, rcx
        mulss   xmm2, DWORD PTR __real@3f000000
        movss   DWORD PTR [rcx], xmm1
        movss   DWORD PTR [rcx+4], xmm2
        ret     0
Vector2f GetCenterShort(float,float) ENDP             ; GetCenterShort

Thus, prefer readability over shortness when a compiler optimization is possible.

score 1 · Answer 3 · edited Nov 08 '19 at 01:51

1

Unless there are curious (and probably unintended) type-conversions then an optimising compiler will produce the same code.

The principal difference between the two is that in the first way you are potentially forcing a conversion to a float type.

Therefore I much prefer the second way: suppose the types for the function change to a double rather than the presumed float? That said

auto x = width / 2;

would be more acceptable.

Many debuggers these days support parameter-by-parameter evaluation, so you're not helping debugging by writing your code out the first way.

edited Nov 08 '19 at 01:51

answered Jul 04 '19 at 13:20

Bathsheba

231,907
34
361
483

Why would it matter if the types change? It would be converted either way when passed to the constructor. – Oscar Jul 04 '19 at 13:36
@Oscar: Imagine if `width` is refactored to a `double`, as is the type of the parameter in the function. If you forget to change the `float` then you have an unwanted truncation. – Bathsheba Jul 04 '19 at 13:40
yes obviously, but it is very far fetched for that to happen since my compiler would warn me. Also I could just use `auto` instead. This was just an example – Oscar Jul 04 '19 at 13:45

eerorika · Answer 4 · 2019-07-04T14:05:31.953

Is there a performance increase to instead write it like this:

Well, did you measure a performance increase? When you care about performance, always measure.

The performance can potentially be different. If the types of width and height as well as the parameters are other than float, then the local variables introduce two conversions, which must be performed. Performing a conversion may be slower than not performing the conversion, and it may cause a different result as well. This dependency on the type can be remedied by using auto type deduction.

But if we assume that there are no conversions involved, then there is no reason to assume that an optimiser couldn't produce the exactly same program in both cases, in which case the performance would be exactly the same. You can compare the assembly generated by the compiler to verify.

I am asking more generally and what is good coding practice in this case.

Readability is more important by default. Often, and in this case as well, readability is subjective.

Performance may be more important if you measure a bottleneck, but this change is unlikely to achieve performance.

Werner Henze · Answer 5 · 2019-07-04T13:18:54.150

0

I assume that Vector2f constructor is taking two floats. Then at least with an optimizing compiler you will see no difference.

If you want to know it exactly for your case you can check the compiler output (probably telling your compiler to keep the intermediate files, on gcc -S -save-temps) or you can check online on https://gcc.godbolt.org/.

edited Jul 04 '19 at 13:18

answered Jul 04 '19 at 13:13

Werner Henze

16,404
12
44
69

Performance vs Readability: Local copies in functions

5 Answers5