I prefer the concise approach when no evaluation needs to be performed on the method arguments, but when it does I prefer the explicit isDebugEnabled check, or else reviewers need to think each time they see one, "is this a hot code path where we can afford to be sloppy, or not?"
On Fri, Nov 23, 2012 at 9:14 AM, Stephen Connolly <stephen.alan.conno...@gmail.com> wrote: > On 22 November 2012 17:51, Radim Kolar <h...@filez.com> wrote: > >> instead of this: >> >> if (logger.isDebugEnabled()) >> logger.debug("INDEX LOAD TIME for " + descriptor + ": " + >> (System.currentTimeMillis() - start) + " ms."); >> >> do this: >> logger.debug("INDEX LOAD TIME for {} : {} ms.", descriptor, >> (System.currentTimeMillis() - start)); >> > > Yes, but until/unless the JVM elides the function call because that logger > is not enabled fro debug, you will incur the penalty of new > Object[]{string,new Long(System.currentTimeMillis() - start). > > On top of that, when debug is enabled you incur the cost of formatting the > string, including the {} search & replace. > > On a long running production system, you are correct that it should > *eventually* be equivalent, and I am not saying one way or the other > whether this should/shouldn't be changed... more just pointing out that > there are consequences to what might appear to be a trivial change... I'll > let the c* devs chime in as to what their strong opinion is in this regard > as they should have more experience handling high loads and I would love to > know which pattern I should be using in my code (FWIW I tend to favour your > approach to the if (debugEnabled) guard... but I always wonder ;-) ) > > -Stephen > > >> >> easier to read. >> -- Jonathan Ellis Project Chair, Apache Cassandra co-founder of DataStax, the source for professional Cassandra support http://www.datastax.com