Shall I optimize or let compiler to do that?

Question

What is the preferred method of writing loops according to efficiency: Way a)

   /*here I'm hoping that compiler will optimize this  
 code and won't be calling size every time it iterates through this loop*/
    for (unsigned i = firstString.size(); i < anotherString.size(), ++i)
    {
    //do something
    }

or maybe should I do it this way: Way b)

unsigned first = firstString.size();
unsigned second = anotherString.size();

and now I can write:

    for (unsigned i = first; i < second, ++i)
    {
    //do something
    }

the second way seems to me like worse option for two reasons: scope polluting and verbosity but it has the advantage of being sure that size() will be invoked once for each object.
Looking forward to your answers.

Answer 1

I would first use the first version, simply because it looks cleaner and easier to type. Then you can profile it to see if anything needs to be more optimized.

But I highly doubt that the first version will cause a noticable performance drop. If the container implements size() like this:

inline size_t size() const
{
    return _internal_data_member_representing_size;
}

then the compiler should be able to inline the function, eliding the function call. My compiler's implementation of the standard containers all do this.

Answer 2

This is one of those things that you should test yourself. Run the loops 10,000 or even 100,000 iterations and see what difference, if any, exists.

That should tell you everything you want to know.

Answer 3

In general, let the compiler do it. Focus on the algorithmic complexity of what you're doing rather than micro-optimizations.

However, note that your two examples are not semantically identical - if the body of the loop changes the size of the second string, the two loops will not iterate the same amount of times. For that reason, the compiler might not be able to perform the specific optimization you're talking about.

Answer 4

How will a good compiler optimize your code? Not at all, as it can't be sure size() has any side-effects. If size() had any side effects your code relied on, they'd now be gone after a possible compiler optimization.

This kind of optimization really isn't safe from a compiler's perspective, you need to do it on your own. Doing on your own doesn't mean you need to introduce two additional local variables. Depending on your implementation of size, it might be an O(1) operation. If size is also declared inline, you'll also spare the function call, making the call to size() as good as a local member access.

Answer 5

Here's how I look at it. Performance and style are both important, and you have to choose between the two.

You can try it out and see if there is a performance hit. If there is an unacceptable performance hit, then choose the second option, otherwise feel free to choose style.

Answer 6

Don't pre-optimize your code. If you have a performance problem, use a profiler to find it, otherwise you are wasting development time. Just write the simplest / cleanest code that you can.

Answer 7

i usually write this code as:

/* i and size are local to the loop */
for (size_t i = firstString.size(), size = anotherString.size(); i < size, ++i) {
  //do something
}

this way i do not pollute the parent scope and avoid calling anotherString.size() for each loop iteration.

this is especially useful for iterators:

for(some_generic_type<T>::forward_iterator it = collection.begin(), end = collection.end();
    it != end; ++it) {
   // do something with *it
}

Answer 8

I'm hoping that compiler will optimize this...

You shouldn't. Anything involving

• A call to an unknown function or
• A call to a method that might be overridden

is hard for a C++ compiler to optimize. You might get lucky, but you can't count on it.

Nevertheless, because you find the first version simpler and easier to read and understand, you should write the code exactly the way it is shown in your simple example, with the calls to size() in the loop. You should consider the second version, where you have extra variables that pull the common call out of the loop, only if your application is too slow and if you have measurements showing that this loop is a bottleneck.

Answer 9

My recommendation is to let inconsequential optimizations creep into your style. What I mean by this is that if you learn a more optimal way of doing something, and you cant see any disadvantages to it (as far as maintainability, readability, etc) then you might as well adopt it.

But don't become obsessed. Optimizations that sacrifice maintainability should be saved for very small sections of code that you have measured and KNOW will have a major impact on your application. When you do decide to optimize, remember that picking the right algorithm for the job is often far more important than tight code.

Answer 10

You shouldn't optimize your code, unless you have a proof (obtained via profiler) that this part of code is bottleneck. Needless code optimization will only waste your time, it won't improve anything.

You can waste hours trying to improve one loop, only to get 0.001% performance increase. If you're worried about performance - use profilers.

Answer 11

There's nothing really wrong with way (b) if you just want to write something that will probably be no worse than way (a), and possibly faster. It also makes it clearer that you know that the string's size will remain constant.

The compiler may or may not spot that size will remain constant; just in case, you might as well perform this optimization yourself. I'd certainly do this if I was suspicious that the code I was writing was going to be run a lot, even if I wasn't sure that it would be a big deal. It's very straightforward to do, it takes no more than 10 extra seconds thinking about it, it's very unlikely to slow things down, and, if nothing else, will almost certainly make the unoptimized build run a bit more quickly.

(Also the first variable in style (b) is unnecessary; the code for the init expression is run only once.)

Answer 12

For the "std::size_t size()const" member function which not only is O(1) but is also declared "const" and so can be automatically pulled out of the loop by the compiler, it probably doesn't matter. That said, I wouldn't count on the compiler to remove it from the loop, and I think it is a good habit to get into to factor out the calls within the loop for cases where the function isn't constant or O(1). In addition, I think assigning the values to a variable leads to the code being more readable. I would not suggest, though, that you make any premature optimizations if it will result in the code being harder to read. Again, though, I think the following code is more readable, since there is less to read within the loop:

 std::size_t firststrlen = firststr.size();
 std::size_t secondstrlen = secondstr.size();
 for ( std::size_t i = firststrlen; i < secondstrlen; i++ ){
      // ...
 }

Also, I should point out that you should use "std::size_t" instead of "unsigned", as the type of "std::size_t" can vary from one platform to another, and using "unsigned" can lead to trunctations and errors on platforms for which the type of "std::size_t" is "unsigned long" instead of "unsigned int".

Answer 13

1. How much percent of time is spent in for as opposed to // do something? (Don't guess - sample it.) If it is < 10% you probably have bigger issues elsewhere.

2. Everybody says "Compilers are so smart these days." Well they're no smarter than the poor coders who write them. You need to be smart too. Maybe the compiler can optimize it but why tempt it not to?