Applying C++11 move semantics to bound functions

559 views Asked by At

I have some existing C++98 code that makes use of boost::function and boost:bind for asynchronous callbacks. Some relevant simplified fragments of the code include:

typedef boost::function<void (boost::system::error_code, size_t)> WriteHandler;

struct WriteOperation
{
    WriteOperation(const boost::shared_ptr<IDevice>& device,
                   const std::string& data, const WriteHandler& handler)
        : m_Device(device), m_Data(data), m_Handler(handler) {}

private:
    boost::shared_ptr<IDevice> m_Device;
    std::string m_Data;
    WriteHandler m_Handler;

    void Complete()
    {
        boost::system::error_code ec;
        size_t len;
        ...
        Async::Post(boost::bind(m_Handler, ec, len));
    }
};

struct Device : public IDevice
{
    void Write(const std::string& data, const WriteHandler& callback)
    {
        ...
        Async::Start(new WriteOperation(shared_from_this(), data,
            boost::bind(&Device::HandleWrite, this, handler, _1, _2)));
    }

 private:
    void HandleWrite(const WriteHandler& callback,
                     boost::system::error_code ec, size_t len)
    {
        ...
        callback(ec, len);
    }
};

There is one required copy, when constructing the WriteOperation, but other than that I'm trying to avoid copies since they can be quite expensive.

I'm pondering how this should best be written in the C++11 world. The obvious low-hanging fruit is that the WriteOperation constructor internally copies its arguments to its fields, so should use the automatic copying idiom:

WriteOperation(boost::shared_ptr<IDevice> device,
               std::string data, WriteHandler handler)
    : m_Device(std::move(device)), m_Data(std::move(data)), m_Handler(std::move(handler))
{}

(And of course the bare new should be replaced with unique_ptr, but that's a side issue.)

However I don't think this actually gains anything given the current implementation of Device::Write, so that ought to change too. My problem is that I don't really see a good way to do it. According to this advice, I have three options:

  1. Declare multiple overloads (one with const& and one with &&) -- but since this has two parameters, both of which could benefit from move semantics, this would require four overloads -- getting exponentially worse for methods with more parameters. Additionally this leads to either code duplication or scattering the code over additional methods, which can impair readability.

  2. Pass by value and move (similar to the WriteOperation constructor). This is perhaps the cleanest option when the body always makes a copy, which is true if the WriteOperation constructor is actually called, but what if the elided section contains logic that might return without constructing a WriteOperation? There's a wasted copy in this case.

  3. Template and perfect forward. This requires an ugly SFINAE hack that confuses Intellisense and impairs readability (or worse, leaving the parameter type unconstrained), and requires that the implementation be put into the header, which is sometimes undesirable. And it interferes with type conversions, eg. a SFINAE enable_if is_same looking for std::string will not accept a const char * literal, while the original const& version will.

Am I missing something? Is there a better solution? Or is this just a case where move semantics don't make any difference?


A related case:

typedef boost::function<void (boost::system::error_code, const std::string&)> ReadHandler;

void Read(const ReadHandler& callback)
{
    ... boost::bind(&Device::HandleRead, this, callback, _1, _2) ...
}

void HandleRead(const ReadHandler& callback,
                boost::system::error_code ec, const std::string& data)
{
    ...
    callback(ec, data);
}

This all seems like it should be ok, with no copying and no need for move semantics. And yet again I'm unsure about the ReadHandler passed to Read.

2

There are 2 answers

5
Yakk - Adam Nevraumont On

In rough order:

If copy is just as expensive as move, take it by const&.

If you reliably keep a copy, and move is cheap, take by value.

Failing that, you can stuff it in a header and are ok with sfinae or unconstrained templates, use forwarding references.

Failing that, if there are a limited number of arguments, write each overload. This is 2^n in the number of parameters, so there better not be too many. Forward internally to a forwarding reference based implementation.

Failing that, do you really need efficiency here?

Failing that, type erase to "creator of T".

template<class T, using Base=std::function<T()>>
struct creator_of: Base
{
  template<class U,
    std::enable_if_t<std::is_constructible<T, U&&>{},int> =0
  >
  creator_of(U&&u):
    Base([&]()->T{ return std::forward<U>(u); })
  {}
  template<class U,
    std::enable_if_t<std::is_constructible<T, std::result_of_t<std::decay_t<U>()>{},int> =0
  >
  creator_of(U&&u):
    Base(std::forward<U>(u))
  {}

  creator_of(creator_of&&)=default;
  creator_of():
    Base([]()->T{return {};}}
  {}
};

augment as needed. A creator_of<std::string> can be constructed from things that can construct a std::string, or from a function object returning a std::string.

Internally you can call () once on it.

(Code not compiled, but design is sound.)

8
Cheers and hth. - Alf On

To avoid the combinatorial explosion, while supporting the case where it turns out inside the called function that no argument copies are needed, you can defer the copying.

That's effectively a lazy evaluation scheme, but for this special case only.

With a support class like the one below one needs to be keenly aware that an instance might just hold a pointer to a short-lived caller's object. I.e., don't move a Lazy_copy_ into storage that outlives the call.

#include <iostream>
#include <new>
#include <string>           // std::string
#include <utility>          // std::move, std::forward
using namespace std;

//------------------------------------ Machinery:

#ifdef TRACE
    inline void trace( string const& s )
    {
        clog << ": " << s << endl;
    }
#else
    inline void trace( string const& ) {}
#endif

struct Emplace {};

template< class Item >
class Lazy_copy_
{
private:
    Item const* p_callers_item_;
    union{ Item item_copy_; };      // Left uninitialized if p_callers_item_.

    void ensure_is_copy()
    {
        if( p_callers_item_ )
        {
            ::new( &item_copy_ ) Item( *p_callers_item_ );
            trace( "ensure_is_copy: made copy" );
            p_callers_item_ = nullptr;
        }
    }

public:
    auto item() const
        -> Item const&
    { return (p_callers_item_? *p_callers_item_ : item_copy_); }

    auto item_copy()
        -> Item&
    {
        ensure_is_copy();
        return item_copy_;
    }

    ~Lazy_copy_()
    {
        if( not p_callers_item_ ) { item_copy_.Item::~Item(); }
    }

    Lazy_copy_( Lazy_copy_ const& other )
        : p_callers_item_( other.p_callers_item_ )
    {
        if( p_callers_item_ )
        {
            ensure_is_copy();
        }
        else
        {
            ::new( &item_copy_ ) Item( other.item_copy_ );
            trace( "<init>( Lazy_copy ): made copy" );
        }
    }

    Lazy_copy_( Lazy_copy_&& other )
        : p_callers_item_( other.p_callers_item_ )
    {
        if( not p_callers_item_ )
        {
            ::new( &item_copy_ ) Item( move( other.item_copy_ ) );
            trace( "<init>( Lazy_copy&& ): moved" );
        }
    }

    Lazy_copy_( Item const& item )
        : p_callers_item_( &item )
    {}

    Lazy_copy_( Item&& temp_item )
        : p_callers_item_( nullptr )
        , item_copy_( move( temp_item ) )
    {
        trace( "<init>( Item&& ): moved" );
    }

    template< class... Args >
    Lazy_copy_( Emplace, Args&&... args )
        : p_callers_item_( nullptr )
        , item_copy_( forward<Args>( args )... )
    {
        trace( "<init>( Emplace, Args... ): Created item from constructor args" );
    }
};

//------------------------------------ Example usage:

struct Thingy
{
    string a, b, c;

    void foo(
        Lazy_copy_<string>      arg_one,
        Lazy_copy_<string>      arg_two,
        Lazy_copy_<string>      arg_three
        )
    {
        if( arg_one.item() == "no_copy" )
        {
            return;     // The case of no copying needed.
        }
        a = move( arg_one.item_copy() );
        b = move( arg_two.item_copy() );
        c = move( arg_three.item_copy() );
    }
};

auto main()
    -> int
{
    Thingy  x;
    string a = "A", b = "B", c = "C";

    trace( "Call with copying:" );
    x.foo( string( "a" ), b, c );
    trace( "" );
    trace( "Call without copying: " );
    x.foo( string( "no_copy" ), b, c );
}

Output when built with TRACE defined:

: Call with copying:
: <init>( Item&& ): moved
: ensure_is_copy: made copy
: ensure_is_copy: made copy
:
: Call without copying:
: <init>( Item&& ): moved