On Mon, Mar 30, 2009 at 9:53 AM, Jeff Ortel <jor...@redhat.com> wrote:
> Yes, Mike is exactly right here.
>
> The only reason to wrap a log.debug() in log4j is when passing the
> argument(s) is very expensive and a simple (2) string concatenation doesn't
> qualify. Even passing an object with an expensive toString() argument isn't
> a problem because the log4j lib is optimized to process the args only when
> the specified logging level criteria is met.
Actually passing an object with an expensive toString() *IS* the problem.
log4j is NOT the problem in any of these cases. As you stated, log4j won't
bother logging DEBUG if we're set to INFO. But the expensive toString() will
still get evaluated before the call to log.debug() is made.
If we want to make the rule be only add isDebugEnabled() when the log message
is expensive to create i.e. more than a simple string, then I'm ok with that.
Leaving them out completely? a bad idea.
[snip]
>> Concatenation does cost ya ...
>>
>> StopWatch st = new StopWatch();
>> String rand = TestUtils.randomString();
>> st.start();
>> for (int i = 0; i < 100000000; i++) {
>> log.debug("Some line of debug" + rand);
>> }
>> st.stop();
>>
>> is actually around 12 seconds. Not horrible but could be bad if the
>> strings are large.
Right 12 seconds * the number of log.debug() that do concatenations in that
particular code path. Sure you're doing a lot of them above, but still.
http://logging.apache.org/log4j/1.2/faq.html#2.3
http://wordstoday.wordpress.com/2007/11/26/log4j-why-use-isdebugenabled-in-your-code/
http://livingtao.blogspot.com/2007/05/when-to-call-isdebugenabled-before.html
And a scary but interesting idea is posed in this blog:
http://surguy.net/articles/removing-log-messages.xml
In summary, let's wrap them if we're doing concatenations especially if it's
object.toString() but for simple "this is a debug message" it's optional.
thoughts?
jesus
_______________________________________________
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel
