16

What is the point of having the if statement here? The severity level can be changed in log4j.xml config file. If the severity level is debug, it'll log the debug message, else it will not.

What is the significance of the if statement below?

   if (log.isDebugEnabled())
    {
        log.debug("I am logging something");
    }

3 Answers 3

27

In your example there is no need for an 'if' statement

But if you take the below example then you can see that we do a concatenation and if the log level is info then unneccesarily this concatenation operation will be done. For better performance we check this condition.

if (log.isDebugEnabled())
{
    log.debug("I am logging " + 1234 + ".");
}

Extra info:

Use slf4j to avoid such if conditions. The above statement can be rewritten in slf4j as follows,

log.debug("I am logging {} .", 1234);
1
  • 2
    on the other hand, we can discuss what has better performance - one if (+1 getter) or several method calls to determine if debug is enabled or disabled...
    – Betlista
    May 3, 2012 at 9:47
3

It's a performance optimisation. It's done to avoid evaluating the arguments to log.debug. It's only worth doing if constructing the log message is particularly expensive (e.g. serialising an XML document).

This is covered in some detail in the Short introduction to log4j.

5
  • The reference article says "This cost of parameter construction can be quite high and it depends on the size of the parameters involved." The article does not say that the test is only worth doing if the construction cost is "particularly expensive" - that's your interpretation or experience. In my opinion it's better to get into the habit of doing the test.
    – djna
    May 3, 2012 at 11:03
  • No, I think that's wrong. It's unnecessary unless constructing the parameters is demonstrably expensive. Unless you're logging e.g. within a tight loop (which you probably wouldn't be, as you'd generate a huge amount of log noise), concatenating a couple of strings won't noticeably affect performance.
    – harto
    May 3, 2012 at 12:22
  • 2
    in any specific case, I accept that it may be likely it's overkill. For me the problems are : a). programmers tend to adopt habits and use them without considering each case b). code changes: used to be that "X="+X was cheap, then someone changed X's toString(), now it's not. It's just too hard to repeatedly perform the performance analysis. It's not so costly in terms of effort or program size to add the test.
    – djna
    May 3, 2012 at 17:28
  • 1
    Well, it's a good point you make about changing toString().
    – harto
    May 7, 2012 at 1:05
  • I'm late to the party, but I just want to comment on "It's not so costly in terms of effort or program size to add the test." - I think this is arguable, not a huge effort to add the test, but to read it. I think this clutters the code a lot, and it is likely that the performance bottleneck is not there. If a toString() method changes from printing a couple properties to serializing a huge XML, I think your logging messages should probably change anyway, because the printed class must have been completely reworked.
    – Joffrey
    Mar 16, 2015 at 14:11
2

This is considered as good practice. For example if there is some string concatenation it's not evaluated and checked in log4j, but it is checked first.

Example:

if ( log.isDebugEnabled() ) {
    log.debug( "N=" + N + ", a=" + Arrays.toString( a ) );
}

method Arrays.toString() and also concatenation is not performed if debug is not enabled. If there is no if it is invoked first and checked later, that's all ;-)

My opinion is that when there is simple String as in your example the if around the logging is not needed, if there is something more complicated (even more complicated as in my example) this can save some CPU time in production mode (without debug mode enabled).

You have to realize also that when in case of concatenation there is String.valuOf() call which (for not null objects) calls toString() method, which can be really performance issue for big data objects (beans with many properties) if you consider that it's invoking no business logic (therefore it is "useless").

5
  • No concatenation implies fewer objects being created, implies less garbage to collect. Deep in inner loops etc the savings can be quite significant.
    – djna
    May 3, 2012 at 9:25
  • I do not understand your first sentance well. In my example even if debug is disabled objects "N=", ", a=" are created (as constants in compile time), so there is no memory saving. Maybe you can describe more ;-)
    – Betlista
    May 3, 2012 at 9:31
  • Generally folks don't just log some constant strings, they log "X="+x+",y="+y etc. showing values. In many cases you may well call toString() on some object, which itself may concatenate values. Better to use a safe idiom in all cases ...
    – djna
    May 3, 2012 at 10:58
  • But is it "considered good practice" always, and in particular in OP's case, or only when there is considerable (by whatever measure) complexity in creating the message (not) to be logged?
    – tobias_k
    Apr 3, 2018 at 14:12
  • I'd say that djna described it well in other comments - "It's just too hard to repeatedly perform the performance analysis." and "programmers tend to adopt habits and use them without considering each case", with this in mind I'd say today it's good practice in all cases (including OP's one).
    – Betlista
    Apr 3, 2018 at 14:25

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.