c++ optimization

Question

I'm working on some existing c++ code that appears to be written poorly, and is very frequently called. I'm wondering if I should spend time changing it, or if the compiler is already optimizing the problem away.

I'm using Visual Studio 2008.

Here is an example:

void someDrawingFunction(....)
{
  GetContext().DrawSomething(...);
  GetContext().DrawSomething(...);
  GetContext().DrawSomething(...);
  .
  .
  .
}

Here is how I would do it:

void someDrawingFunction(....)
{
  MyContext &c = GetContext();
  c.DrawSomething(...);
  c.DrawSomething(...);
  c.DrawSomething(...);
  .
  .
  .
}

Answer 1

If you're sure it's a performance problem, change it. If GetContext is a function call (as opposed to a macro or an inline function), then the compiler is going to HAVE to call it every time, because the compiler can't necessarily see what it's doing, and thus, the compiler probably won't know that it can eliminate the call.

Of course, you'll need to make sure that GetContext ALWAYS returns the same thing, and that this 'optimization' is safe.

Answer 2

Don't guess at where your program is spending time. Profile first to find your bottlenecks, then optimize those.

As for GetContext(), that depends on how complex it is. If it's just returning a class member variable, then chances are that the compiler will inline it. If GetContext() has to perform a more complicated operation (such as looking up the context in a table), the compiler probably isn't inlining it, and you may wish to only call it once, as in your second snippet.

If you're using GCC, you can also tag the GetContext() function with the pure attribute. This will allow it to perform more optimizations, such as common subexpression elimination.

Answer 3

Obviously, if GetContext() has side effects (I/O, updating globals, etc.) than the suggested optimization will produce different results.

So unless the compiler can somehow detect that GetContext() is pure, you should optimize it yourself.

Answer 4

If it is logically correct to do it the second way, i.e. calling GetContext() once on multiple times does not affect your program logic, i'd do it the second way even if you profile it and prove that there are no performance difference either way, so the next developer looking at this code will not ask the same question again.

Answer 5

If you're wondering what the compiler does, look at the assembly code.

Answer 6

That is such a simple change, I would do it.
It is quicker to fix it than to debate it.

But do you actually have a problem?
Just because it's called often doesn't mean it's called TOO often.
If it seems qualitatively piggy, sample it to see what it's spending time at.
Chances are excellent that it is not what you would have guessed.