20

Imagine you have a number of overloaded methods that (before C++11) looked like this:

class MyClass {
public:
   void f(const MyBigType& a, int id);
   void f(const MyBigType& a, string name);
   void f(const MyBigType& a, int b, int c, int d);
   // ...
};

This function makes a copy of a (MyBigType), so I want to add an optimization by providing a version of f that moves a instead of copying it.

My problem is that now the number of f overloads will duplicate:

class MyClass {
public:
   void f(const MyBigType& a, int id);
   void f(const MyBigType& a, string name);
   void f(const MyBigType& a, int b, int c, int d);
   // ...
   void f(MyBigType&& a, int id);
   void f(MyBigType&& a, string name);
   void f(MyBigType&& a, int b, int c, int d);
   // ...
};

If I had more parameters that could be moved, it would be unpractical to provide all the overloads.

Has anyone dealt with this issue? Is there a good solution/pattern to solve this problem?

Thanks!

alexc
  • 534
  • 2
  • 9
  • Could probably be automated by preprocessor macros, both declaration and definition. – Some programmer dude Jan 15 '15 at 18:53
  • 4
    Templates with universal references? – Iron Savior Jan 15 '15 at 18:53
  • @IronSavior that wouldn't allow implicit conversions – Ryan Haining Jan 15 '15 at 18:53
  • 1
    @RyanHaining Do you think it's wise to allow implicit conversions in the first place? – Iron Savior Jan 15 '15 at 18:54
  • @IronSavior in many cases yes. such as `const char *` to `std::string` – Ryan Haining Jan 15 '15 at 18:55
  • 2
    @RyanHaining I think that's probably not as good an example as you think it is. Implicit conversions easily lead to extra copying, which is counter to the question asker's intent. – Iron Savior Jan 15 '15 at 18:56
  • @IronSavior how about `std::pair` to `std::pair`? They'll construct a temporary and then move from it if there's an rvalue reference overload, which is fine – Ryan Haining Jan 15 '15 at 18:59
  • @RyanHaining Allowing implicit conversions in the case you brought up now is a design choice that I don't necessarily disagree with. However, it seems to be a better practice (in general, there may be exceptions) to prefer to deal strictly in explicit conversions with respect to user-defined classes. Doing so can help to minimize surprises later on. – Iron Savior Jan 15 '15 at 19:07
  • 1
    Why do you have so many overloads? – Lightness Races in Orbit Jan 15 '15 at 19:16
  • 2
    @RyanHaining: Implicit conversions are the devil's work – Lightness Races in Orbit Jan 15 '15 at 19:19
  • 4
    Is `MyBigType` cheap to move? – Howard Hinnant Jan 15 '15 at 19:26
  • 3
    What are you doing with the copy of `MyBigType`? Is it just used locally in `f()` or is it stored in `MyClass`? – Chris Drew Jan 15 '15 at 19:47
  • There is an ambiguity in the question. The following is ambiguous: "This function makes a copy of ...". I can think of (at least) two different possibilities as to what the questioner wants. First, a local (non-ref) variable in `f` might be constructed each call `MyBigType x = ??? a ???; `. A second option is an *existing* object might be assigned with the contents of `a`, as follows `this->x = ??? a ???;`. The question should clarify exactly what the "final" target is of the argument. Is the final object constructed, or assigned to, each time `f` is called? – Aaron McDaid Jan 16 '15 at 13:50

7 Answers7

14

Herb Sutter talks about something similar in a cppcon talk

This can be done but probably shouldn't. You can get the effect out using universal references and templates, but you want to constrain the type to MyBigType and things that are implicitly convertible to MyBigType. With some tmp tricks, you can do this:

class MyClass {
  public:
    template <typename T>
    typename std::enable_if<std::is_convertible<T, MyBigType>::value, void>::type
    f(T&& a, int id);
};

The only template parameter will match against the actual type of the parameter, the enable_if return type disallows incompatible types. I'll take it apart piece by piece

std::is_convertible<T, MyBigType>::value

This compile time expression will evaluate to true if T can be converted implicitly to a MyBigType. For example, if MyBigType were a std::string and T were a char* the expression would be true, but if T were an int it would be false.

typename std::enable_if<..., void>::type // where the ... is the above

this expression will result in void in the case that the is_convertible expression is true. When it's false, the expression will be malformed, so the template will be thrown out.

Inside the body of the function you'll need to use perfect forwarding, if you are planning on copy assigning or move assigning, the body would be something like

{
    this->a_ = std::forward<T>(a);
}

Here's a coliru live example with a using MyBigType = std::string. As Herb says, this function can't be virtual and must be implemented in the header. The error messages you get from calling with a wrong type will be pretty rough compared to the non-templated overloads.


Thanks to Barry's comment for this suggestion, to reduce repetition, it's probably a good idea to create a template alias for the SFINAE mechanism. If you declare in your class

template <typename T>
using EnableIfIsMyBigType = typename std::enable_if<std::is_convertible<T, MyBigType>::value, void>::type;

then you could reduce the declarations to

template <typename T>
EnableIfIsMyBigType<T>
f(T&& a, int id);

However, this assumes all of your overloads have a void return type. If the return type differs you could use a two-argument alias instead

template <typename T, typename R>
using EnableIfIsMyBigType = typename std::enable_if<std::is_convertible<T, MyBigType>::value,R>::type;

Then declare with the return type specified

template <typename T>
EnableIfIsMyBigType<T, void> // void is the return type
f(T&& a, int id);


The slightly slower option is to take the argument by value. If you do

class MyClass {
  public:
    void f(MyBigType a, int id) {
        this->a_ = std::move(a); // move assignment
    } 
};

In the case where f is passed an lvalue, it will copy construct a from its argument, then move assign it into this->a_. In the case that f is passed an rvalue, it will move construct a from the argument and then move assign. A live example of this behavior is here. Note that I use -fno-elide-constructors, without that flag, the rvalue cases elides the move construction and only the move assignment takes place.

If the object is expensive to move (std::array for example) this approach will be noticeably slower than the super-optimized first version. Also, consider watching this part of Herb's talk that Chris Drew links to in the comments to understand when it could be slower than using references. If you have a copy of Effective Modern C++ by Scott Meyers, he discusses the ups and downs in item 41.

Community
  • 1
  • 1
Ryan Haining
  • 35,360
  • 15
  • 114
  • 174
  • And this is the other correct answer (besides http://stackoverflow.com/a/27971091/576911). – Howard Hinnant Jan 15 '15 at 19:28
  • Note that since OP has lots of different things like `f`, he should probably provide an alias... `template using IsBigType = typename std::enable_if<...>::type` so that these can all be `template > void foo(T&& a, ...);` – Barry Jan 15 '15 at 19:44
  • @Barry good idea, but wouldn't that allow the caller to explicitly call with both template arguments and defaut the SFINAE? I'll add something – Ryan Haining Jan 15 '15 at 19:49
  • 1
    @RyanHaining True. Could have `IsBigType` be the return type of `foo` too. It just looks weirder there. – Barry Jan 15 '15 at 19:50
  • @Barry I agree, I originally had the `enable_if` as a defaulted template parameter too but changed it once I realized. – Ryan Haining Jan 15 '15 at 19:52
  • 1
    You should point out that the "slightly slower" option to take the argument by value can be much slower as [Herb explained](https://www.youtube.com/watch?v=xnqTKD8uD64&t=1h03m44s) in the talk you linked. – Chris Drew Jan 15 '15 at 20:18
9

You may do something like the following.

class MyClass {
public:
   void f(MyBigType a, int id) { this->a = std::move(a); /*...*/ }
   void f(MyBigType a, string name);
   void f(MyBigType a, int b, int c, int d);
   // ...
};

You just have an extra move (which may be optimized).

Jarod42
  • 203,559
  • 14
  • 181
  • 302
  • 1
    @mustafa no, it will just copy and then move from the copy. – Ryan Haining Jan 15 '15 at 19:11
  • 2
    @RyanHaining, it is even more problematic. The point is to optimize by moving instead of copying. Now you are forcing a copy for every function call. It defies the point. – Mustafa Jan 15 '15 at 19:12
  • 6
    I'm not sure why this was down voted. There are two correct answers to this question and this is definitely one of them. – Howard Hinnant Jan 15 '15 at 19:25
  • 6
    @mustafa actually I believe that this is fine, it will be slightly less optimized because it will be copy-then-move in the lvalue case and move-then-move in the rvalue case, but in either there won't be any more *copies* as a result. – Ryan Haining Jan 15 '15 at 19:31
  • 2
    `f()` is not a constructor, you can't use a constructor initialization list. – Chris Drew Jan 15 '15 at 20:01
  • 1
    Herb Sutter pointed out in his [Back to the Basics! Essentials of Modern C++ Style](https://www.youtube.com/watch?v=xnqTKD8uD64&t=1h03m44s) talk at CppCon that this option can be an optimization but can also be much slower. In the case of l-values it will not reuse any existing capacity. – Chris Drew Jan 15 '15 at 20:11
  • Compiler error: `error: only constructors take member initializers` – Mustafa Jan 15 '15 at 20:29
  • @ChrisDrew: We didn't use this code because it's an optimization, we used it because we're eliminating duplication. – Mooing Duck Jan 15 '15 at 21:40
  • 2
    @MooingDuck If you don't need to optimize, then just pass by const reference. No code duplication. – Chris Drew Jan 15 '15 at 21:42
  • @ChrisDrew, the question of "existing capacity" is interesting yes. I guess it depends on what the final target is of the data. Do we assign it to an object that existed before the call, or do we construct a new (local) variable for each call (in which case, there is no existing capacity)? The optimal answer depends on which of those two is in effect. – Aaron McDaid Jan 16 '15 at 13:55
8

My first thought is that you should change the parameters to pass by value. This covers the existing need to copy, except the copy happens at the call point rather than explicitly in the function. It also allows the parameters to be created by move construction in a move-able context (either unnamed temporaries or by using std::move).

Mark B
  • 95,107
  • 10
  • 109
  • 188
  • Unless we're both overlooking something this seems to be the best way. – Quentin Jan 15 '15 at 19:58
  • 1
    Herb Sutter pointed out in his [Back to the Basics! Essentials of Modern C++ Style](https://www.youtube.com/watch?v=xnqTKD8uD64&t=1h03m44s) talk at CppCon that this option can be an optimization but can also be much slower. In the case of l-values it will not reuse any existing capacity. – Chris Drew Jan 15 '15 at 20:23
  • 2
    Also note [the implications for exception safety guarantees](http://stackoverflow.com/questions/25667284/sink-arguments-and-move-semantics-for-functions-that-can-fail-strong-exception) that such a function is able to provide. Moving into such a function will destroy the original for sure. In some cases it is desirable though that the value is not being moved from if the function throws an exception. This is possible with an rvalue overload, but not with pass-by-value. – ComicSansMS Jan 16 '15 at 09:40
3

Why you would do that

These extra overloads only make sense, if modifying the function paramers in the implementation of the function really gives you a signigicant performance gain (or some kind of guarantee). This is hardly ever the case except for the case of constructors or assignment operators. Therefore, I would advise you to rethink, whether putting these overloads there is really necessary.

If the implementations are almost identical...

From my experience this modification is simply passing the parameter to another function wrapped in std::move() and the rest of the function is identical to the const & version. In that case you might turn your function into a template of this kind:

template <typename T> void f(T && a, int id);

Then in the function implementation you just replace the std::move(a) operation with std::forward<T>(a) and it should work. You can constrain the parameter type T with std::enable_if, if you like.

In the const ref case: Don't create a temporary, just to to modify it

If in the case of constant references you create a copy of your parameter and then continue the same way the move version works, then you may as well just pass the parameter by value and use the same implementation you used for the move version.

void f( MyBigData a, int id );

This will usually give you the same performance in both cases and you only need one overload and implementation. Lots of plusses!

Significantly different implementations

In case the two implementations differ significantly, there is no generic solution as far as I know. And I believe there can be none. This is also the only case, where doing this really makes sense, if profiling the performance shows you adequate improvements.

Ralph Tandetzky
  • 22,780
  • 11
  • 73
  • 120
1

You might introduce a mutable object:

#include <memory>
#include <type_traits>

// Mutable
// =======

template <typename T>
class Mutable
{
    public:
    Mutable(const T& value) : m_ptr(new(m_storage) T(value)) {}
    Mutable(T& value) : m_ptr(&value) {}
    Mutable(T&& value) : m_ptr(new(m_storage) T(std::move(value))) {}
    ~Mutable() {
        auto storage = reinterpret_cast<T*>(m_storage);
        if(m_ptr == storage)
            m_ptr->~T();
    }

    Mutable(const Mutable&) = delete;
    Mutable& operator = (const Mutable&) = delete;

    const T* operator -> () const { return m_ptr; }
    T* operator -> () { return m_ptr; }
    const T& operator * () const { return *m_ptr; }
    T& operator * () { return *m_ptr; }

    private:
    T* m_ptr;
    char m_storage[sizeof(T)];
 };


// Usage
// =====

#include <iostream>
struct X
{
    int value = 0;

    X() { std::cout << "default\n"; }
    X(const X&) { std::cout << "copy\n"; }
    X(X&&) { std::cout << "move\n"; }
    X& operator = (const X&) { std::cout << "assign copy\n"; return *this; }
    X& operator = (X&&) { std::cout << "assign move\n"; return *this; }
    ~X() { std::cout << "destruct " << value << "\n"; }
};

X make_x() { return X(); }

void fn(Mutable<X>&& x) {
    x->value = 1;
}

int main()
{
    const X x0;
    std::cout << "0:\n";
    fn(x0);
    std::cout << "1:\n";
    X x1;
    fn(x1);
    std::cout << "2:\n";
    fn(make_x());
    std::cout << "End\n";
}
1

This is the critical part of the question:

This function makes a copy of a (MyBigType),

Unfortunately, it is a little ambiguous. We would like to know what is the ultimate target of the data in the parameter. Is it:

  • 1) to be assigned to an object that existing before f was called?
  • 2) or instead, stored in a local variable:

i.e:

void f(??? a, int id) {
    this->x = ??? a ???;
    ...
}

or

void f(??? a, int id) {
    MyBigType a_copy = ??? a ???;
    ...
}

Sometimes, the first version (the assignment) can be done without any copies or moves. If this->x is already long string, and if a is short, then it can efficiently reuse the existing capacity. No copy-construction, and no moves. In short, sometimes assignment can be faster because we can skip the copy contruction.


Anyway, here goes:

template<typename T>
void f(T&& a, int id) {
   this->x = std::forward<T>(a);  // is assigning
   MyBigType local = std::forward<T>(a); // if move/copy constructing
}
Aaron McDaid
  • 26,501
  • 9
  • 66
  • 88
0

If the move version will provide any optimization then the implementation of the move overloaded function and the copy one must be really different. I don't see a way to get around this without providing implementations for both.

Mustafa
  • 1,814
  • 3
  • 17
  • 25