5

Here is my problem. I have a function that takes struct as input, allocates a new struct and then returns it. The struct has the following contents

struct Data {
std::vector<custom_type> vector_vars;
std::string key;
std::map<std::string, Data*> index;
};

vector_vars is of size 500-1000. custom_type if relevant is this class. The index helps me access different Data struct by key. This is the function.

Data func(Data inputData) {

 Data outputData;
//compare values with inputData
//change some values in Data struct
 return outputData
}
  1. Do I use stack allocation and return it so that it is copied to RHS. Is this advisable since copying may create a lot of overhead?

  2. Can I return a reference/pointer to struct allocated on the stack within a function?

  3. Is it advisable to use dynamic allocation instead here (may be with std::shared_ptr) for efficiency?

trincot
  • 317,000
  • 35
  • 244
  • 286
user592748
  • 1,194
  • 3
  • 21
  • 45
  • Do you need `outputData` to be different to `inputData`? If so, allocate on the stack and return by value. – juanchopanza Mar 01 '13 at 16:07
  • 1
    2 is certainly invalid. You can measure how 1 and 3 perform for your specific data and decide for yourself, my bet is on return by value (the compiler will optimize out the copying). You probably should pass the function argument by reference though. – n. m. could be an AI Mar 01 '13 at 16:07
  • `1` is usually correct; all modern compilers perform RVO, and in C++11 even if RVO isn't possible the return value will be moved. – ecatmur Mar 01 '13 at 16:09

4 Answers4

14
  1. Return the object by value. Copy elision is an optimization that the standard allows compilers to make that removes unnecessary copies. In C++03, the copy will almost certainly be elided by the compiler. In C++11, the copy will first be considered as a move (which will in turn move its contents) and even that may be elided.

    in a return statement in a function with a class return type, when the expression is the name of a non-volatile automatic object (other than a function or catch-clause parameter) with the same cv-unqualified type as the function return type, the copy/move operation can be omitted by constructing the automatic object directly into the function’s return value

    Specifically, when this criteria results in copy elision, it is commonly known as named return value optimization, a form of return value optimization.

    So the most idiomatic and performance-friendly option here is to return by value. Of course, if you measure that even under optimization returning by value is a problem, then sure, you may consider dynamically allocating (see point 3).

  2. No, you should not return a reference or pointer to a local variable. The object with automatic storage duration (what you refer to as allocating on the stack) will not exist any more. The reference or pointer will be left dangling. Using it in any meaningful way will result in undefined behaviour.

  3. No, as mentioned above, it is advisable to return by value. However, if you really, really need to, you may dynamically allocate your Data and return it by smart pointer. The preferred smart pointer here would be a std::unique_ptr as your function is passing ownership to the calling function (not sharing ownership).

Joseph Mansfield
  • 108,238
  • 20
  • 242
  • 324
  • Thanks for the explaining in such detail. How should the `std::map` look? Should it be `std::map` or `std::map`? – user592748 Mar 01 '13 at 16:15
  • 1
    @user592748 It depends on whether the map should be referring to the same objects as everywhere else or if a copy is fine. That depends on the problem. – Joseph Mansfield Mar 01 '13 at 16:17
2
  1. Actually, one of two fast operations could happen here: either outputData could be moved into the return value, or this move could be elided. In neither case will there be any expensive copies. The copy of inputData seems unnecessary, perhaps you should pass it as Data const& inputData.
  2. You could do this, but it would result in undefined behaviour.
  3. No. Dynamic allocation is likely to be less safe, slower, more confusing and generally worse.
Mankarse
  • 39,818
  • 11
  • 97
  • 141
  • +1, but I think it is clearer to say that the copy could be elided *or* if not, it could still be moved. Elision trumps move. – juanchopanza Mar 01 '13 at 16:10
2
Data func1(Data inputData) {
    Data outputData;
    return outputData
}

takes the object of type Data by value and creates another instance of Data which is then returned by value. So the copy is created not only when the outputData is being returned, but also when inputData is being received. Don't worry about the performance while returning by value, there are mechanisms that will take care of it (worth to mention copy elision and possible return value optimization). I recommend you to pas inputData by reference though: Data func1(const Data& inputData).

Is this advisable since copying may create a lot of overhead?

Rule of thumb: Don't worry about the performance of your code unless you face performance issues.

Can I return a reference/pointer to struct allocated on the stack within a function?

Don't return a reference / pointer to the object with automatic storage duration defined within the scope of that function. The object is destructed once the execution goes out of scope, so you will end up with dangling pointer (invalid reference) which will lead to undefined behavior

Is it advisable to use dynamic allocation instead here for efficiency?

Avoid dynamic allocation wherever possible. Not because of efficiency, but for the sake of the memory management within your code. Follow RAII idiom and avoid handling the ugly memory management on your own.

LihO
  • 41,190
  • 11
  • 99
  • 167
1

You could rewrite func() to take return a Data* instead of a Data. Certainly it's much faster to copy a Data* than it is to copy a Data. But before that: do you care? Does this function run once during program initialization, taking a tenth of second of wall clock time, or do you expect to call it once a frame, or what?

When you're writing a program, or optimizing an existing one, your goal should always be to make it "fast enough" (and "responsive enough", but never mind that for now). The definition of "fast enough" varies a lot: in a simulation you might be happy to crunch one terabyte of data per hour, while in a video game you need to show the player a new graphics frame every 16 milliseconds.

So: is your program "fast enough" with just stack allocation? If it is, leave it alone. If it's not, use dynamic allocation instead. In general, it's good C++ style to return pointers instead of large structs, but you shouldn't worry about that while you're still learning. Make it work, make it right, then make it fast.

David Seiler
  • 9,557
  • 2
  • 31
  • 39