8

Here is an exercise from C++ Primer 5th Edition:

Exercise 13.53: As a matter of low-level efficiency, the HasPtr assignment operator is not ideal. Explain why. Implement a copy-assignment and move-assignment operator for HasPtr and compare the operations executed in your new move-assignment operator versus the copy-and-swap version.(P.544)

File hasptr.h:

//! a class holding a std::string*
class HasPtr
{
    friend void swap(HasPtr&, HasPtr&);
    friend bool operator <(const HasPtr& lhs, const HasPtr& rhs);
public:
    //! default constructor.
    HasPtr(const std::string &s = std::string()):
        ps(new std::string(s)), i(0)
    { }

    //! copy constructor.
    HasPtr(const HasPtr& hp) :
        ps(new std::string(*hp.ps)), i(hp.i)
    { }

    //! move constructor.
    HasPtr(HasPtr&& hp) noexcept :
        ps(hp.ps), i(hp.i)
    { hp.ps = nullptr; }

    //! assignment operator
    HasPtr&
    operator = (HasPtr rhs);

    //! destructor.
    ~HasPtr()
    {
        delete ps;
    }

private:
    std::string *ps;
    int    i;
};

A part of the file hasptr.cpp:

//! specific swap.
inline void
swap(HasPtr &lhs, HasPtr &rhs)
{
    using std::swap;
    swap(lhs.ps, rhs.ps); // swap the pointers, not the string data
    swap(lhs.i, rhs.i);   // swap the int members

    std::cout <<"swapping!\n";
}

//! operator = using specific swap
HasPtr&
HasPtr::operator = (HasPtr rhs)
{
    swap(*this,rhs);
    return *this;
}

My question is why it is not efficient to do so?

Yue Wang
  • 1,710
  • 3
  • 18
  • 43
  • In order to answer the question, you first have to implement copy and move. Then use them. Then compare what operations occurred -- count how many move and copy constructors are called by your move and copy assignments, and the one above. – Yakk - Adam Nevraumont Jan 09 '14 at 02:23
  • @Yakk 'you first have to implement copy and move'--you mean copy and move assignments? – Yue Wang Jan 09 '14 at 02:29
  • Because it copies it? – Shoe Jan 09 '14 at 02:29
  • @Jefffrey I tried something like :`hp = std::move(hp2);` where `hp` and `hp2` are objects of `HasPtr`. It did call the move constructor. I think the direction of answering this question should be comparing how many times it call copy and move operations, but not quite sure. – Yue Wang Jan 09 '14 at 02:38
  • 1
    Another different question is why would you maintain a *pointer* to a `std::string`, and the answer is that in 99% of the cases you don't want to do that. But going back to the original question, what would be the effect of assigning an *lvalue* with a string of the same or smaller length? – David Rodríguez - dribeas Jan 09 '14 at 02:58
  • @DavidRodríguez-dribeas This class is just a demonstration used in this book for the c++ syntax. However, I'm still want to know why you thought so? Is it a bad idea to use a raw pointer to a `std::string`? – Yue Wang Jan 09 '14 at 03:05
  • 2
    @Alan.W: There is no reason (in most cases) to create `std::string` dynamically, and it comes at the cost of an additional memory allocation and the pain of having to manage the lifetime. Given that there is no advantage and some disadvantages, I would avoid doing it (I have never encountered that in production code, and I would not accept that in a code review) – David Rodríguez - dribeas Jan 09 '14 at 03:11

2 Answers2

12

Step 1

Set up a performance test which exercises the move assignment operator.

Set up another performance test which exercises the copy assignment operator.

Step 2

Set up the assignment operator both ways as instructed in the problem statement.

Step 3

Iterate on Steps 1 and 2 until you have confidence that you did them correctly.

Step 3 should help educate you as to what is going on, most likely by telling you where the performance is changing and where it is not changing.

Guessing is not an option for Steps 1-3. You actually have to do them. Otherwise you will (rightly) have no confidence that your guesses are correct.

Step 4

Now you can start guessing. Some people will call this "forming a hypothesis." Fancy way of saying "guessing." But at least now it is educated guessing.

I ran through this exercise while answering this question and noted no significant performance difference on one test, and a 6X performance difference on the other. This further led me to an hypothesis. After you do this work, if you are unsure of your hypothesis, update your question with your code, results, and subsequent questions.

Clarification

There are two special member assignment operators which typically have the signatures:

HasPtr& operator=(const HasPtr& rhs);  // copy assignment operator
HasPtr& operator=(HasPtr&& rhs);       // move assignment operator

It is possible to implement both move assignment and copy assignment with a single assignment operator with what is called the copy/swap idiom:

HasPtr& operator=(HasPtr rhs);

This single assignment operator can not be overloaded with the first set.

Is it better to implement two assignment operators (copy and move), or just one, using the copy/swap idiom? This is what Exercise 13.53 is asking. To answer, you must try both ways, and measure both copy assignment and move assignment. And smart, well meaning people get this wrong by guessing, instead of testing/measuring. You have picked a good exercise to study.

Howard Hinnant
  • 206,506
  • 52
  • 449
  • 577
  • Hi Thx for your suggestion. Just wondering what are you referring to in the sentence : _noted no significant performance difference on **one test**, and a 6X performance difference on **the other**._ – Yue Wang Jan 09 '14 at 03:22
  • 2
    There are two assignment operators to be examined here: The copy assignment operator, and the move assignment operator. An assignment operator taking the rhs by value can implement both with one signature. Deciding if that is a good idea or not is the point of this exercise. You must test both copy assignment and move assignment to fully address this exercise. – Howard Hinnant Jan 09 '14 at 04:00
  • Thx for the clarification. One more question, as a newbie who never used any performance measuring tool before, should I use any tool to measure the performance? Which tool fit this code? Any recommendation? – Yue Wang Jan 09 '14 at 04:38
  • 5
    I like to use the C++11 `` facilities. For this exercise I wrapped my test in `auto t0 = high_resolution_clock::now();` and `auto t1 = high_resolution_clock::now();`, and then printed out the elapsed time in milliseconds with: `std::cout << duration_cast(t1-t0).count() << "ms\n";`. The above assumes a `using namespace std::chrono;`. Without it, several of the identifiers I have listed won't be found. If you don't have `auto`, use `high_resolution_clock::time_point` in its place. – Howard Hinnant Jan 09 '14 at 04:44
  • 2
    I also placed a large number of copy/move assignments between `t0` and `t1` so that I got a non-zero number of milliseconds. Placing too few leads to quicker test runs, but more variability in the results. Placing too many leads to irritatingly long tests. It is an art form to get performance tests right. – Howard Hinnant Jan 09 '14 at 04:53
2

As the problem suggests, it's "a matter of low-level efficiency". When you use HasPtr& operator=(HasPtr rhs) and you write something like hp = std::move(hp2);, ps member is copied twice (the pointer itself not the object to which it points): Once from hp2 to rhs as a result of calling move constructor, and once from rhs to *this as a result of calling swap. But when you use HasPtr& operator=(HasPtr&& rhs), ps is copied just once from rhs to *this.

MrDetective
  • 65
  • 6