1

I'm performing some tests about move semantics, and my class behavior seems weird for me.

Given the mock class VecOfInt:

class VecOfInt {
public:
    VecOfInt(size_t num) : m_size(num), m_data(new int[m_size]) {}
    ~VecOfInt() { delete[] m_data; }
    VecOfInt(VecOfInt const& other) : m_size(other.m_size),  m_data(new int[m_size]) {
        std::cout << "copy..." <<std::endl;
        std::copy(other.m_data, other.m_data + m_size, m_data);
    }
    VecOfInt(VecOfInt&& other) : m_size(other.m_size) {
        std::cout << "move..." << std::endl;
        m_data = other.m_data;
        other.m_data = nullptr;
    }
    VecOfInt& operator=(VecOfInt const& other) {
        std::cout << "copy assignment..." << std::endl;
        m_size = other.m_size;
        delete m_data;
        m_data = nullptr;
        m_data = new int[m_size];
        m_data = other.m_data;
        return *this;
    }
    VecOfInt& operator=(VecOfInt&& other) {
        std::cout << "move assignment..." << std::endl;
        m_size = other.m_size;
        m_data = other.m_data;
        other.m_data = nullptr;
        return *this;
    }
private:
    size_t m_size;
    int* m_data;
};
  1. OK CASE

    When I insert a single value in-place:

    int main() {
        std::vector<VecOfInt> v;
        v.push_back(10);
        return 0;
    }
    

    Then it gives me the following output (what I think is fine):

    move...

  2. WEIRD CASE

    When I insert three different values in-place:

    int main() {
        std::vector<VecOfInt> v;
        v.push_back(10);
        v.push_back(20);
        v.push_back(30);
        return 0;
    }
    

    Then the output calls the copy constructor 3 times:

    move... move... copy... move... copy... copy...

What I'm missing here?

einpoklum
  • 118,144
  • 57
  • 340
  • 684
anc
  • 106
  • 4
  • 18
  • 2
    On a side note, `operator=(VecOfInt const&)` should check for self-assignment before making a copy, and it is not `delete`'ing `m_data` correctly. And `operator=(VecOfInt&&)` is leaking memory since the original array pointed to by `m_data` is lost. You can combine the two `operator=` into a single implementation by passing `other` by value and letting its constructors decide between copy and move operations. And also look into the [copy-swap idiom](https://stackoverflow.com/questions/3279543/what-is-the-copy-and-swap-idiom) to avoid leaks. – Remy Lebeau Feb 19 '20 at 21:52
  • 1
    In addition, `operator=` is changing the state of `*this` before new memory is allocated, thus if an exception is thrown, your object is corrupted. – PaulMcKenzie Feb 19 '20 at 21:54
  • 1
    You should change `VecOfInt` to use `std::vector` internally, then all these issues go away. – Remy Lebeau Feb 19 '20 at 21:57
  • Since the code lacks `v.reserve(3);` as the vector grows it has to copy the objects to the resized vector. – Eljay Feb 19 '20 at 22:01

5 Answers5

6

Move construction and move assignment aren't used by std::vector when reallocating unless they are noexcept or if there is no copying alternatives. Here is your example with noexcept added :

class VecOfInt {
public:
    VecOfInt(size_t num) : m_size(num), m_data(new int[m_size]) {}
    ~VecOfInt() { delete[] m_data; }
    VecOfInt(VecOfInt const& other) : m_size(other.m_size),  m_data(new int[m_size]) {
        std::cout << "copy..." <<std::endl;
        std::copy(other.m_data, other.m_data + m_size, m_data);
    }
    VecOfInt(VecOfInt&& other) noexcept : m_size(other.m_size) {
        std::cout << "move..." << std::endl;
        m_data = other.m_data;
        other.m_data = nullptr;
    }
    VecOfInt& operator=(VecOfInt const& other) {
        std::cout << "copy assignment..." << std::endl;
        m_size = other.m_size;
        delete m_data;
        m_data = nullptr;
        m_data = new int[m_size];
        m_data = other.m_data;
        return *this;
    }
    VecOfInt& operator=(VecOfInt&& other) noexcept {
        std::cout << "move assignment..." << std::endl;
        m_size = other.m_size;
        m_data = other.m_data;
        other.m_data = nullptr;
        return *this;
    }
private:
    size_t m_size;
    int* m_data;
};

A live live example outputs :

move...
move...
move...
move...
move...
move...

This is done to keep exception safety. When resizing a std::vector fails, it will try to leave the vector as it was before the attempt. But if a move operation throws half way through reallocation, there is no safe way to undo the moves that have already been made successfully. They very well may also throw. The safest solution is to copy if moving might throw.

François Andrieux
  • 28,148
  • 6
  • 56
  • 87
2

std::vector allocates a block of contiguous memory for its elements. When the allocated memory is too short for storing new elements, a new block is allocated and all current elements are copied from the old block to the new block.

You can use std::vector::reserve() to pre-size the capacity of the std::vector memory before adding new elements.

Try the following:

int main() {
    std::vector<VecOfInt> v;
    v.reserve(3);
    v.push_back(10);
    v.push_back(20);
    v.push_back(30);
    return 0;
}

and you will get:

move...
move...
move...

But to get move constructor being called even when reallocating you should make it noexcept like:

VecOfInt(VecOfInt&& other) noexcept {...}
NutCracker
  • 11,485
  • 4
  • 44
  • 68
  • 2
    This works around the problem but isn't the cause. `std::vector` should move internally when reallocating. – François Andrieux Feb 19 '20 at 21:55
  • @FrançoisAndrieux Adding the noexcept destructor as described in https://stackoverflow.com/questions/18655588/vector-reallocation-uses-copy-instead-of-move-constructor causes the reallocation to move instead of copy. Coliru demo: http://coliru.stacked-crooked.com/a/c79b65a067ec8b46 – LWolf Feb 19 '20 at 22:01
2

tl;dr: std::vector will copy instead of moving if your move constructor isn't noexcept.

1. It's not about the members and the dynamic allocations

The issue is not with what you do with the fields of foo. So your source could be just:

class foo {
public:
    foo(size_t num) {}
    ~foo() = default
    foo(foo const& other)  {
        std::cout << "copy..." <<std::endl;
    }
    foo(foo&& other) {
        std::cout << "move..." << std::endl;
    }
    foo& operator=(foo const& other) {
        std::cout << "copy assignment..." << std::endl;
        return *this;
    }
    foo& operator=(foo&& other) {
        std::cout << "move assignment..." << std::endl;
        return *this;
    }
};

and you still get the same behavior: try it.

2. The move's you do see are a distraction

Now, push_back() will first construct an element - foo in this case; then make sure there's space for it in the vector; then std::move() it into its place. So 3 of your moves are of that kind. Let's try using emplace_back() instead, which constructs the vector element in its place:

#include <vector>
#include <iostream>

struct foo { // same as above */ };

int main() {
    std::vector<foo> v;
    v.emplace_back(10);
    v.emplace_back(20);
    v.emplace_back(30);
    return 0;
}

This gives us:

copy 
copy 
copy 

try it. So the moves were just a distraction really.

3. The copies are due to the vector resizing itself

Your std::vector gradually grows as you insert elements - necessitating either moves or copy constructions. See @NutCracker's post for details.

4. The real issue is exceptions

See this question:

How to enforce move semantics when a vector grows?

std::vector doesn't know it can safely move elements when resizing - where "safely" means "without exceptions", so it falls back to copying.

5. "But my copy ctor can throw an exception too!"

I guess the rationale is that if you get an exception while copying the smaller buffer - you still haven't touched it, so at least your original, non-resized vector is valid and can be used. If you started moving elements and got an exception - then you have no valid copy of the element anyway, not to speak of a valid smaller vector.

einpoklum
  • 118,144
  • 57
  • 340
  • 684
  • Actually I don't need any of the ctors or operators, because the compiler already implements it for me. I was just performing some tests. – anc Feb 19 '20 at 22:08
  • @anc The compiler generated ones won't work out for you because of `m_data`. See the rule of 3/5/0. – François Andrieux Feb 19 '20 at 22:36
1

Your move constructor does not have the specifier noexcept.

Declare it like

VecOfInt(VecOfInt&& other) noexcept : m_size(other.m_size) {
    std::cout << "move..." << std::endl;
    m_data = other.m_data;
    other.m_data = nullptr;
}

Otherwise the class template std::vector will call the copy constructor.

Vlad from Moscow
  • 301,070
  • 26
  • 186
  • 335
0

Add noexcept decorators to your move constructor and move assignment operator:

VecOfInt(VecOfInt&& other) noexcept : m_size(other.m_size) {
    std::cout << "move..." << std::endl;
    m_data = other.m_data;
    other.m_data = nullptr;
}

VecOfInt& operator=(VecOfInt&& other) noexcept {
    std::cout << "move assignment..." << std::endl;
    m_size = other.m_size;
    m_data = other.m_data;
    other.m_data = nullptr;
    return *this;
}

Some functions (for instance, std::move_if_noexcept, used by std::vector) will decide to copy your object if its move operations are not decorated with noexcept., i.e. there is no guarantee they will not throw. That's why you should aim to make your move operations (move constructor, move assignment operator) noexcept. This can significantly improve the performance of your program.

According to Scott Meyer's in Effective Modern C++:

std::vector takes advantage of this "move if you can, but copy if you must" strategy , and it's not the only function in the Standard Library that does. Other functions sporting strong exception safety guarantee in C++98 (e.g. std::vector::reserve, std::deque::insert, etc) behave the same way. All these functions replace calls to copy operations in c++98 with calls to move operations in C++11 only if the move operations are known not to emit exceptions. But how can a function know if a move operation won't produce an exception? The answer is obvious: it checks to see if the operation is declared noexcept.

jignatius
  • 6,304
  • 2
  • 15
  • 30