4

I need to store a sequence of elements of type ThirdPartyElm, and I'm using a std::vector (or a std::array if I need a fixed size sequence).

I'm wondering how I should initialise the sequence. The first version creates a new element and (if I'm right) creates a copy of the element when it is inserted in the sequence:

for (int i = 0; i < N; i++)
{
   auto elm = ThirdPartyElm();
   // init elm..
   my_vector.push_back(elm);  // my_array[i] = elm;
}

The second version stores a sequence of pointers (or better smart pointers with c++11):

for (int i = 0; i < N; i++)
{
   std::unique_ptr<ThirdPartyElm> elm(new ThirdPartyElm());
   // init elm..
   my_vector.push_back(std::move(elm));  // my_array[i] = std::move(elm);
}

Which is the most lightweight version?

Please highlight any errors.

Nick
  • 10,309
  • 21
  • 97
  • 201
  • 1
    Can you define "lightweight"? The second verison has that advantage that if you are willing to define a deleter function object `ThirdPartyElmDeleter` for ThirdPartyElm then you can achieve pimpl idiom by doing `std::vector >`. – tohava Dec 05 '14 at 17:50
  • Don't look at the new smart pointers as pointers, especially in terms of "lightweight" (the smart pointer way actually uses *more* memory, since you then need the space for the actual smart pointer object and its pointer), instead you should most of the time look a `std::unique_ptr` and `std::shared_ptr` from an *ownership* perspective. – Some programmer dude Dec 05 '14 at 17:51

4 Answers4

6

You can just declare it with the size, and it will call the default constructor on those elements.

std::vector<ThirdPartyElem> my_vector(N);

As far as your statement

The first version creates a new element and (if I'm right) creates a copy of the element when it is inserted in the sequence

Don't worry about that. Since ele is a local variable that is about to fall out of scope, your compiler will likely use copy elision such that a move will be invoked instead of a copy.

I was mistaken about the above, please disregard that.

Community
  • 1
  • 1
Cory Kramer
  • 114,268
  • 16
  • 167
  • 218
  • This skips out the `// init elm..` bit. – Chris Drew Dec 05 '14 at 18:03
  • 3
    In the case of the first example, no elision will be performed as the compiler doesn't know at that point that the local variable will fall out of scope. Also copy elision is an entirely different thing than treating a local variable going out of scope as an rvalue. – Felix Glas Dec 05 '14 at 18:03
  • @Snps So am I mistaken that the compiler will likely optimize the copy to a move? Or did I just use the wrong terminology? If the latter, what is the correct name of that type of optimization? – Cory Kramer Dec 05 '14 at 18:08
  • 2
    @Cyber Yes, to my knowledge you are mistaken. No optimization will be performed. The copy will never be implicitly turned into a move in this case, unless explicitly specified using `std::move`. An implicit cast to rvalue will only be performed at the `return` statement when returning a local variable in a function. – Felix Glas Dec 05 '14 at 18:12
  • copy elision (NRVO / RVO) is an explicit license to the compiler to violate the as-if rule under specific circumstances. If those don't exist, it must follow the as-if rule, which will probably mean it cannot be eliminated (at least until we have the sufficiently smart compiler). – Deduplicator Dec 05 '14 at 18:17
1

Storing pointers means you then have to clean those up after, or rely on smart pointers to do it for you, which adds unnecessary indirection and overhead.

As Cyber mentions, copy elision may prevent a copy, but you already explicitly avoid that by using std::move.

Since you mention C++11, I would suggest using emplace_back - push_back with std::move should have the same result (see answers to this question) but it's better practice to use emplace_back just on principle really; the other optimisation you can undertake, and the one most likely to have a major effect, is reserving the correct size in the vector at the start to ensure there are no unnecessary reallocations:

my_vector.reserve(N);
for (int i = 0; i < N; i++)
{
   auto elm = ThirdPartyElm();
   // init elm..
   my_vector.emplace_back(std::move(elm));
}

Edit: As per @Chris Drew's comment, this is not an effective optimisation if the type is not moveable. A more robust optimisation in that case, if construction is costly and copy-construction is to be avoided if possible, would be to emplace_back and then modify the newly emplaced element:

my_vector.reserve(N);
for (int i = 0; i < N; i++)
{
  my_vector.emplace_back(ThirdPartyElm());       
  my_vector.back().initialise();  // or whatever
}

There is slight additional overhead in accessing myvector.back() but this will be less costly than copy construction for non-trivial types.

Community
  • 1
  • 1
Riot
  • 15,723
  • 4
  • 60
  • 67
1

Avoid dynamic allocation whenever you can. Thus, generally prefer saving the elements themselves instead of smart-pointers to them in the vector.

That said, either is fine, and if ThirdPartyElem is polymorphic, you wouldn't have a choice.

Other considerations are the cost and possibility of moving and copying the type, though generally don't worry.

There are two refinements to option one which might be worthwhile though:

  1. std::move the new element to its place, as that is probably less expensive than copying (which might not even be possible).
    If the type is only copyable and not movable (legacy, ask for update), that falls back to copying.

  2. Try to construct it in-place, to eliminate copy or move, and needless destruction.

for (int i = 0; i < N; i++)
{
   my_vector.emplace_back();
   try {
       auto&& elm = my_vector.back();
       // init elm..
   } catch(...) {
       my_vector.pop_back();
       throw;
   }
}

If the initialization cannot throw, the compiler will remove the exception-handling (or you can just omit it).

Deduplicator
  • 44,692
  • 7
  • 66
  • 118
1

Focusing on a slightly different aspect from other answers, you are using push_back().

Instead of that if you know the size before entering the loop, please consider doing

my_vector.resize(N);

This way, you will be able to do the array style element insertion.

my_vector[i] = elem;

You may ask, what are the advantages:

  1. push_back() does a bounds check everytime, it wants to insert a new element.
  2. If you didn't do a reserve(), a push_back() may occasionally incur the resizing penalty.
  3. In the case of a large enough array, the resizings, may involve copying a lot of elements.
  4. Even if you did a reserve(N) or construct the vector(N), it must still do a bounds-check!

Of course, this approach is better if you are dealing with (smart or otherwise)pointers, as opposed to fat objects. The construction costs have to be weighed before taking this approach.

In my measurements, I have seen at least 1.2x performance improvement by going with resize() approach.

KalyanS
  • 527
  • 3
  • 8
  • Reserve and resize are not the same; resize allocates and initialises every element. This may be undesirable in cases where initialising one element needs to happen before the next one is constructed, which could be the case in the original post. Also it's worth mentioning that push_back is not as inefficient as you make it sound, as it reserves some capacity in estimation of future needs - it does not reallocate on every single push_back. – Riot Dec 05 '14 at 18:43
  • Agreed, it is worth mentioning that it is not resizing on every push_back. In my case, I was dealing with smart pointers, []= was measurably better than push_back(). Can share the numbers and code sample if you like. – KalyanS Dec 05 '14 at 18:46