Monday 31 December 2012

Don't optimise your code

G'day:
Well that headline is a gross over-simplification of what I'm going to say here. Obviously one should write code that's optimal for the task at hand, but there's a time and place for doing this sort of thing.

The Railo Google Group seems to be my muse this week (which is a sorry indictment of me on a coupla levels), after yesterday's [ed: it was y/day when I started this, it was a few days ago now] inspiration regarding making CFML built-in functions "first-class", and now this article about premature optimisation.


Here's another thread on that mailing list: "what is faster: compare(str1,str), EQ, IS ?". This is a case of SSIA, but you should read the thread if you like. Here's the important bit of the original question:

[...]
From past experience using Marcomedia and/or Adobe Coldfusion I always used " Not compare(str1,str1) is 0" to test if strings are not equal.
Now I am wondering if this habit is outdated.
Is there any performance difference between :

Not Str1 eq Str2
Not Str is Str2
Not Compare(str1,str2) Is 0

[...]
Firstly, it's a reasonable question to ask. However I think the rest of the thread is mostly dithering, other than something Peter Boughton says:

In 99% of cases the answer is: it doesn't matter.

The only time it matters is when there is a measurable difference - otherwise, if you don't have any actual proof of speed differences, use the one that is most readable.
By way of agreement, I posted in response to what Peter says basically a "yeah, wot he said", which seemed reasonable to me. This however yielded a bitch-slap by someone else, which yielded a disproportionately grumpy response from me (unapologetically disproportionate, if I'm to be honest about it), but by then my interest was piqued: I wanted to find out whether there was any actual merit in the ongoing discussion.

The gist of my second response (the grumpy one), was it was all very fascinating dithering about the topic, but unless someone was gonna pony-up with some sort of answer, there was little point beyond "don't worrry about it".

Anyway, my instinct was that there would be some sort of difference between using compare(), compareNoCase(), and EQ by way of checking string equality. I expected compare() to be fastest (it's case-sensitive, so has less work to do), and the other two to be pretty much on a par. But I also thought that for all intents and purposes, the difference would be insignificant, and definitely fall into the "premature optimisation bucket", or even the "don't even bother" bucket. But I thought that perhaps with really big strings, it might make a tangible difference.

So I concocted a quick test. Here's the code:

Basically this reads in a string that is 1000000 characters long, which was randomly generated by a different bit of code, before hand (more about that later), and then duplicates it and checks if the two duplicates are equal.

It permits some variations:
  • it can change the length of one of the strings being compared, in case this improves performance: a length check will be cheaper than a value check, and if the lengths differ, the strings ain't equal irrespective of the contents.
  • It can test equal strings or unequal strings. I suspected testing equal strings might be slower than checking unequal ones, if the comparison is char-by-char: equal strings need to be checked their entire length; unequal strings become unequal as soon as there's a difference, which will - intrinsically - be before the end of the string.
  • There's also a param to reset the metrics being stored in session (to track multiple runs).
  • And, lastly but most significantly, it allows a number of different comparison options:
    • compare()
    • compareNoCase()
    • EQ
    • compare() EQ 0

Before I get to the results, I'd just like to clarify that I think tests like this are mostly a stupid waste of time. They do not reflect the reality of how code is used in production, nor do they accurately reflect all the contributing factors that will impact code. Tests that do millions of loop-de-loops to eke amplify some meagre difference in code performance are just as bad (or worse, as they start from the premise that what is being tested won't give conclusive results if only tested once), and whilst simultaneously load testing rigs like JMeter are better, they still don't really demonstrate much. If you have a perceived performance issue in your code, don't write tests like this thinking it will necessarily demonstrate anything. I suppose it can demonstrate the difference between one coding approach that ends up taking 1000ms to run, and another than takes 1ms to run, but even then this sort of thing can be impacted by other code running simultaneously in a production environment.

I also think that measurements with the "accuracy" of these results probably fall within a margin of error based on the "background noise" of other things the computer is doing at the time. They're mostly meaningless even by way of comparison.

But for the purposes of what I'm looking at here, this sort of test will do. Given it's measuring an arbitrary, non-real-world issue anyhow (or, indeed, just a non-issue. That was perhaps a spoiler ;-).

My test was as follows:
  • hit the file without any modifiers (so just hitting comparator.cfm) a coupla times so that it's compiled and the code is in server memory. This should only take one hit, but I have found that template performance continues to improve for about the first four (or so) requests. This might be because it's being migrated through various generations in the heap? Dunno. But I always hit my code a few times before using it in any benchmarking tests.
  • Then I hit it with comparator.cfm?reset=true) to clear the metrics that had already been put into session.
  • Then I took the reset=true back off, and performed another 19 requests, so I got the mean of 20 results.
  • Next I did the same thing with comparator.cfm?method=comparenocase;
  • then comparator.cfm?method=eq;
  • then comparator.cfm?method=compare_eq_0;
  • and finally with method=eq, and lendiff=true.
  • That was on CF10: next I did all that on Railo 4.0.2.
 The results were as follows:

Test Mean (ms)
CF Railo
compare11
compareNoCase43
eq34
compare_eq_011
lendiff=true, method=eq33

(a bunch of the metrics I'm reporting in that code were from me troubleshooting stuff, or just verifying the code was working how I expected it to, and are otherwise meaningless to the results).

So to make this perfectly clear: the difference between best and worst at comparing the equality of two million-character strings via different methods is 3ms. One could conclude that compare() is 3-4 times faster than using the case-insensitive options, but I think it'd perhaps more important to focus on the 2-3ms diff between fastest and slowest. Not significant. Bear in mind that this is comparing two vast strings... if one was doing it on more typically-sized strings, there just isn't going to be a perceivable difference over all the rest of the background noise. Also bear in mind that - by way of coparison - to read the data from disk took about 15-20ms on my machine. And fetching that from a DB would be similar. And creating the strings was orders of magnitude slower (best measured in seconds, not milliseconds).

So... which approach is "better" as far as performance goes?
  1. It doesn't frickin' matter;
  2. but compare() is, all things being equal, slightly faster.
So, back to the title of this article: if this is the sort of thing you're looking at by way of performance improvement in your code: don't bother.

You should focus on what makes your code clearer in this case. There are definitely cases for using compare(): case-sensitive string comparisons being the most obvious: EQ is not case-sensitive. Also if one is concerned about CF casting a string to a numeric when doing a EQ comparison, compare() will prevent this. EG:

<cfscript>
    s1 = "0100";
    s2 = "100";
    writeOutput("#s1# EQ #s2#: #s1 EQ s2#<br />");
    writeOutput("compare(#s1#, #s2#): #compare(s1,s2)#<br />");
</cfscript>

This outputs:
0100 EQ 100: true
compare(0100, 100): -1

But, other than that, there's probably no reason to use compare() when EQ or == is less typing and kinda clearer as to what is going on. Plus if one only uses compare() when it's necessary, it signals that its usage in code is significant (if that makes sense).

Also, if your code is running sub-optimally, I would not mess around with this sort of thing, and look at any number of other things that are more likely to be a problem, and will be easier to get better "wins" out of.

I'm gonna write another quick article on what I found about creating million-character strings... that had some interesting differences.  Back soon...

--
Adam