Search code examples
c++c++11movemove-assignment-operator

Questions about the move assignment operator


Imagine the following class that manages a resource (my question is only about the move assignment operator):

struct A
{
    std::size_t s;
    int* p;
    A(std::size_t s) : s(s), p(new int[s]){}
    ~A(){delete [] p;}
    A(A const& other) : s(other.s), p(new int[other.s])
    {std::copy(other.p, other.p + s, this->p);}
    A(A&& other) : s(other.s), p(other.p)
    {other.s = 0; other.p = nullptr;}
    A& operator=(A const& other)
    {A temp = other; std::swap(*this, temp); return *this;}
    // Move assignment operator #1
    A& operator=(A&& other)
    {
        std::swap(this->s, other.s);
        std::swap(this->p, other.p);
        return *this;
    }
    // Move assignment operator #2
    A& operator=(A&& other)
    {
        delete [] p;
        s = other.s;
        p = other.p;
        other.s = 0;
        other.p = nullptr;
        return *this;
     } 
};

Question:

What are the advantages and disadvantages of the two move assignment operators #1 and #2 above? I believe the only difference I can see is that std::swap preserves the storage of the lhs, however, I don't see how that would be useful as rvalues would be destroyed anyways. Maybe the only time would be with something like a1 = std::move(a2);, but even in this case I don't see any reason to use #1.


Solution

  • This is a case where you should really measure.

    And I'm looking at the OP's copy assignment operator and seeing inefficiency:

    A& operator=(A const& other)
        {A temp = other; std::swap(*this, temp); return *this;}
    

    What if *this and other have the same s?

    It seems to me that a smarter copy assignment could avoid making a trip to the heap if s == other.s. All it would have to do is the copy:

    A& operator=(A const& other)
    {
        if (this != &other)
        {
            if (s != other.s)
            {
                delete [] p;
                p = nullptr;
                s = 0;
                p = new int[other.s];
                s = other.s;
            }
            std::copy(other.p, other.p + s, this->p);
        }
        return *this;
    }
    

    If you don't need strong exception safety, only basic exception safety on copy assignment (like std::string, std::vector, etc.), then there is a potential performance improvement with the above. How much? Measure.

    I've coded this class three ways:

    Design 1:

    Use the above copy assignment operator and the OP's move assignment operator #1.

    Design 2:

    Use the above copy assignment operator and the OP's move assignment operator #2.

    Design 3:

    DeadMG's copy assignment operator for both copy and move assignment.

    Here is the code I used to test:

    #include <cstddef>
    #include <algorithm>
    #include <chrono>
    #include <iostream>
    
    struct A
    {
        std::size_t s;
        int* p;
        A(std::size_t s) : s(s), p(new int[s]){}
        ~A(){delete [] p;}
        A(A const& other) : s(other.s), p(new int[other.s])
        {std::copy(other.p, other.p + s, this->p);}
        A(A&& other) : s(other.s), p(other.p)
        {other.s = 0; other.p = nullptr;}
        void swap(A& other)
        {std::swap(s, other.s); std::swap(p, other.p);}
    #if DESIGN != 3
        A& operator=(A const& other)
        {
            if (this != &other)
            {
                if (s != other.s)
                {
                    delete [] p;
                    p = nullptr;
                    s = 0;
                    p = new int[other.s];
                    s = other.s;
                }
                std::copy(other.p, other.p + s, this->p);
            }
            return *this;
        }
    #endif
    #if DESIGN == 1
        // Move assignment operator #1
        A& operator=(A&& other)
        {
            swap(other);
            return *this;
        }
    #elif DESIGN == 2
        // Move assignment operator #2
        A& operator=(A&& other)
        {
            delete [] p;
            s = other.s;
            p = other.p;
            other.s = 0;
            other.p = nullptr;
            return *this;
         } 
    #elif DESIGN == 3
        A& operator=(A other)
        {
            swap(other);
            return *this;
        }
    #endif
    };
    
    int main()
    {
        typedef std::chrono::high_resolution_clock Clock;
        typedef std::chrono::duration<float, std::nano> NS;
        A a1(10);
        A a2(10);
        auto t0 = Clock::now();
        a2 = a1;
        auto t1 = Clock::now();
        std::cout << "copy takes " << NS(t1-t0).count() << "ns\n";
        t0 = Clock::now();
        a2 = std::move(a1);
        t1 = Clock::now();
        std::cout << "move takes " << NS(t1-t0).count() << "ns\n";
    }
    

    Here is the output I got:

    $ clang++ -std=c++11 -stdlib=libc++ -O3 -DDESIGN=1  test.cpp 
    $ a.out
    copy takes 55ns
    move takes 44ns
    $ a.out
    copy takes 56ns
    move takes 24ns
    $ a.out
    copy takes 53ns
    move takes 25ns
    $ clang++ -std=c++11 -stdlib=libc++ -O3 -DDESIGN=2  test.cpp 
    $ a.out
    copy takes 74ns
    move takes 538ns
    $ a.out
    copy takes 59ns
    move takes 491ns
    $ a.out
    copy takes 61ns
    move takes 510ns
    $ clang++ -std=c++11 -stdlib=libc++ -O3 -DDESIGN=3  test.cpp 
    $ a.out
    copy takes 666ns
    move takes 304ns
    $ a.out
    copy takes 603ns
    move takes 446ns
    $ a.out
    copy takes 619ns
    move takes 317ns
    

    DESIGN 1 looks pretty good to me.

    Caveat: If the class has resources that need to be deallocated "quickly", such as mutex lock ownership or file open-state ownership, the design-2 move assignment operator could be better from a correctness point of view. But when the resource is simply memory, it is often advantageous to delay deallocating it as long as possible (as in the OP's use case).

    Caveat 2: If you have other use cases you know to be important, measure them. You might come to different conclusions than I have here.

    Note: I value performance over "DRY". All of the code here is going to be encapsulated within one class (struct A). Make struct A as good as you can. And if you do a sufficiently high quality job, then your clients of struct A (which may be yourself) won't be tempted to "RIA" (Reinvent It Again). I much prefer to repeat a little code within one class, rather than repeat the implementation of entire classes over and over again.