Is it sensible optimization to check if an iterator has a value that we want to assign in C++

Question

There is a similar question 16114572 But I am more specific here.

Please, answer the question on the 3rd line:

void change_head(map<K, V>& m, V const& value) {
  auto head = m.begin();
  if (head->second != value)   // <--- Make sense?
    head->second = value;
}

Say, the only thing we know about V that is has operator != and it is copyable. However, there is no information about how fast those operations are (copy and !=).

UPD: Imagine, we are building a library. We would probably assume that equality would be faster than copying but I am not sure! Would this optimization worth it?

My primary language is not C++, so I just want to know, what people from C++ world do.

_However, there is no information about how fast those operations are (copy and !=)._ IMHO, this is the important fact which makes this hard to answer. For primitive types, I would prevent the comparison and just assign the value. Preventing the extra branch usually armotizes the assignment. For non-primitive types, it depends on the costs of copy and `operator!=()`. The other question is whether it is worth to optimize a single assignment in general. If it was something which occurs million of times in a loop... — Scheff's Cat, Sep 14 '19 at 16:16
not related to your question but, are you certain about the `const` reference to the map if your intent is to change its content? — prog-fh, Sep 14 '19 at 16:20
You're asking to comment on the performance of this code, but making the caveat that we can't assume anything about the performance of the parts of the code. This question is impossible to answer. — Ayjay, Sep 14 '19 at 17:59

Useless · Accepted Answer · 2019-09-17T15:32:29.103

Is it sensible optimization

It isn't an optimization at all unless you can measure an improvement. It's just optimizy-looking cargo-cult junk, like go-faster stripes.

(That's not an insult - everyone is susceptible to wanting their code to be fast, and this often leads you to write code that looks fast. The solution is to learn not to trust appearances and instead to develop the habit of profiling your code properly).

However, there is no information about how fast those operations are

Don't write extra code unless you have a reason to.

Here, you don't have any reason to believe the longer version is more correct or that it performs better. The only thing we know for sure is that we added some cognitive overhead (there's more code to read) and a test-and-branch (or conditional move).

We should be required to justify writing more and more complex code - which here we cannot. IME this is just as likely to be worse than the naive version because the test-and-branch spills the branch predictor, or because of the load dependency, or because of the extra instruction cache pressure.

Edit:

UPD: Imagine, we are building a library

If you expect the things being stored may be very expensive to copy but cheap to compare, it'd be easier to provide an optional wrapper for the value type instead of baking it into your container:

template <typename T>
class CheapCompareExpensiveCopy
{
    T value_;
public:
    using SelfT = CheapCompareExpensiveCopy<T>;

    // usual constructors etc.
    SelfT& operator=(SelfT const &other)
    {
      if (other.value_ != value_) {
        value_ = other.value_;
      }
      return *this;
    }
};

Is it sensible optimization to check if an iterator has a value that we want to assign in C++

1 Answers1