I’ve written in the past that readability is the most important -ility, until it isn’t. Readability stops being the most important -ility once you can prove it impedes scalability. The key there is that you need to prove it.
As Aaron noted in one of his recent posts, we have really good metrics baked into our system. A few weeks ago, we started seeing that that something was taxing out system in a profound way. Looking into thread dumps, we noticed a particular stack profile that seemed over represented. Looking more closely, we learned that several users from one customer were hitting a screen that was, in retrospect, the equivalent of us building our house out of straw.
The big bad wolf (customer) liked the page built out of straw. They used it often, despite the fact that I’m sure it was totally unusable at their scale. That page happened to repeatedly hit a code path that uses an incredibly readable idiom in a recursive function.
The problem in our app was related to project scoping. Here’s a quick primer on project scoping for those of you who don’t use Rally: each workspace has a tree of projects. On most screens, you can choose to include parent projects (scopeUp) and/or child projects (scopeDown).
The offending code looked something like this:
Project currentProject = getCurrentProject(); List<Project> inScopeProjects = getInScopeProjects(currentProject, scopeUp, scopeDown); return Iterables.filter(inScopeProjects, openAndAccessibleTo(currentUser);
This code is totally readable. The function getInScopeProjects() uses two recursive functions to do its jobs. The problem with those functions was that they traverse our domain model, which has ample N+1 potential. Thanks (but no thanks) to our second-level cache, the recursive N+1 wasn’t as bad as it could be in terms of database calls. What we were seeing was thrashing because of the sheer number of objects built, and occasional DB high-load events because soft-referenced objects were repeatedly GC-ed and rebuilt (re-queried for).
The bottom line is that readable in the context of ORM is not necessarily the same as scalable. Once you know that an approach is no longer scalable, it’s ok to ditch readable code. You should replace it with more readable code, but some level of abstraction that impedes readability is welcome in this scenario. In another post, I’ll discuss the technical solution to this problem.