2

I've got some light objects to push around and manipulate, which I then like to include in a more complex one. There's a lookup table which should remain unmodified. The idea appears simple enough, but the one line doing this - b += c(a); - creates an expensive temporary.

#include <vector>
static int count;

struct costly {
    /* std::map<std::string, int> and whatnot */
    int b;

    ~costly() { count++; }
     costly(int b): b(b) { }
     costly &operator+= (costly &rhs) { b += rhs.b; return *this; }
};

/* Note the assumption above constructor exists for rhs */
costly operator* (const costly &lhs, costly rhs) {
    rhs.b *= lhs.b; return rhs;
}

struct cheap {
    /* Consider these private or generally unaccessible to 'costly' */
    int index, mul;

    cheap(int index, int mul): index(index), mul(mul) { }
    costly operator() (const std::vector<costly> &rhs) {
        /* Can we do without this? */
        costly tmp = rhs[index] * mul; return tmp;
    }
};

int main(int argc, char* argv[]) {
    std::vector<costly> a = {1, 2}; costly b(1); cheap c = {1, 2};
    /* Above init also calls the destructor, don't care for now */
    count = 0;
    b += c(a);
    return count;
}

I've been reading up on RVO and C++11's rvalues, but can't really wrap my head around them well enough yet to completely eliminate the introduced intermediate. And above only creates one because rhs's constructor is available. Initially I had this;

costly operator* (costly lhs, int rhs) {
    lhs.b *= rhs; return lhs;
}

/* ... */

costly operator() (const std::vector<costly> &rhs) {
    return rhs[index] * mul;
}

But that, counter to my intuition, resulted in count even being 2. Why's the compiler not getting my intentions?

Meeuwisse
  • 157
  • 6
  • How about changing your lookup table to a `shared_ptr>`, and making a copy of it (and replacing it) only when it needs to change? That will make the copies cheap. – nishantjr Nov 16 '14 at 14:16

3 Answers3

1

RVO does not apply to function parameters, so your * operators are inhibiting it. In order to enable RVO, you need a local copy of the parameter. You can then optimize by providing an overload taking an rvalue reference (provided costly has an efficient move copy constrctor). For example,

costly operator*(const costly& c, int i)
{
  costly ret = c;
  ret += 1;
  return ret;
}

costly operator*(costly&& c, int i)
{
  costly ret = std::move(c);
  ret += 1;
  return ret;
}
juanchopanza
  • 223,364
  • 34
  • 402
  • 480
1

This is a completely different approach, that's orthogonal to whether you optimize via RVO, but here goes:

Since your the inner data-member that makes the copy expensive is more or less const, why not just avoid copying that particular member?

If you change costly like so:

struct costly {
    shared_ptr<const map<string, int>> lookup_table;
    int m;
    ...
};

Copying becomes much cheaper. Note the pointer to the table is non-const even though the map it points to is const.

Sean Parent gave a really good talk about this, regarding how they implemented history and layers in Photoshop. I can't look for the URL as of now because my bandwidth is limited.

nishantjr
  • 1,788
  • 1
  • 15
  • 39
  • 1
    I think this is the talk you're refering to: https://channel9.msdn.com/Events/GoingNative/2013/Inheritance-Is-The-Base-Class-of-Evil ; papers here: https://github.com/sean-parent/sean-parent.github.io/wiki/Papers-and-Presentations – Mat Nov 16 '14 at 16:01
  • This is mostly a hypothetical example - note what's making `costly` expensive isn't actually const. But I get your idea; it's essentially introducing a "semi-`costly`" intermediate. I'll have a look at the talk, might be a worthwhile line of reasoning. – Meeuwisse Nov 16 '14 at 16:30
0

I think part of the problem is the arithmetic operators are best suited to value types that are relatively cheap to copy. If you want to avoid copying costly at all I think it is best to avoid overloading these operators.

It may put too much logic on costly but you could add a function to costly that does exactly what you want without making any copies:

void addWithMultiple(const costly& rhs, int mul) {
    b += rhs.b * mul;
}

which can then by called by cheap like so:

void operator() (costly &b, const std::vector<costly> &a) {
    b.addWithMultiple(a[index], mul);
}

But it is a considerable refactoring from what you started with so might not meet all your needs.

Chris Drew
  • 14,926
  • 3
  • 34
  • 54
  • I solved it earlier as b.add(c, a) or something along those lines but was bummed out `cheap` had to allow `costly` access. Your suggestion also works, but I still would prefer it to return the result rather than modify an argument. – Meeuwisse Nov 16 '14 at 15:16
  • If you return a result, that result is fundamentally a different instance to any other you currently have so there is no way of avoiding the copy and there is nothing RVO can do to help you. – Chris Drew Nov 16 '14 at 16:25
  • That makes a disappointing lot of sense. Can't return an rvalue of nothing. – Meeuwisse Nov 16 '14 at 16:35