[infinispan-dev] trace vs log.isTraceEnabled

[Bela Ban]

On 12/7/11 1:07 PM, Sanne Grinovero wrote:
> On 7 December 2011 12:04, Bela Ban<bban at redhat.com>  wrote:
>>
>>
>> On 12/7/11 12:56 PM, Sanne Grinovero wrote:
>>> On 7 December 2011 11:19, Bela Ban<bban at redhat.com>  Â wrote:
>>>> If you use a constant, this is not an issue, but what about:
>>>>
>>>> String constant="Error message bla bla";
>>>> for(int i=1; i<=10; i++) {
>>>> Â  Â log.trace(constant + "-" + i);
>>>> }
>>>>
>>>> ?
>>>
>>> That should definitely be rewritten as
>>>
>>> String constant="Error message bla bla";
>>> if (log.isTraceEnabled()) {
>>> Â  Â  final String constant="Error message bla bla";
>>> Â  Â  for(int i=1; i<=10; i++) {
>>> Â  Â  Â  Â log.trace(constant + "-" + i);
>>> Â  Â  }
>>> }
>>
>>
>>
>> No ! My example was meant to show a loop with *many* statements; one of
>> them being a log.trace() !
>>
>> E.g.
>>
>> for(......) {
>>
>> ...
>>
>> log.trace();
>>
>> ...
>> }
>
> sorry :)
> Well then this way:
>
> final boolean trace = log.isTraceEnabled();
> for(int i=1; i<=10; i++) {
>   ....//other statements
>     if (trace) log.trace( ...{preferrabily a JBLogger format}... );
>     ...//other statements
> }


Better, but this is awkward code IMO. You have to introduce additional 
variables before tight loops, hmm...

I used to do (in JGroups):

public static final boolean trace=true;

and

#1 if(trace && log.isTraceEnabled()) {
#2   log.trace();
#3 }

If I wanted to create a version without tracing, I compiled JGroups with 
trace=false. The compiler would then optimize lines 1-3 away.

Turns out this was awkward and I removed it in the next release, as I 
couldn't change the logging at runtime (e.g. via probe.sh).

My current philosophy is that I do use the "if(log.isTraceEnabled() 
log.trace()" pattern, but I use it wisely, and I try not to use this in 
tight loops, or frequently traversed code paths.

-- 
Bela Ban
Lead JGroups (http://www.jgroups.org)
JBoss / Red Hat