move shared_ptr on constructor initialization list

Question

Recently I saw few examples of code like this, where std::move was used on constructor initialization list (not move constructor).

class A {
public:
    A(std::shared_ptr<Res> res) : myRes(std::move(res)) {
    // ...
    }

private:
    std::shared_ptr<Res> myRes;
}

I got information that this construction was made for optimization reason. Personally I use std::move as rare as possible. I threat them as cast (as Scott Meyers said), and only in caller code (only exception is move constructor). For me it looks like some kind of obfuscation or micro optimization, but maybe I'm wrong. Does it's true, compiler does not produce faster code, without std::move?

I think the intent is to avoid an atomic increment of the reference counter(s). It is also a question of expressing your intent precisely and a rather common pattern (pass-by-value, then move the local parameter). — dyp, Sep 07 '14 at 20:42
possible duplicate of [Should I std::move a shared\_ptr in a move constructor?](http://stackoverflow.com/questions/10953325/should-i-stdmove-a-shared-ptr-in-a-move-constructor) — eerorika, Sep 07 '14 at 20:44
@user2079303 I'm not sure if it's a *full duplicate* of that particular question. The main focus there is on moving a data member in the move constructor; that's different from the problem in this question. The performance part is *partially* answered there, though. — dyp, Sep 07 '14 at 20:49
Fair point, I retracted the vote, but I'll leave the link since I think it's relevant regarding to the performance aspect. — eerorika, Sep 07 '14 at 20:53
Related: [Why do we copy then move?](http://stackoverflow.com/questions/16724657/why-do-we-copy-then-move) — David G, Sep 07 '14 at 20:59
possible duplicate of [Is std::move really needed on initialization list of constructor for heavy members passed by value?](http://stackoverflow.com/questions/23929800/is-stdmove-really-needed-on-initialization-list-of-constructor-for-heavy-membe) — PiotrNycz, Sep 07 '14 at 21:20
It looks to me like you really ought to be passing a `unique_ptr` rather than a `shared_ptr` in the first place. You're not sharing a pointer here, you're giving it. — screwnut, Sep 08 '14 at 18:13

score 6 · Accepted Answer · answered Sep 07 '14 at 21:43

I consider a missing std::move() where a non-trivial object can be moved but the compilers can't detect that this is the case to be an error in the code. That is, the std::move() in the constructor is mandatory: clearly, the temporary object the constructor is called with is about to go out of scope, i.e., it can be safely moved from. On the other hand, construction of member variables from an argument isn't one of the cases where the copy can be elided. That is, the compiler has to create a copy which certainly isn't very expensive for a std::shared_ptr<T> but it also isn't free. In particular, the reference counts updated need to be synchronized. Whether the difference can be measured is a different question. Running a simple benchmark (see below) seems to imply that there is indeed a performance improvement. Typically results I get are like this:

// clang:
copy: 440
move: 206
copy: 414
move: 209
// gcc:
copy: 361
move: 167
copy: 335
move: 170

Note, that in this context you are the called for the member's constructor! It is correct that std::move(res) is just a fancy way to write a cast (it is a replacement for static_cast<std::shared_ptr<RES>&&>(res)). However, it is crucial to be used in places where objects are about to go out of scope but are copied otherwise. Semantically, the use of std::move() is irrelevant in many cases (it is only semantically relevant when dealing with movable but non-copyable types). Avoiding unnecessary copies is an important performance improvement and std::move() helps doing so in the contexts where the compilers can't deduce that it is OK or isn't allowed to do so: the specific case is something the compiler could probably even detect on its own that a move would be safe but it isn't allowed to replace the copy by a move. It would be nice if compilers would warn about missing std::move() in these cases!

#include <algorithm>
#include <chrono>
#include <cstdlib>
#include <iostream>
#include <iterator>
#include <memory>
#include <ostream>
#include <vector>

class timer
{
    typedef std::chrono::high_resolution_clock clock;
    clock::time_point d_start;
public:
    timer(): d_start(clock::now()) {}
    std::ostream& print(std::ostream& out) const {
        using namespace std::chrono;
        return out << duration_cast<microseconds>(clock::now() - this->d_start).count();
    }
};

std::ostream& operator<< (std::ostream& out, timer const& t)
{
    return t.print(out);
}

struct ResCopy
{
    std::shared_ptr<unsigned int> d_sp;
    ResCopy(std::shared_ptr<unsigned int> sp): d_sp(sp) {}
    unsigned int value() const { return *this->d_sp; }
};

struct ResMove
{
    std::shared_ptr<unsigned int> d_sp;
    ResMove(std::shared_ptr<unsigned int> sp): d_sp(std::move(sp)) {}
    unsigned int value() const { return *this->d_sp; }
};

template <typename Res>
void measure(char const* name, std::vector<std::shared_ptr<unsigned int>> const& v)
{
    timer t;
    unsigned long value(0);
    for (int c(0); c != 100; ++c) {
        for (std::size_t i(0), end(v.size()); i != end; ++i) { 
            value += Res(v[i]).value();
        }
    }
    std::cout << name << ": " << t << '\n';
}

int main()
{
    std::vector<std::shared_ptr<unsigned int>> v;
    std::generate_n(std::back_inserter(v), 100,
                    []{ return std::shared_ptr<unsigned int>(new unsigned int(std::rand())); });

    measure<ResCopy>("copy", v);
    measure<ResMove>("move", v);
    measure<ResCopy>("copy", v);
    measure<ResMove>("move", v);
}

move shared_ptr on constructor initialization list

1 Answers1