Re: [boost] rvalue ref best practices?

Subject: Re: [boost] rvalue ref best practices?
From: Howard Hinnant (howard.hinnant_at_[hidden])
Date: 2012-06-09 18:03:17

On Jun 9, 2012, at 4:21 PM, Daniel Larimer wrote:

> I am trying to define my library API and have been using rvalue references and have observed some patterns that I would like to run by the community.
> 
> In c++03 I would pass almost everything by const& unless it was to be modified.  This results in extra copies any time a temporary is passed.
> 
> In c++11 I would pass anything that I plan on 'storing' or 'consuming' by &&.  This results in the most efficient code, but 'breaks' lvalue compatibility and forces me to consider having two (or more) versions of every method.  Unfortunately, you get a explosion of permutations of rvalue and const& as the number of arguments increases and it makes getting function pointers much more 'ugly'.  
> 
> Instead of providing two (or more) overloads, one for lvalues and one for rvalue, I have decided to only provide the rvalue overload and if the user wants to pass an lvalue they wrap with std::ref(), std::cref(), std::move() or my own helper copy() which makes the copy explicit and converts the lvalue to a rvalue.   (std::ref() and std::cref() could only work if the arguments are template parameters... in which case the method would probably use std::forward<>() to handle the auto-detection of move vs copy semantics. 
> 
> template<typename T>
> T copy( const T& v ) { return v; }
> 
> If I am writing a method that does not 'store' or 'consume' the input then I use const&
> 
> As rvalue references are quite new, this 'convention' may be unexpected by users.  How would you feel about using an API that assumed move semantics unless you explicitly specify copy or reference semantics?   Is there any reason that there shouldn't be a boost::copy() in boost/utility for use in this situation?
> 
> Are there some down sides to this approach that I am missing?  In my experience 90% of functions that take std::string() end up using a temporary and 90% of strings are 'calculated', 'used', and 'forgotten', yet most methods take const& and result in a temporary value.
> 
> I should probably clarify, that I would use const& for any types that do not benefit from move semantics (no deep copies).  But when you are writing template code, you do not want to make that assumption about the type.
> 
> Another practice I am considering is making certain classes 'explicit copy only'.
> 
> template<typename T>
> T copy( const T& t ) { return t; }
> 
> class test  {
>  public:
>    test(){...};
>    test( test&& t ){...}
> 
>  private:
>    template<typename T>
>    friend T copy(const T&t);
> 
>    test& operator=(const test&); // not implemented
>    test( const test& t ){...}
> 
>    std::vector<char> large_data;
> };
> 
> int main( int argc, char** argv ) {
>  test x;
>  test y = copy(x);
>  test z = x; // error, test(const test&) is private...
>  return 0;
> }
> 
> I would use this for any class that contains any 'movable members', such a strings, vectors, maps, etc.   Technically there is no reason why I couldn't copy them, but why shouldn't I attempt move semantics everywhere possible unless I really do want to copy?  With implicit copies there is no way for the compiler to warn me that I am doing something potentially expensive.
> 
> If it didn't break backward compatibility I would suggest that all of the standard containers adopt an explicit copy or move syntax or at least have a compiler flag to 'enable/disable' implicit copying for 'testing purposes'.    Perhaps we could put them in a different namespace for those of us that want to disable implicit copying of deep data types, ie:  xc::vector<>, xc::string, etc.  
> 
> Are these sane conventions?  What could go wrong?  I am I abusing rvalue references? 

Generic code that expects traditional copy syntax is going to fail to compile when used with your convention:

template<class T>
    T
    min(initializer_list<T> t);

template <class InputIterator1, class InputIterator2, class OutputIterator>
    OutputIterator
    merge(InputIterator1 first1, InputIterator1 last1,
          InputIterator2 first2, InputIterator2 last2, OutputIterator result);

template <class InputIterator, class RandomAccessIterator>
    RandomAccessIterator
    partial_sort_copy(InputIterator first, InputIterator last,
                      RandomAccessIterator result_first, RandomAccessIterator result_last);

template <class ForwardIterator, class OutputIterator>
    OutputIterator
    rotate_copy(ForwardIterator first, ForwardIterator middle, ForwardIterator last, OutputIterator result);

template <class InputIterator, class OutputIterator, class T>
    OutputIterator
    remove_copy(InputIterator first, InputIterator last, OutputIterator result, const T& value);

template <class ForwardIterator, class T>
    void
    fill(ForwardIterator first, ForwardIterator last, const T& value);

template <class OutputIterator, class Size, class T>
    OutputIterator
    fill_n(OutputIterator first, Size n, const T& value);

etc.

On Jun 9, 2012, at 4:49 PM, Giovanni Piero Deretta wrote:

> As per http://cpp-next.com/archive/2009/08/want-speed-pass-by-value/ ,
> if the function is likely to consume or copy the parameter, the best
> solution is to pass by value. This means you'll only need to provide a
> single overload.

"The best solution" has an implicit "always" in it.  Telling programmers they don't have to think, design or measure is a disservice to our industry.  Pass-by-value is a good tool to have in the toolbox.  It is not always the best solution.  There is no solution that is always best.

#include <string>
#include <vector>
#include <chrono>
#include <iostream>

class A
{
    std::string str_;
public:
#if 1
    explicit A(std::string str) : str_(std::move(str)) {}
#else
    explicit A(const std::string& str) : str_(str) {}
    explicit A(std::string&& str) : str_(std::move(str)) {}
#endif
};

static_assert(std::is_nothrow_move_constructible<A>::value, "");

int main()
{
    typedef std::chrono::high_resolution_clock Clock;
    typedef std::chrono::duration<double, std::milli> ms;
    std::vector<A> v;
    const unsigned N = 1000000;
    v.reserve(N);
    std::string s("1234567890");
    auto t0 = Clock::now();
    for (unsigned i = 0; i < N; ++i)
        v.push_back(A(s));
    auto t1 = Clock::now();
    std::cout << ms(t1-t0).count() << "ms\n";
}

For me this runs about 19% slower one way than the other.  And I have no idea how portable that result is.  Maybe for your application the 19% speed hit (more or less) is inconsequential.  Maybe for someone else's application a 19% speed hit (more or less) is critical.  <shrug>  We the advise givers have no idea.

Pass-by-value is a good tool in the toolbox.  It is not free.  There are other good tools in the toolbox too.  None of them are free either.

Howard