CODEWITHSUNDEEP
c++c++11stdvectorcopy-assignment
Narek
Narek

Reputation: 39881

Is copy assignment operator with copy and swap idiom and self assignment check recommended?

Here you can see copy assignment operator implementation with self assignment check:

String & operator=(const String & s)
{
    if (this != &s)
    {
        String(s).swap(*this); //Copy-constructor and non-throwing swap
    }

    // Old resources are released with the destruction of the temporary above
    return *this;
}

This is good for self assignment, but bad for performance:

  1. as each time it check as if statement (I don't know how much will be it optimal, considering the branch prediction)
  2. Also we lose here copy elision for rvalue arguments

So I still don't understand if I would implement std::vector's operator= how I would implement it.

Upvotes: 6

Views: 526

Answers (2)

SergeyA
SergeyA

Reputation: 62553

Yes, this code is superflous. And it is true that it is causing extra unncessary branch. With proper swap and move semantics, following should be much more performant:

String& String::operator=(String s) { // note passing by value!

    std::swap(s, *this); // expected to juggle couple of pointers, will do nothing for self-assingment
    return *this;
}

Also note, it is more benefical to accept the argument by value.

Upvotes: 5

uh oh somebody needs a pupper
uh oh somebody needs a pupper

Reputation: 3396

as each time it check as if statement (I don't know how much will be it optimal, considering the branch prediction)

I think you've got yourself in a premature optimization circle here.

Check for self assignment -> self-assignment is unnecessary if you wrote the code properly -> why not write swap explicitly if you mean it? -> we're back to square one

Realistically, I would just implement Allocator and not worry about it.

Also we lose here copy elision for rvalue arguments

I don't think so.

#include <iostream>

#define loud(x) std::cout << x << "\n";

struct foo
{
    foo() { loud("default") }
    ~foo() { loud("destruct") }

    foo(const foo&) { loud("copy") }
    foo(foo&&) { loud("move") }

    foo & operator=(const foo & s)
    {
        if (this != &s)
        {
            loud("copy assign")
        }

        return *this;
    }
};

int main()
{
    foo f;
    foo g;
    g = f;
}

Outputs:

default
default
copy assign
destruct
destruct

This is with -fno-elide-constructors.

You claim the branch may be a problem, but the assembly output for -O2 shows me that GCC doesn't even emit code for the operator= and just outputs the "copy assign" string directly. Yes, I realized I have a simplified example, but it really is the wrong end of the pile to start from.

Upvotes: 1

Related Questions