log.info using log.isInfoEnabled

Question

We are using SLF4J

Based on a recent discussion in the team

if(LOG.isDebugEnabled()){
  LOG.debug("hello " + a + " world" + b);
}

is better than

LOG.debug("hello {} world {}", a, b);

because in the latter case, the String hello {} world {} is created even if "debug" is not enabled. In other words, we are always creating Strings even when it is not required.

I like the latter version as it improves readability significantly.

Can anybody please provide inputs?

Regards,

Shardul.

EDIT

Let me put it differently.

Which is a better approach OR Which is the most widely accepted approach?

Shardul.

Answer 1

The performance impact of creating the String would be negligible. But as SLF4J documentation specifies "...this form does not incur the cost of parameter construction in case the log statement is disabled " meaning that the String is not even created.

After some consideration, the "human overhead", i.e. the importance of code clarity made us decide in favor fo the second example. More so as there is also overhead involved in calling (and writing and reading) the isXEnabled mehod.

Answer 2

No, the shorter version does not create the "hello {} world {}" string. The string is already created and placed in the constant pool during compilation and class loading. You are always referencing the same instance from the constant pool, it is as cheap as referencing constant int.

But the string is created in the first form since you are using string concatenation.

The only extra overhead is calling LOG.debug with three arguments which does nothing as it calls isDebugEnabled() internally. Also chances are it will be inlined.

That being said I would go for shorter form in 99% of the cases. The only situation where I would explicitly call isDebugEnabled is when computing the message to be logged has a significant cost:

if(LOG.isDebugEnabled()){
  LOG.debug("hello {} world {}", cheap, veryExpensive());
}

If veryExpensive() is, ekhem, very expensive, then it is smart to avoid calling it if it will be discarded. BTW veryExpensive() should not have any side effects and it is hard to imagine long-running, side-effect free method...