Performance hit after non-functional refactoring

Question

I was asked to refactor some C++ code recently for the purposes of increasing unit testing coverage. The problem was that the code was tightly coupled on one compilation unit, so we had the equivalent of this:

beatles.h

class beatles { ... }

beatles.cpp

class john { ... }
class paul { ... }
class ringo { ... }
class george { ... }

...

// Implementation of beatles, john, paul, ringo and george classes ...

Because the 'internal' classes were only visible within the implementation file, they couldn't be unit tested very easily. Not only that, but the implementation file was huge and tricky to follow. I refactored the header and implementation to give something more like the following:

john.h

class john { ... }

paul.h

class paul { ... }

ringo.h

class ringo { ... }

george.h

class george { ... }

beatles.h

#include <john.h>
#include <paul.h>
#include <ringo.h>
#include <george.h>

class beatles { ... }

For simplicity, I've omitted that each one has its own separate implementation file. Naturally this is a drastic simplification but hopefully it illustrates the point.

Now we can add unit tests for the classes that previously couldn't be tested, which is great. However, for some of the operations at runtime we now see a significant performance hit, which we've concluded is due to the compiler not being able to optimise as much as it previously was, as there are absolutely no functional differences in this change.

Now, I'm aware of the benefits of loose coupling and strong cohesion. However, there comes a point where other factors such as code maintainability and testing etc. also start to come into this. I have to admit that I'm also a little uncomfortable that simply structuring our code one way or another can have such a profound, and potentially unpredictable affect on aspects such as performance.

My question is this: Is it plausible to think that the optimiser is at the root of this issue and, if so, is there a way that we can structure the code to allow the public interface for testing purposes while not throttling the optimiser?

Answer 1

You didn't say what percentage in performance change. And whether it is for a tight benchmark or as part of an application.

You might see a function getting ten times slower - from 1 nanosecond to 10 - which turns out to be completely irrelevant. Maybe another function changes from 1,000 to 1,009 nanoseconds for the same reason, and that's even more irrelevant.

The optimiser might see that calling function f() has no side effects. If you call it in a loop a billion times it still has no side effect, so that loop has zero runtime. But if the function is hidden in another compilation unit, the compiler doesn't know, and makes a billion calls.

Performance of benchmarks is irrelevant. What counts is performance of actual production code.

You might also check if you haven't made other changes unintentionally. For example, you might have turned optimisation off while refactoring for easier debugging, and not turned it on again.

Answer 2

It's certainly plausible that moving definitions to a new translation unit has meant the optimiser couldn't do as well.

You can #include whatever files you like, so you can re-create the old structure as seen by the compiler while keeping the new files.

beatles.h

class beatles { ... }

beatles.cpp

#include "john.cpp" // transitively #includes "john.h"
#include "paul.cpp"
#include "george.cpp"
#include "ringo.cpp"

...

// Implementation of beatles

test_john.cpp

#include "john.h"

You will have to be careful that you don't try to link beatles.o and john.o in the same executable.

This structure will surprise some people, so you probably want a big warning at the top of beatles.cpp along the lines of

IMPORTANT! We found an unacceptably long (yyy seconds wall clock time) performance degradation splitting these translation units (under compiler(s) foo x.y.z bar h.j.k)

Do not rearrange unless you can show they all now have better optimisers