Mailing-List: contact dev-help@cassandra.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@cassandra.apache.org
Received-SPF: pass (athena.apache.org: domain of
 stephen.alan.connolly@gmail.com designates 74.125.82.172 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <50AFAFA0.9000303@mebigfatguy.com>
References: <50AE660D.1050802@filez.com>
	<CA+nPnMx5bNKfm9irDuCO4vjE8pDiWiyP-wPvXPn3xjbOV-5_Qw@mail.gmail.com>
	<CALdd-zjhbeTnjLvXCEkqH8Q9w_8SfTfzXrCNYd60vSu_3HyJaA@mail.gmail.com>
	<50AFAFA0.9000303@mebigfatguy.com>
Date: Fri, 23 Nov 2012 17:24:55 +0000
Message-ID: 
 <CA+nPnMz-u+5D1ZF57ezsJd-SO-dMVd2j8Q1roy3s_Rb22o6x3Q@mail.gmail.com>
Subject: Re: slf4j
From: Stephen Connolly <stephen.alan.connolly@gmail.com>
To: dev <dev@cassandra.apache.org>
Content-Type: multipart/alternative; boundary=f46d044481bfb74b4304cf2cdcd6

--f46d044481bfb74b4304cf2cdcd6
Content-Type: text/plain; charset=ISO-8859-1

On 23 November 2012 17:17, Dave Brosius <dbrosius@mebigfatguy.com> wrote:

> There are actually 2 arguments the OP is making.. the second, using
>
> {} : {}
>
>
> over
>
>
> descriptor + ": " + (System.currentTimeMillis() - start)
>
> is reasonable.
>
>
Having had to do a search & replace over an entire code base (thank you
IntelliJ structural search) to remove the string concat in logging due to
the GC pressure it caused. I tend to agree on that second point.


>
> 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 <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.
>>>>
>>>>
>>
>>
>

--f46d044481bfb74b4304cf2cdcd6--