On Fri, Feb 1, 2013 at 8:37 AM, Antony Polukhin <antoshkka@gmail.com> wrote:
2013/1/31 Paul Smith <pl.smith.mail@gmail.com>:
On Thu, Jan 31, 2013 at 9:24 AM, Antony Polukhin <antoshkka@gmail.com> wrote:
From theoretical point of view you are absolutely correct, and my example is lame. Moreover, current implementation of move assignment and move constructors for recursive_wrapper were implemented to model that behavior.
Just pointing out that move assignment is not affected by this discussion. Everything is already allocated so it's as efficient as a pointer swap.
Can not agree (variant makes some additional manipulations to guarantee non emptyness):
I meant move-assignment of recursive_wrapper, not variant. It was never under discussion AFAICT. Sorry if it wasn't clear.
void move_assign(recursive_wrapper<T>&& rhs) { ... // heap allocation in move constructor of recursive_wrapper // can be optimized away (see #7960) variant temp( detail::variant::move(rhs) );
// *this stores type different from recursive_wrapper<T>, // so it another call to move constructor of recursive_wrapper // will be made // Potential call to less effective move_assign implementation if // variant does not have fallback_type (+1 heap allocation and deallocation) variant_assign( detail::variant::move(temp) ); ... // heap deallocation in destructor of recursive_wrapper }
With recursive_ptr this would be:
void move_assign(recursive_ptr<T>&& rhs) { ... // swap ptrs // can be optimized away (see #7960) variant temp( detail::variant::move(rhs) );
// recursive_ptr move constructor does not throw, so // variant will use a fastest possible move assignment implementation // *this stores type different from recursive_wrapper<T>, // so it calls to move constructor of recursive_ptr // => swap ptrs variant_assign( detail::variant::move(temp) );
... // destructors of temporaries will call delete on nullptrs
// Whole function will be noexcept => variant can be move // assigned in STL containers (containers will use copy-assignments, // if this function can throw) }
With recursive_ptr it is as efficient, as pointers swap! With recursive_wrapper it is multiple times slower.
IIUC you are talking about a scenario where the user assigns a recursive_wrapper to a variant directly (i.e. some_variant = some_recursive_wrapper_T). But this is ill-formed anyway - you can only assign an object of the recursive_wrapper's underlying type (i.e. some_variant = some_T). However, there are other scenarios which you can optimize if you're up for some extra work. When you need to back up a recursive_wrapper, there's no need to move-construct a temporary recursive_wrapper. Just copy and clear its pointer. Destroy the recursive_wrapper. Do the assignment. If it succeeds, delete the pointer. If it fails, reconstruct a recursive_wrapper using that pointer (have it take ownership over the existing object). And, ofcourse, Peter's optimization is equally viable for move-assignment as it is for move-construction.
2013/2/1 Dave Abrahams <dave@boostpro.com>:
on Thu Jan 31 2013, Paul Smith <pl.smith.mail-AT-gmail.com> wrote:
On Thu, Jan 31, 2013 at 9:24 AM, Antony Polukhin <antoshkka@gmail.com> wrote:
From theoretical point of view you are absolutely correct, and my example is lame. Moreover, current implementation of move assignment and move constructors for recursive_wrapper were implemented to model that behavior.
Just pointing out that move assignment is not affected by this discussion. Everything is already allocated so it's as efficient as a pointer swap.
Actually the correct semantics of move assignment is the same as "swap + clear" if there's an empty state. See http://cpp-next.com/archive/2009/09/your-next-assignment/
I recommend reading the whole article.
Oops, if move assignment shall be equal to "swap + clear", then the current implementations of recursive_wrappers move assign must be fixed (it just swaps).
Except that recursive_wrapper doesn't have "clear". The most sensible thing to do, if you want a truely "interference-free" move assignment, is to propagate it down to the underlying instance (i.e. lhs.get() = move(rhs.get()) ). Note that this can result in a copy. That's up to you. -- Paul Smith