Search code examples
c++c++11optimizationmovemove-semantics

move shared_ptr on constructor initialization list


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?


Solution

  • 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);
    }