Should I std::move a shared_ptr in a move constructor?

Question

Consider:

#include <cstdlib>
#include <memory>
#include <string>
#include <vector>
#include <algorithm>
#include <iterator>
using namespace std;

class Gizmo
{
public:
    Gizmo() : foo_(shared_ptr<string>(new string("bar"))) {};
    Gizmo(Gizmo&& rhs); // Implemented Below

private:
    shared_ptr<string> foo_;
};

/*
// doesn't use std::move
Gizmo::Gizmo(Gizmo&& rhs)
:   foo_(rhs.foo_)
{
}
*/


// Does use std::move
Gizmo::Gizmo(Gizmo&& rhs)
:   foo_(std::move(rhs.foo_))
{
}

int main()
{
    typedef vector<Gizmo> Gizmos;
    Gizmos gizmos;
    generate_n(back_inserter(gizmos), 10000, []() -> Gizmo
    {
        Gizmo ret;
        return ret;
    });

    random_shuffle(gizmos.begin(), gizmos.end());

}

In the above code, there are two versions of Gizmo::Gizmo(Gizmo&&) -- one uses std::move to actually move the shared_ptr, and the other just copies the shared_ptr.

Both version seem to work on the surface. One difference (the only difference I can see) is in the non-move version the reference count of the shared_ptr is temporarily increased, but only briefly.

I would normally go ahead and move the shared_ptr, but only to be clear and consistent in my code. Am I missing a consideration here? Should I prefer one version over the other for any technical reason?

Answer 1

The main issue here is not the small performance difference due to the extra atomic increment and decrement in shared_ptr but that the semantics of the operation are inconsistent unless you perform a move.

While the assumption is that the reference count of the shared_ptr will only be temporary there is no such guarantee in the language. The source object from which you are moving can be a temporary, but it could also have a much longer lifetime. It could be a named variable that has been casted to an rvalue-reference (say std::move(var)), in which case by not moving from the shared_ptr you are still maintaining shared ownership with the source of the move, and if the destination shared_ptr has a smaller scope then the lifetime of the pointed object will needlessly be extended.

Answer 2

I upvoted James McNellis' answer. I would like to make a comment about his answer but my comment won't fit in the comment format. So I'm putting it here.

A fun way to measure the performance impact of moving a shared_ptr vs copying one is to use something like vector<shared_ptr<T>> to move or copy a whole bunch of them and time it. Most compilers have a way to turn on/off move semantics by specifying the language mode (e.g. -std=c++03 or -std=c++11).

Here is code I just tested at -O3:

#include <chrono>
#include <memory>
#include <vector>
#include <iostream>

int main()
{
    std::vector<std::shared_ptr<int> > v(10000, std::shared_ptr<int>(new int(3)));
    typedef std::chrono::high_resolution_clock Clock;
    typedef Clock::time_point time_point;
    typedef std::chrono::duration<double, std::micro> us;
    time_point t0 = Clock::now();
    v.erase(v.begin());
    time_point t1 = Clock::now();
    std::cout << us(t1-t0).count() << "\u00B5s\n";
}

Using clang/libc++ and in -std=c++03 this prints out for me:

195.368µs

Switching to -std=c++11 I get:

16.422µs

Your mileage may vary.

Answer 3

The use of move is preferable: it should be more efficient than a copy because it does not require the extra atomic increment and decrement of the reference count.