Re: [infinispan-dev] trace vs log.isTraceEnabled
On 12/7/11 1:07 PM, Sanne Grinovero wrote:
On 7 December 2011 12:04, Bela Ban<bban(a)redhat.com> wrote:
>
>
> On 12/7/11 12:56 PM, Sanne Grinovero wrote:
>> On 7 December 2011 11:19, Bela Ban<bban(a)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