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

Reply via email to