9
class Foo {
  std::vector<SomeType> data_;
};

Say Foo can only be constructed by make a copy (technically I mean a copy or move) of a std::vector<SomeType> object. What's the best way to write constructor(s) for Foo?

My first feeling is

Foo(std::vector<SomeType> data) noexcept : data_(std::move(data)) {};

Using it, construction of an instance takes 0 or 1 times of vector copy, depending on whether the argument for {data} is moveable or not.

updogliu
  • 6,066
  • 7
  • 37
  • 50

3 Answers3

17

Your first feeling is good. Strictly speaking it is not optimal. But it is so close to optimal that you would be justified in saying you don't care.

Explanation:

Foo(std::vector<SomeType> data) noexcept : data_(std::move(data)) {};

When the client passes in an lvalue std::vector<SomeType> 1 copy will be made to bind to the data argument. And then 1 move will be made to "copy" the argument into data_.

When the client passes in an xvalue std::vector<SomeType> 1 move will be made to bind to the data argument. And then another move will be made to "copy" the argument into data_.

When the client passes in a prvalue std::vector<SomeType> the move will be elided in binding to the data argument. And then 1 move will be made to "copy" the argument into data_.

Summary:

client argument    number of copies     number of moves
  lvalue                  1                   1
  xvalue                  0                   2
  prvalue                 0                   1

If you instead did:

Foo(const std::vector<SomeType>&  data)          : data_(data) {};
Foo(      std::vector<SomeType>&& data) noexcept : data_(std::move(data)) {};

Then you have a very slightly higher performance:

When the client passes in an lvalue std::vector<SomeType> 1 copy will be made to copy the argument into data_.

When the client passes in an xvalue std::vector<SomeType> 1 move will be made to "copy" the argument into data_.

When the client passes in a prvalue std::vector<SomeType> 1 move will be made to "copy" the argument into data_.

Summary:

client argument    number of copies     number of moves
  lvalue                  1                   0
  xvalue                  0                   1
  prvalue                 0                   1

Conclusion:

std::vector move constructions are very cheap, especially measured with respect to copies.

The first solution will cost you an extra move when the client passes in an lvalue. This is likely to be in the noise level, compared to the cost of the copy which must allocate memory.

The first solution will cost you an extra move when the client passes in an xvalue. This could be a weakness in the solution, as it doubles the cost. Performance testing is the only reliable way to assure that either this is, or is not an issue.

Both solutions are equivalent when the client passes a prvalue.


As the number of parameters in the constructor increases, the maintenance cost of the second solution increases exponentially. That is you need every combination of const lvalue and rvalue for each parameters. This is very manageable at 1 parameter (two constructors), less so at 2 parameters (4 constructors), and rapidly becomes unmanageable after that (8 constructors with 3 parameters). So optimal performance is not the only concern here.

If one has many parameters, and is concerned about the cost of an extra move construction for lvalue and xvalue arguments, there are other solutions, but they involve relatively ugly template meta-programming techniques which many consider too ugly to use (I don't, but I'm trying to be unbiased).

For std::vector, the cost of an extra move construction is typically small enough you won't be able to measure it in overall application performance.

Howard Hinnant
  • 206,506
  • 52
  • 449
  • 577
  • @dyp: Copy/paste error, fixed, thanks! Technically the move-version should not be `noexcept` either because the `vector` move constructor is not marked `noexcept` in the standard. However as a practical matter, I'm aware of only one vendor that does not mark the vector move constructor with `noexcept` as an extension. – Howard Hinnant Feb 23 '14 at 20:42
  • I'm rather confused about the exception safety guarantees of `vector`, but I would have thought that for the default allocator, the move constructor *may* not throw ..? – dyp Feb 23 '14 at 20:50
  • @dyp: I agree with you. However according to the standard `is_nothrow_move_constructible>::value` is false. However the standard also grants leeway to vendors to make `is_nothrow_move_constructible>::value` == true (i.e. by adding noexcept spec), and still be conforming. As far as I know, libc++ and libstdc++ do, vc++ does not. – Howard Hinnant Feb 23 '14 at 23:25
  • `Foo(std::vector&& data) noexcept : data_(std::move(data)) {};` Shouldn't it be forward instead of move? – Klaim Feb 24 '14 at 01:41
  • @Klaim: `std::forward>(data)` would work. But I think move is the better choice here. Either is going to always cast data to an rvalue (technically an xvalue). One could also use `data_(static_cast&&>(data))`, which again casts to xvalue. Out of the three, `move` is the most concise way to achieve that cast. It is the only one that does not require you to repeat the type of the vector. If we were in a context where sometimes the cast should be to lvalue, and sometimes to xvalue, then `forward` would become the better tool. – Howard Hinnant Feb 24 '14 at 14:58
  • @HowardHinnant in his CppCon 2014 talk Herb Sutter shows your quote with a different emphasis -- preferring the second solution basically. With the rationale that a copy assignment may be cheaper than a full copy, and that "noexcept" isn't strictly correct on the first approach... Should this answer be updated to reflect that? – Massimiliano Oct 25 '14 at 19:06
  • From the talk: Howard Hinnant: "Don’t blindly assume that the cost of construction is the same as assignment." For strings and vectors, "Capacity plays a large role in their performance. Copy construction always allocates (except for short). Copy assignment (except for short) allocates/deallocates 50% of the time with random capacities on the lhs and rhs. To keep an eye on performance, one must count allocations and deallocations." – Massimiliano Oct 25 '14 at 19:07
  • Actually, now I see that he mentions later that this approach is OK for constructors, but that's probably the only special case? Maybe worth mentioning that this is a special case, not to be used for other functions? – Massimiliano Oct 25 '14 at 19:21
  • 1
    @MaxGalkin: Exactly. Herb is talking about a setter, which is going to do an assignment from the argument. Here we are looking at a constructor, which must construct from the argument. An important lesson here: never lump construction and assignment into one performance argument using the words "copy" or "move". Always analyze "copy construction" or "copy assignment", not "copying". And your analysis may be different for different types. `vector` has different properties from `array`, and so you can get different answers. And if your type is generic, you have to be conservative. – Howard Hinnant Oct 25 '14 at 20:11
2

The complexity problem of the performance optimal solution mentioned in Howard Hinnant's answer for constructors taking multiple arguments would be solved by use of perfect forwarding:

template<typename A0>
Foo(A0 && a0) : data_(std::forward<A0>(a0)) {}

In case of more parameters, extend accordingly:

template<typename A0, typename A1, ...>
Foo(A0 && a0, A1 && a1, ...)
 : m0(std::forward<A0>(a0))
 , m1(std::forward<A1>(a1))
 , ...
 {}
  • 2
    I think one of the ugly parts Howard is mentioning relates to the constructibility of `Foo`: This ctor is "lying" when asked about `is_constructible`, as the instantiation itself will succeed for *any type* (and overload resolution), but the program might be ill-formed nevertheless. You'd need to add some SFINAE to disable construction when the construction of the data members is illegal. – dyp Feb 23 '14 at 20:29
  • 2
    Agreed with dyp. I recommend constraining with `is_constructible, A0>`. – Howard Hinnant Feb 23 '14 at 20:46
-5
class Foo {
  using data_t = std::vector<SomeType>;
  data_t data_;
public:
  constexpr Foo(data_t && d) noexcept : data_(std::forward<data_t>(d)) {}
};
OneOfOne
  • 95,033
  • 20
  • 184
  • 185