incubator-cassandra-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Dave Brosius <dbros...@mebigfatguy.com>
Subject Re: slf4j
Date Fri, 23 Nov 2012 17:17:20 GMT
There are actually 2 arguments the OP is making.. the second, using

{} : {}


over

descriptor + ": " + (System.currentTimeMillis() - start)

is reasonable.


On 11/23/2012 10:18 AM, Jonathan Ellis wrote:
> 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.connolly@gmail.com> wrote:
>> On 22 November 2012 17:51, Radim Kolar <hsn@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.
>>>
>
>


Mime
View raw message