"Clean" Code, Horrible Performance

This is a free bonus video from the Performance-Aware Programming series. It shows the real-world performance costs of following “clean code” guidelines. For more information about the course, please see the About page or the Table of Contents.A lightly-edited transcript of the video appears below.Some of the most-often repeated programming advice, especially to beginner programmers, is that they should be writing “clean” code. That moniker is accompanied by a long list of rules that tell you what you should do in order for your code to be “clean”.A large portion of these rules don't actually affect the runtime of the code that you write. These sorts of rules cannot be objectively assessed, and we don't necessarily have to because they're fairly arbitrary at that point. However, several “clean” code rules — some of the ones most emphatically stressed — are things we can objectively measure because they do affect the runtime behavior of the code.If you look at a “clean” code summary and pull out the rules that actually affect the structure of your code, you get:Prefer polymorphism to “if/else” and “switch”Code should not know about the internals of objects it’s working withFunctions should be smallFunctions should do one thing“DRY” - Don’t Repeat YourselfThese rules are rather specific about how any particular piece of code should be created in order for it to be “clean”. What I would like to ask is, if we create a piece of code that follows these rules, how does it perform?In order to construct what I would consider the most favorable case for a “clean” code implementation of something, I used existing example code contained in “clean” code literature. This way, I am not making anything up, I’m just assessing “clean” code advocates’ rules using the example code they give to illustrate those rules.If you look at “clean” code examples, you'll often see an example like this:/* ======================================================================== LISTING 22 ======================================================================== */ class shape_base { public: shape_base() {} virtual f32 Area() = 0; }; class square : public shape_base { public: square(f32 SideInit) : Side(SideInit) {} virtual f32 Area() {return Side*Side;} private: f32 Side; }; class rectangle : public shape_base { public: rectangle(f32 WidthInit, f32 HeightInit) : Width(WidthInit), Height(HeightInit) {} virtual f32 Area() {return Width*Height;} private: f32 Width, Height; }; class triangle : public shape_base { public: triangle(f32 BaseInit, f32 HeightInit) : Base(BaseInit), Height(HeightInit) {} virtual f32 Area() {return 0.5f*Base*Height;} private: f32 Base, Height; }; class circle : public shape_base { public: circle(f32 RadiusInit) : Radius(RadiusInit) {} virtual f32 Area() {return Pi32*Radius*Radius;} private: f32 Radius; };It’s a base class for a shape with a few specific shapes derived from it: circle, triangle, rectangle, square. We then have a virtual function that computes the area.Like the rules demand, we are preferring polymorphism. Our functions do only one thing. They are small. All that good stuff. So we end up with a “clean” class hierarchy, with each derived class knowing how to compute its own area, and storing the data required to compute that area.If we image using this hierarchy to do something — say, finding the total area of a series of shapes that we pass in — we would expect to see something like this:/* ======================================================================== LISTING 23 ======================================================================== */ f32 TotalAreaVTBL(u32 ShapeCount, shape_base **Shapes) { f32 Accum = 0.0f; for(u32 ShapeIndex = 0; ShapeIndex < ShapeCount; ++ShapeIndex) { Accum += Shapes[ShapeIndex]->Area(); } return Accum; }You'll notice I haven't used an iterator here because there was nothing in the rules that suggested you had to use iterators. As such, I figured I would give “clean” code the benefit of the doubt and not add any kind of abstracted iterator that might confuse the compiler and lead to worse performance.You may also notice that this loop is over an array of pointers. This is a direct consequence of using a class hierarchy: we have no idea how big in memory each of these shapes might be. So unless we were going to add another virtual function call to get the data size of each shape, and use some kind of a variable skipping procedure to go through them, we need pointers to find out where each shape actually begins.Because this is an accumulation, there's a loop-carried dependency here which might slow the loop down. Since accumulation can be reordered arbitrarily, I also wrote a hand-unrolled version just to be safe:/* ======================================================================== LISTING 24 ======================================================================== */ f32 TotalAreaVTBL4(u32 ShapeCount, shape_base **Shapes) { f32 Accum0 = 0.0f; f32 Accum1 = 0.0f; f32 Accum2 = 0.0f; f32 Accum3 = 0.0f; u32 Count = ShapeCount/4; while(Count--) { Accum0 += Shapes[0]->Area(); Accum1 += Shapes[1]->Area(); Accum2 += Shapes[2]->Area(); Accum3 += Shapes[3]->Area(); Shapes += 4; } f32 Result = (Accum0 + Accum1 + Accum2 + Accum3); return Result; }If I run these two routines in a simple test harness, I can get a rough measure of the total number of cycles per shape that are required to do that operation:The harness times the code in two different ways. The first way is running the code only once, to show what happens in an arbitrary “cold” state — the data should be in L3 but L2 and L1 have been flushed, and the branch predictor has not “practiced” on the loop.The second way is running the code many times repeatedly, to see what happens when the cache and branch predictor are operating in their most favorable way for the loop. Note that none of these are hard-core measurements, because as you’ll see, the differences we’ll be looking at are so large that we don’t need to break out any serious analysis tools.What we can see from the results is there's not much of a difference between the two routines. It's around 35 cycles to do the “clean” code area calculation on a shape. Maybe it gets down more towards 34 sometimes if you're really lucky.So 35 cycles is what we can expect from following all the rules. What would happen if instead we violated just the first rule? Instead of using polymorphism here, what if we just use a switch statement instead1?Here I've written the exact same code, but instead of writing it using a class hierarchy (and therefore, at runtime, a vtable), I've written it using an enum and a shape type that flattens everything into one struct:/* ======================================================================== LISTING 25 ======================================================================== */ enum shape_type : u32 { Shape_Square, Shape_Rectangle, Shape_Triangle, Shape_Circle, Shape_Count, }; struct shape_union { shape_type Type; f32 Width; f32 Height; }; f32 GetAreaSwitch(shape_union Shape) { f32 Result = 0.0f; switch(Shape.Type) { case Shape_Square: {Result = Shape.Width*Shape.Width;} break; case Shape_Rectangle: {Result = Shape.Width*Shape.Height;} break; case Shape_Triangle: {Result = 0.5f*Shape.Width*Shape.Height;} break; case Shape_Circle: {Result = Pi32*Shape.Width*Shape.Width;} break; case Shape_Count: {} break; } return Result; }This is the “old school” way you would have written this before “clean” code.Note that because we no longer have specific datatypes for every shape variant, if a type doesn’t have one of the values in question (like “height”, for example), it simply doesn’t use it.Now, instead of getting the area from a virtual function call, a user of this struct gets it from a function with a switch statement: exactly the thing that a “clean” code lecture would tell you to never ever do. Even so, you’ll note that the code, despite being much more concise, is basically the same. Each case of the switch statement is just the same code as the corresponding virtual function in the class hierarchy.For the summation loops themselves, you can see that they are nearly identical to the “clean” version:/* ======================================================================== LISTING 26 ======================================================================== */ f32 TotalAreaSwitch(u32 ShapeCount, shape_union *Shapes) { f32 Accum = 0.0f; for(u32 ShapeIndex = 0; ShapeIndex < ShapeCount; ++ShapeIndex) { Accum += GetAreaSwitch(Shapes[ShapeIndex]); } return Accum; } f32 TotalAreaSwitch4(u32 ShapeCount, shape_union *Shapes) { f32 Accum0 = 0.0f; f32 Accum1 = 0.0f; f32 Accum2 = 0.0f; f32 Accum3 = 0.0f; ShapeCount /= 4; while(ShapeCount--) { Accum0 += GetAreaSwitch(Shapes[0]); Accum1 += GetAreaSwitch(Shapes[1]); Accum2 += GetAreaSwitch(Shapes[2]); Accum3 += GetAreaSwitch(Shapes[3]); Shapes += 4; } f32 Result = (Accum0 + Accum1 + Accum2 + Accum3); return Result;