• Advertisement
Sign in to follow this  

Do you return const references?

This topic is 4143 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

Say I have a Transform object that holds a Vector3f object (vector of 3 floats obviously) for translation. I don't mean to early optimize, but I am, and I can't help thinking of saving a copy by returning a const reference when asking for it.
const Vector3f& GetTranslation() { return m_Translation; }
I can't think of any reason not to do this. It will always be copied over when used as in an expression or assignment, so you could still do things like
Vector3f Trans = TransformObj->GetTranslation();

or

Vector3f SomeTrans = TransformObj->GetTranslation()+OtherTrans;
And it makes me feel better about calling GetTranslation().x (or y or z) multiple times. The only downside is that the function could never dynamically calculation a translation value and return it, but if I can apply that restraint, is there anything wrong with doing this?

Share this post


Link to post
Share on other sites
Advertisement
I may be wrong, but I believe any decent compiler will optimize a non const reference return value to not make the extra copy.


jfl.

Share this post


Link to post
Share on other sites
Quote:
Original post by jflanglois
I may be wrong, but I believe any decent compiler will optimize a non const reference return value to not make the extra copy.


jfl.


No, I'm comparing it to


Vector3f GetTranslation() { return m_Translation; }


Which, by the standard I believe, is passed by value. That optimization seems dangerous, as if the return value is ever altered, really weird affects would occur when I don't expect it to be altered. Does it really check to see if it's never touched and not do the extra copy?

Share this post


Link to post
Share on other sites
The compiler MAY optimize certain things, but I would always return a const ref in the case you are referring to. i.e.

const Vector3f& GetTranslation() const { return m_Translation; }

I would also never return an object by value assuming the compiler would optimize it. IMHO that is bad coding practice, not to mention certain bugs can be avoided by specifing the method signature returns a const ref.

Share this post


Link to post
Share on other sites
I personally wouldn't return a Vector3f as a reference, because I don't think the saving of copying a couple floats is worth it. However, I ways return larger things like Matrix3x3f as references, or anything that will result in an expensive copy operation like std::string, etc.

Share this post


Link to post
Share on other sites
Quote:
I don't think the saving of copying a couple floats is worth it
What's the cost to save it?

Share this post


Link to post
Share on other sites
Your return type should be dictated first of all by the semantics you want - optimization is a secondary concern. Since Vector3f is likely to be a very small type anyway, speed issues can be completely ignored (even as a secondary concern) until you find that there is a bottleneck caused by this function.

So, the difference in semantics is what should guide you. In this case, it only makes a difference in a fairly unusual case:

If GetTranslation returns a const reference:

Transform foo;

const Vector3f &foo_translation = foo.GetTranslation();

foo.Translate(Vector3f(10.0f, 5.0f, 15.0f));

// foo_translation has now changed
// (assuming an obvious implementation of Transform)




Whereas, if GetTranslation returns a value, foo_translation will not change.

Of course, most of the time you'll either use the result directly, or store it in a variable directly (not assign it to a reference), and so it won't make any difference.

Personally, I would return a value, because I think GetTranslation() should probably return a snapshot of the current translation value, not a reference to a value that may change in the future.

Edit: One thing is certain though, the GetTranslation function itself should be marked const, since it does not modify the contents of the Transform object. ie: Vector3f GetTranslation() const { ... }

John B

Share this post


Link to post
Share on other sites
In programming the small things like copying objects adds up. If you have expression:

float3 n = normalize(a + b * length(c)) + cross(a - b, a - c);

Suddenly HALF the work is copying values around, not actually doing the computations which we intend to to. The ONLY thing here we are interested are n.x, n.y and n.z - the faster we get them the better we are off.

This case doesn't give much leverage for using const reference return values but that's life. :)

One important factor to notice for this thread's sakes is NRVO (Named Return Value Object), it wasn't until fairly recently that Microsoft products started implementing this optimization strategy. They still don't do it perfectly, experimentation to find the facts yourself (tm) is advised. Also pay heed that on different compilers like g++ the rules are again different, it all depends what tools you use. And another warning that should be said is that when again new Microsoft compiler comes out thigns could turn around once again.

The *safest* thing to do is to do the RIGHT THING (tm) to begin with. Then you're not at the mercy of the latest compiler so much. Good, solid code rarely goes slower with the latest compiler upgrade so you guys should be safe when you do the RIGHT THING (tm).

It's up to everyone himself to figure it out what the RIGHT THING (tm) for them. If someone wants to return by value when const reference would do the trick just tandy, that's their call. So what if the value is "optimized" out, that's on the compiler you use today what about tomorrow? What about other platform, other compiler?

x86 is a common compiler am I wrong and Windows is a common platform, right or wrong? Likewise, why care about petty things like endianess or alignment, why bother, those always worked out for my (insert name of my windows application here)

Share this post


Link to post
Share on other sites
I usually prefer returning const references to instances, if the instances consist of several 32-bit variables like matrices.

Though, if you would have a function like "const Vector3f& GetTranslation" and it's not compiled inline it returns a reference/address to the vector. Then when the vector member variables are accessed it's done by addressing the memory location of the variables. If the vector is located in the heap memory how fast access is depends on if it's already in the cache memory for the processor or not. It's likely to be in the cache but if it's not in the cache it'll be slow to get it all the way from the RAM.

If the vector had instead been returned as a copy and stored in a local variable, then access to its member variables is done using an offset value in the stack memory. The local stack memory is probably more likely to be in the cache memory since the local code block is currently executed on the CPU, and in the cache.

But like already discussed, it depends a lot on how the compiler compiles the code, and the processor.

Share this post


Link to post
Share on other sites
For a small type is not really that big a deal because of what torakka mentioned: If you return a const ref then you have the small problem that JohnBSmall has mentioned, if you don't return a const ref you're scared of the copy. But do note torakka's post as well. He mentions NRV (named return value optimization), there's also the RVO (return value optimiation, not named).

Anyway, what RVO/NRV does basically, is get rid of the temp that you're concerned about. A function like:


Vector3f Trans = TransformObj->GetTranslation();

will be transformed such that Trans will become


GetTranslation( Vector3f& dest ) {
dest.Vector3f::Vector3f( m_Translation );
}


So if the compiler implements this optimization, then there's no need to worry really. Im not sure about the guarantees of this transformation happening though.

you can read about NRV in VC++ 2005 here. The above variabtion on GetTransformation is the NRV approach if I'm not mistaken, RVO is implemented a little differently (though in this case I think it'll be implemented the same way??)

But do make the function const (GetTransformation() const).

Share this post


Link to post
Share on other sites
Quote:
Original post by okonomiyaki
Quote:
Original post by jflanglois
I may be wrong, but I believe any decent compiler will optimize a non const reference return value to not make the extra copy.


jfl.


No, I'm comparing it to


Vector3f GetTranslation() { return m_Translation; }


Which, by the standard I believe, is passed by value. That optimization seems dangerous, as if the return value is ever altered, really weird affects would occur when I don't expect it to be altered. Does it really check to see if it's never touched and not do the extra copy?


That is what I meant as well (sorry for being unclear). I was thinking of NRVO and RVO, as mentioned by IFooBar, although I couldn't remember the details.

Share this post


Link to post
Share on other sites
Ah, I see. The optimization skips the creation of the temporary if it can, but still does a copy. That makes sense. Thanks!

I know it doesn't matter much for a 3d vector, but what I really had in mind was matrices and strings and such. I put up a bad example. I think it should return by value, but I'll return const references in a couple cases.

Share this post


Link to post
Share on other sites
Returning a reference to a member variable also puts the burden on calling code to not keep the reference around past the lifetime of the containing class of the member, which is generally speaking A Bad Thing.

I've never seen an actual game that's performance bound by copying small vectors, so this really is a case of premature optimization.

Share this post


Link to post
Share on other sites
So, you like:
const Vector3f& GetTranslation() { return m_Translation; }


Well, how about:
const Vector3f& tmp = foo->GetTranslation();


Now, tmp involves no copies -- it is a raw reference to the data that GetTranslation() returns. This means you have leaked an implementation detail -- you are now forced to store the return value of GetTranslation() for an indefinate period.

Note that copying a Vector3f is 32*3 bytes. Copying a reference to a Vector3f is 32 bytes. The difference is small -- it is possible that this will be a performance bottleneck, but I seriously doubt it. Most programs don't spend most of their time reading Vector3f's from interfaces.

Share this post


Link to post
Share on other sites
Quote:
Original post by JasonBlochowiak
Returning a reference to a member variable also puts the burden on calling code to not keep the reference around past the lifetime of the containing class of the member, which is generally speaking A Bad Thing.

I've never seen an actual game that's performance bound by copying small vectors, so this really is a case of premature optimization.


Quote:
Original post by NotAYakk
So, you like:

const Vector3f& GetTranslation() { return m_Translation; }



Well, how about:

const Vector3f& tmp = foo->GetTranslation();



Now, tmp involves no copies -- it is a raw reference to the data that GetTranslation() returns. This means you have leaked an implementation detail -- you are now forced to store the return value of GetTranslation() for an indefinate period.

Note that copying a Vector3f is 32*3 bytes. Copying a reference to a Vector3f is 32 bytes. The difference is small -- it is possible that this will be a performance bottleneck, but I seriously doubt it. Most programs don't spend most of their time reading Vector3f's from interfaces.


Don't read into this so much - obviously this applies more for 4x4 matrices and strings that are called a lot per frame. For these functions, I don't know anyone who references them like that. They are always used in an expression or copied over to local variable on the stack. And if not, it is misused, and problems will occur with that code, but because of the const, nothing can happen to the referenced data.

Anyway, I've decided to leave it up to the compiler except for certain circumstances where a ton of copying would occur. It annoys me when people tell others to not care about small coding optimizations, and to sloppily write an app and optimize the bottlenecks. Sure, this isn't going save a ton of time, but minor tweaks like this could make a small difference in the end which is good enough for me to care about. Why not make this small difference while I'm coding? I certainly would never go back and optimize an app this way, but while I'm coding I might as well do it as efficiently as possible. Of course, without doing anything stupid that would make it break easily, which in my opinion this is not one of those cases.

Share this post


Link to post
Share on other sites
Quote:
Original post by JohnBSmall
Your return type should be dictated first of all by the semantics you want - optimization is a secondary concern.

***snip***

John B


Most important post of the thread.

Share this post


Link to post
Share on other sites
Quote:

Why not make this small difference while I'm coding?

Because it can be very dangerous.

Quote:

but because of the const, nothing can happen to the referenced data.

Note the issue mentioned about leaking an implementation detail. Because you've created a reference to a member variable of an instance (const or not), you must now maintain the lifetime of that member variable for at least as long as the reference, which is impossible to do unless the class and its clients are pathologically coupled.

Consider:

class foo
{
int bar;

public:
foo() : bar(10) { }
const int& getBar() const { return (bar); }
};

int main(void)
{
foo *f = new foo();
const int &evil = f->getBar();

delete f;

/* now try to use evil... oops! */
}


While obvious here, when this sort of thing occurs in the real world (and it will, eventually), it can be a huge pain to track down and propertly correct.

Furthermore, by exposing a reference you are basically ensuring that the value returned is cached in some way, rather than calculated. Which means you have made a decision that reduces encapsulation, since you cannot easily change that aspect of the internal implementation (to compute the result of getBar() on the fly, for example, instead of caching it) without breaking your interface.

You should concern yourself with the semantics of your interface before the performance. It doesn't matter if your talking about vectors, matrices, strings, whatever -- just about anything can be made cheap to copy if copying actually becomes an issue. However, a broken interface is much harder to fix, and seemingly benign decisions that appear to "benign micro-optimizations" like this can turn out to be serious liabilities to the stability of your interface.

Share this post


Link to post
Share on other sites
Returning a const& is "the same as" returning a const pointer to a const object.

Do you understand the dangers of returning a const pointer from a class? A non-reference counted const pointer?

Even if the object pointed to is const, if the object pointed to is replaced, reading the const pointer to a const object will generate undefined and unpredictable behaviour.

The fact of the matter is, developer time can be turned into performance boosts. Bugs are turned into developer time. So by writing code that preemptively prevents bugs, you free up developer time, and thus free up developer time to boost performance.

Code to defend yourself against bugs and for flexibility (so you can fix design mistakes) and for speed of coding and for maintainability. Micro-optimizations that also open you up to memory corruption bugs and reduce abstraction don't seem all that tempting to me.

Share this post


Link to post
Share on other sites
Quote:

While obvious here, when this sort of thing occurs in the real world (and it will, eventually), it can be a huge pain to track down and propertly correct.


My point was that you shouldn't reference it like that, and any code doing that with these functions is wrong. But I see your point in that it's exposing the possibility of error.

Quote:

Furthermore, by exposing a reference you are basically ensuring that the value returned is cached in some way, rather than calculated. Which means you have made a decision that reduces encapsulation, since you cannot easily change that aspect of the internal implementation (to compute the result of getBar() on the fly, for example, instead of caching it) without breaking your interface.


Yeah, I already said this as one problem.

Quote:

You should concern yourself with the semantics of your interface before the performance. It doesn't matter if your talking about vectors, matrices, strings, whatever -- just about anything can be made cheap to copy if copying actually becomes an issue. However, a broken interface is much harder to fix, and seemingly benign decisions that appear to "benign micro-optimizations" like this can turn out to be serious liabilities to the stability of your interface.


I was trying to say that I had no reason to have a semantic of returning by value. I was rather indifferent about it, and so I'd rather choose the faster option. However, you've made your point, and I'm simply figuring out the good ways of coding. Obviously, in passing an std::string, you should pass by const reference for optimization. There's nothing wrong with coding efficiently, but semantics and robustness do come first. I admit that this may have been more dangerous than I initially thought. That was simply my question in this post!

Quote:

Returning a const& is "the same as" returning a const pointer to a const object.


Touche, you 1337 coder (you do realize your rating is 1337? :))

Share this post


Link to post
Share on other sites
Quote:
Original post by NotAYakk
Returning a const& is "the same as" returning a const pointer to a const object.

Do you understand the dangers of returning a const pointer from a class? A non-reference counted const pointer?

Even if the object pointed to is const, if the object pointed to is replaced, reading the const pointer to a const object will generate undefined and unpredictable behaviour.

The fact of the matter is, developer time can be turned into performance boosts. Bugs are turned into developer time. So by writing code that preemptively prevents bugs, you free up developer time, and thus free up developer time to boost performance.

Code to defend yourself against bugs and for flexibility (so you can fix design mistakes) and for speed of coding and for maintainability. Micro-optimizations that also open you up to memory corruption bugs and reduce abstraction don't seem all that tempting to me.


Returning a const pointer is no more dangerous than returning a const reference.


class foo
{
public:
const int& reference()const
{
return data;
}

const int* pointer()const
{
return &data;
};

private:
int data;
};
foo f;
assert( foo.reference() == *foo.pointer() );
assert( &foo.reference() == foo.pointer() );



Share this post


Link to post
Share on other sites
Quote:
Original post by Nitage

Returning a const pointer is no more dangerous than returning a const reference.

*** Source Snippet Removed ***


Exactly, he's saying they are equally dangerous.

Share this post


Link to post
Share on other sites
Quote:
Original post by okonomiyaki
Quote:
Original post by Nitage

Returning a const pointer is no more dangerous than returning a const reference.

*** Source Snippet Removed ***


Exactly, he's saying they are equally dangerous.


Ah, ok.

Share this post


Link to post
Share on other sites
Quote:
Original post by okonomiyaki
Anyway, I've decided to leave it up to the compiler except for certain circumstances where a ton of copying would occur. It annoys me when people tell others to not care about small coding optimizations, and to sloppily write an app and optimize the bottlenecks. Sure, this isn't going save a ton of time, but minor tweaks like this could make a small difference in the end which is good enough for me to care about. Why not make this small difference while I'm coding? I certainly would never go back and optimize an app this way, but while I'm coding I might as well do it as efficiently as possible. Of course, without doing anything stupid that would make it break easily, which in my opinion this is not one of those cases.


Dude, it annoys you when people who have shipped high performance AAA titles give you real-world advice about what to worry about? Really?

Nobody here has suggested that you write anything in a sloppy fashion, but silly little pseudo-optimizations like this are, as others here have mentioned, dangerous, dumb, and ultimately counterproductive. Focusing on getting your algorithms right is going to win you 100x more speedups than whether or not you copy around a few floats.

Share this post


Link to post
Share on other sites
It's ultimately up to you what you want to do. Returning const references is very commonly used in the game industry. It's not hard to fabricate a potential problem in doing so, but I have never seen such a problem actually happen. Even the worst coders I've known know better than to keep references around to such things as transforms, positions.

You'd actually have to go out of your way to screw up like that anyhow. You have to initialize const &, making them not often useful as class members, unless you plan to initialize it in the constructor initializer list to some placeholder static/global, and I sure as hell wouldn't make a global const ref. In my normal use, and nearly every game engine I've seen, they are strictly temporary access to things such as this, as well as commonly used function parameter modifiers.

There's little reason not to write code from the start like this. You can write bad code to demonstrate why you shouldn't, but it's just examples of bad code. I wouldn't force my whole game to return copies of transforms and positions(which are typically accessed alot), for the almost negligable chance that someone will hold a reference around longer than they should, or get a reference, modify the object, then use the reference(and whose to say they wouldnt want the const ref to reflect the changes anyhow?).

There's a fine line in premature optimization and just smart coding. Some might consider these things as premature optimization, whereas I for example consider them smart coding. I tend to write most accessor functions that are used to access a member variable of a class using the const ref method(non pod type members). Deliberately choosing a faster method up front is not premature optimziation in my book, regardless of whether it would have ended up being a hot spot in your profiling at the end. Using that logic one could argue against using efficient containers until one profiles and determines that the containers are indeed slow in a profile. That doesn't make any sense. Returning by const & isn't a micro optimization. It's a well known coding practice that has been recommended by many sources for many years. Just because the yields may not be as great on todays computers doesn't make it any less of a decent practice. Whether you choose to use it or not is up to you, and you should certainly be aware of what it means to do it either way.

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement