Re: [Boost-users] [move] case study: simple cloning smart pointer

[Just a few suggestions...] On Mon, Jul 16, 2012 at 4:54 AM, Krzysztof Czainski <1czajnik@gmail.com>wrote:

Hello,

In order to learn how to implement move semantics, I decided to write a simple cloning smart pointer (called ptr) with diagnostic information, and an example showing all constructor and assignment use cases I could think of. I wanted the pointer to be convertible (and movable) to pointers to base classes.

I've prepared two versions of conversion constructors and assignment operators in ptr. In the first version I tried to write everything like Boost.Move docs for implementing Copyable and Movable classes suggest, and then added the conversion constructors and assignment operators. Then I wrote a second version, using the pass-by-value and swap idiom.

Any comments about my attempts here are welcome. Did I get it right in both versions?

The attached file swap_idiom.zip contains: ptr.hpp - the deffinition of the ptr class template main.cpp - example for ptr<>, showing all constructor and assignment use cases I could think of 03.txt, 03_by_value.txt, 0x.txt, 0x_by_value.txt - the output I got for both versions of ptr, in two versions of C++: 03 and 0x. For this test I used MinGW-4.5.0.

Here's how the ptr<> class looks like (ptr.hpp) (note: {{{this}}} is how I mark code): {{{ extern int ptr_s; // helper for diagnostics template < class T > T* clone_me( T const* p );

template < class T > class ptr { public:

explicit ptr( T* p = 0 ) : p_(p) { ++ptr_s; std::cout << "_p "; }

// copy ctor ptr( ptr const& b ) : p_( clone_me(b.get()) ) { ++ptr_s; std::cout << "cp "; }

// move ctor ptr( BOOST_RV_REF(ptr) b ) : p_( b.release() ) { ++ptr_s; std::cout << "mp "; } }}}

So far so good.

Agreed.

This is where the hard part comes in - the conversion constructors and assignment operators. First version looks like this:

{{{ private:

#if !defined(BY_VALUE)

BOOST_COPYABLE_AND_MOVABLE(ptr)

public:

// generalized copy ctor for pointers to derived template < class U > ptr( ptr<U> const& b, typename boost::enable_if< boost::is_convertible

...

::type* = 0 ) : p_( clone_me(b.get()) ) { ++ptr_s; std::cout << "Cp "; }

// generalized move ctor for pointers to derived template < class U > ptr( BOOST_RV_REF(ptr<U>) b, typename boost::enable_if< boost::is_convertible

...

::type* = 0 ) : p_( b.release() ) { ++ptr_s; std::cout << "Mp "; }

I think you'll generally do better with the conversion copy constructor taking by-value and relying on RVO (in C++03). (Implicit) Rvalues of ptr<U> won't be moved into the newly constructed ptr<T> as written above. Note that the RV_REF overload is still useful even if you change the conversion copy constructor to taking by-value. Of course, in C++11, you can't have both by-value and rvalue reference overloads :( ptr& operator=( BOOST_COPY_ASSIGN_REF(ptr) b )

{ T* tmp = clone_me(b.get()); // this can throw boost::checked_delete(p_); p_ = tmp; return *this; }

ptr& operator=( BOOST_RV_REF(ptr) b ) { boost::checked_delete(p_); p_ = b.release(); return *this; }

template < class U > typename boost::enable_if< boost::is_convertible, ptr& >::type operator=( ptr<U> const& b ) { T* tmp = clone_me(b.get()); // this could throw boost::checked_delete(p_); p_ = tmp; return *this; }

Probably want to disable this overload when U == T. And, again, by-value is probably what you want in C++03. template < class U >

typename boost::enable_if< boost::is_convertible, ptr& >::type operator=( BOOST_RV_REF(ptr<U>) b ) { boost::checked_delete(p_); p_ = b.release(); return *this; } }}} This was long, and contains some tricky places, like the one commented 'this could throw'. Did I even get it right? I think so, but I'm just beginning to learn about move semantics here ;-)

I admit, I didn't really look at the bodies, so no comments regarding correctness there :)

Anyway, next comes the second version: implementing conversion constructors and assignment operators in terms of pass-by-value and swap. Note the use of BOOST_COPYABLE_AND_MOVABLE_ALT macro so that an operator=(ptr&) isn't inserted. {{{ #else // BY_VALUE

BOOST_COPYABLE_AND_MOVABLE_ALT(ptr)

public:

// generalized copy/move constructor implemented by pass-by-value & steal template < class U > ptr( ptr<U> b, typename boost::enable_if< boost::is_convertible

...

::type* = 0 ) : p_( b.release() ) { ++ptr_s; std::cout << "Vp "; }

// assignment - works for all types convertible to ptr ptr& operator=( ptr b ) { swap(*this,b); return *this; }

#endif // BY_VALUE }}} Compared to the first version, this is really simple. I see no tricky parts here.

The rest of ptr<> follows: {{{ ~ptr() { boost::checked_delete(p_); ++ptr_s; std::cout << "~p "; }

T* get() const { return p_; }

T* release() { T* r = p_; p_ = 0; return r; }

friend void swap( ptr& a, ptr& b ) { boost::swap(a.p_,b.p_); std::cout << "sp "; }

// reset, operator* and ->

private:

T* p_; }; }}}

Writing the conversion constructors and assignment operators in the second version was much simpler. While version 1 contains 6 functions, version 2 contains only 2 functions. But is this always correct, and what is the cost of simplifying things? I tried to answer that question by writing the example (main.cpp). I started by preparing two diagnostic classes: {{{ struct A { static int a, c; A() { ++a; ++c; cout << "_A "; } A( A const& ) { ++a; ++c; cout << "cA "; } virtual ~A() { ++a; --c; cout << "~A "; } }; int A::a = 0; int A::c = 0; struct B : A { static int b, d; B() { ++b; ++d; cout << "_B "; } B( B const& x ) : A(x) { ++b; ++d; cout << "cB "; } ~B() { ++b; --d; cout << "~B "; } }; int B::b = 0; int B::d = 0;

ptr<A> make_a() { return ptr<A>( new A ); }

ptr<B> make_b() { return ptr<B>( new B ); } }}}

Then some machinery for printing and zeroing the static variables incremented/decremented by corresponding constructors/destructors.

{{{ void trace( char const* msg ) { cout << endl << boost::format("%34s P:%d A:%d B:%d eA:%d eB:%d") % msg % ptr_s % A::a % B::b % A::c % B::d << endl; ptr_s = 0; A::a = 0; B::b = 0; }

#define TRACE( x ) x; trace( #x ); }}}

And now main() itself, containing all use cases of ptr<> I could think of: {{{ int main() { { TRACE( ptr<A> z ) TRACE( ptr<A> a( new A ) ) TRACE( ptr<B> b( new B ) ) TRACE( ptr<A> c( new B ) ) TRACE( ptr<A> d( a ) ) TRACE( ptr<A> e( b ) ) TRACE( ptr<A> f( boost::move(a) ) ) TRACE( ptr<A> g( boost::move(b) ) ) TRACE( ptr<B> h( new B ) ) TRACE( ptr<A> i( boost::move(c) ) ) TRACE( ptr<A> j( i ) ) // slice

TRACE( ptr<A> x( new A ) ) TRACE( x = f ) TRACE( x = h ) TRACE( x = boost::move(f) ) TRACE( x = boost::move(g) ) TRACE( x = boost::move(h) ) TRACE( x = i ) // slice } trace( "" ); { TRACE( ptr<A> a = make_a() ) TRACE( ptr<A> b = make_b() ) TRACE( ptr<A> c( make_b() ) ) TRACE( a = make_a() ) TRACE( b = make_b() ) } trace( "" ); } }}}

And now for the costs. I compiled the attached code with MinGW-4.5.0 in 4 configurations: -std=c++0x disabled/enabled, and version 1/2. I compared the program output to derive the conclusions:

C++03: version 1 vs. version 2: - whenever a conversion is needed (about half of the use cases above), version 2 introduces an additional temporary ptr<> object (without a deep copy); in one case it's 2 additional temporary ptr<>s; - the last 4 use cases {{{ ptr<A> b = make_b(); ptr<A> c( make_b() ); a = make_a(); b = make_b(); }}} introduce a deep copy in version 1, while while the deep copy is avoided in version 2.

I'm pretty sure these extraneous deep copies would be avoided if you make the changes I suggested above.

C++0x: version 1. vs. version 2: - whenever a conversion is needed (about half of the use cases above), version 2 introduces an additional temporary ptr<> object (without a deep copy); in one case it's 2 additional temporary ptr<>s; - no unnecessary deep copies are introduced in either version.

If you want maximum performance in either C++03 or C++11, you're probably going to have to (a) target the overload set specifically to the presence/absence of rvalue references; and (b) in C++03, use both by-value overloads as well as explicit RV_REF overloads, or use a type-erasure trick to capture rvalues of move-emulation-enabled types. Yes, it can be a royal pain; macros to automatically generate some commonly recurring overload sets can help alleviate these issues. Version 1: C++03 vs. C++0x:

- the last 4 use cases {{{ ptr<A> b = make_b(); ptr<A> c( make_b() ); a = make_a(); b = make_b(); }}} introduce a deep copy in version C++03, while while the deep copy is avoided in C++0x.

Version 2: C++03 vs. C++0x: - no difference.

General conclusion: 'pass-by-value and swap' idiom is cool ;-) Pros: - better move emulation, - simple implementation. Cons: - additional temporary objects, that are then swapped to the right place.

Thanks for staying with me ;-)

Hope that provides with additional insight, - Jeff